JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.27k stars 1.18k forks source link

Use browser sessionStorage API to back rememberSaveable on web #4896

Closed eygraber closed 1 month ago

eygraber commented 5 months ago

It would be nice if rememberSaveable in the browser would use something like the browser's sessionStorage API so the state is saved across tab refreshes, browser restarts, etc...

It's questionable if those actions are viewed as explicit "close" actions by the user. The existence of sessionStorage further indicates that it is not, since it persists data across tab refreshes and browser restarts. It seems analogous to state being persisted across process restarts on Android.

MatkovIvan commented 5 months ago

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.

eygraber commented 5 months ago

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!!
}
MatkovIvan commented 5 months ago

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

eygraber commented 5 months ago

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

eygraber commented 5 months ago

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 :sob: ) and my History classes, and tests validating the save and restore.

okushnikov commented 2 months ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.