Use browser sessionStorage API to back rememberSaveable on web about compose-multiplatform HOT 5 OPEN

Still didn't dig too deep, but it looks like browser history should be single source of truth there and compose navigation should just handle url from history like deeplink. But it doesn't seem really related to this topic

I've been thinking about this topic specifically with compose navigation in mind so I'm a little dialed in there.

Maybe this should be a separate issue, but it does highlight the need for persisted nav stack for me. The interaction between NavController and browser history is pretty complex if you want to maintain the user expectation of how navigation works in a browser, while also allowing the developer to use compose navigation naturally (which is important for a seamless multiplatform developer experience).

The browser history can't be the single source of truth unless the implementation of compose navigation is rewritten for web to use browser history, because as the application interacts with the NavController the changes need to be propagated to browser history, and as the user interacts with browser history the changes need to be propagated to NavController.

I'm finishing up my work on this (i.e. force pushing onto master), but if you want to see the issue, take a look at:

https://github.com/eygraber/virtue/tree/master/virtue-nav

https://github.com/eygraber/virtue/tree/master/virtue-history/

https://github.com/eygraber/virtue/blob/master/virtue-session/src/commonMain/kotlin/com/eygraber/virtue/session/VirtueSession.kt#L47

from compose-multiplatform.

Let's keep it and see what @eymar and @Schahen (they are responsible for web part in the team) think about it.

For me it doesn't sound as good default behaviour because rememberSaveable is about saving view state like scroll position. After explicit refresh I'd expect that it won't be saved.

from compose-multiplatform.

I tested a bunch of websites and some of them do maintain the scroll position and typed text after an explicit refresh or closing and restoring the browser (e.g. GitHub, etc...) while others don't (e.g. Reddit, Twitter, etc...). I guess it comes down to the content of the website. Not sure what would be a good default there though.

I think it does become important with navigation, especially since the browser will remember the history stack across refresh and restoring the browser, so if something like NavController doesn't it could be problematic.

Here's an example of how I'm doing it with my bridge class between NavController and browser history (not sure if it's correct because I haven't tested it yet):

private const val SAVE_KEY = "com.eygraber.virtue.history.History"

private class HistorySaveableStateRegistry(
  private val browserPlatform: BrowserPlatform,
  private val backPressDispatcher: OnBackPressedDispatcher,
) : SaveableStateRegistry by SaveableStateRegistry(
  restoredValues = mapOf(
    SAVE_KEY to listOf(
      browserPlatform.loadSessionState(SAVE_KEY)?.let { savedState ->
        with(
          WebHistory.Saver(
            browserPlatform = browserPlatform,
            backPressDispatcher = backPressDispatcher
          )
        ) {
          restore(savedState)
        }
      } ?: WebHistory(
        browserPlatform = browserPlatform,
        history = TimelineHistory(),
        backPressDispatcher = backPressDispatcher
      )
    )
  ),
  canBeSaved = { it is String }
) {
  fun save() {
    val map = performSave()
    map[SAVE_KEY]?.firstOrNull()?.let { history ->
      if(history is String) {
        browserPlatform.saveSessionState(SAVE_KEY, history)
      }
    }
  }
}

@Composable
public actual fun rememberHistory(): History {
  val backPressDispatcher = checkNotNull(LocalOnBackPressedDispatcher.current) {
    "No OnBackPressedDispatcher was provided via LocalOnBackPressedDispatcher"
  }

  val browserPlatform = BrowserPlatform()

  val saveableStateRegistry = remember {
    HistorySaveableStateRegistry(
      browserPlatform = browserPlatform,
      backPressDispatcher = backPressDispatcher
    )
  }

  var history: History? = null

  CompositionLocalProvider(
    LocalSaveableStateRegistry provides saveableStateRegistry
  ) {
    history = rememberSaveable(
      saver = WebHistory.Saver(
        browserPlatform = browserPlatform,
        backPressDispatcher = backPressDispatcher,
      ),
      key = SAVE_KEY,
    ) {
      WebHistory(
        browserPlatform = browserPlatform,
        history = TimelineHistory(),
        backPressDispatcher = backPressDispatcher,
      )
    }
  }

  DisposableEffect(Unit) {
    onDispose {
      saveableStateRegistry.save()
    }
  }

  return history!!
}

from compose-multiplatform.

Not sure what would be a good default there though.

Maybe it makes sense to do it under some flag

I think it does become important with navigation, especially since the browser will remember the history stack across refresh and restoring the browser, so if something like NavController doesn't it could be problematic.

Still didn't dig too deep, but it looks like browser history should be single source of truth there and compose navigation should just handle url from history like deeplink. But it doesn't seem really related to this topic

from compose-multiplatform.

Here's my implementation for NavController (I realized after too many hours that saveState and restoreState aren't actually implemented so it doesn't work 😭 ) and my History classes, and tests validating the save and restore.

from compose-multiplatform.