Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP persist tab state #1168

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

WIP persist tab state #1168

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e428463
Move contentComposed to the end
ZacSweers Jan 31, 2024
a8544ae
WIP persist tab state
ZacSweers Jan 31, 2024
13622e9
Spotless
ZacSweers Jan 31, 2024
920299b
Merge branch 'main' into z/stateInTabs
ZacSweers Feb 2, 2024
dca28da
Add RetainedStateHolder impls
ZacSweers Feb 2, 2024
73d98bd
Add leak canary wrapper rule
ZacSweers Feb 2, 2024
21b0e15
Add RetainedStateRestorationTester
ZacSweers Feb 2, 2024
068c1a8
Spotless
ZacSweers Feb 2, 2024
70a1512
Use in HomeScreen
ZacSweers Feb 2, 2024
b2d0e16
Switch back
ZacSweers Feb 2, 2024
924ecab
Add to manifest
ZacSweers Feb 2, 2024
848ca4e
Merge branch 'main' into z/stateInTabs
ZacSweers Mar 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/CircuitContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,7 @@ public fun CircuitContent(
unavailableContent: (@Composable (screen: Screen, modifier: Modifier) -> Unit) =
circuit.onUnavailableContent,
) {
val navigator =
remember(onNavEvent) {
object : Navigator {
override fun goTo(screen: Screen) {
onNavEvent(NavEvent.GoTo(screen))
}

override fun resetRoot(newRoot: Screen): List<Screen> {
onNavEvent(NavEvent.ResetRoot(newRoot))
return emptyList()
}

override fun pop(): Screen? {
onNavEvent(NavEvent.Pop)
return null
}

override fun peek(): Screen {
return screen
}
}
}
val navigator = remember(onNavEvent) { Navigator.navEventNavigator(screen, onNavEvent) }
CircuitContent(screen, navigator, modifier, circuit, unavailableContent)
}

Expand Down
25 changes: 25 additions & 0 deletions circuit-foundation/src/commonMain/kotlin/com/slack/circuit/foundation/NavEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,31 @@ public fun Navigator.onNavEvent(event: NavEvent) {
}
}

public fun Navigator.Companion.navEventNavigator(
screen: Screen,
onNavEvent: (event: NavEvent) -> Unit,
): Navigator {
return object : Navigator {
override fun goTo(screen: Screen) {
onNavEvent(NavEvent.GoTo(screen))
}

override fun resetRoot(newRoot: Screen): List<Screen> {
onNavEvent(NavEvent.ResetRoot(newRoot))
return emptyList()
}

override fun pop(): Screen? {
onNavEvent(NavEvent.Pop)
return null
}

override fun peek(): Screen {
return screen
}
}
}

/** A sealed navigation interface intended to be used when making a navigation callback. */
public sealed interface NavEvent : CircuitUiEvent {
/** Corresponds to [Navigator.pop]. */
Expand Down
2 changes: 2 additions & 0 deletions circuit-runtime/src/commonMain/kotlin/com/slack/circuit/runtime/Navigator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public interface Navigator {

override fun resetRoot(newRoot: Screen): List<Screen> = emptyList()
}

public companion object
}

/** Calls [Navigator.pop] until the given [predicate] is matched or it pops the root. */
Expand Down
34 changes: 28 additions & 6 deletions samples/star/src/commonMain/kotlin/com/slack/circuit/star/home/HomeScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.saveable.rememberSaveableStateHolder
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import com.slack.circuit.codegen.annotations.CircuitInject
import com.slack.circuit.foundation.CircuitContent
import com.slack.circuit.foundation.LocalCircuit
import com.slack.circuit.foundation.NavEvent
import com.slack.circuit.foundation.navEventNavigator
import com.slack.circuit.foundation.onNavEvent
import com.slack.circuit.foundation.rememberPresenter
import com.slack.circuit.foundation.rememberUi
import com.slack.circuit.retained.rememberRetained
import com.slack.circuit.runtime.CircuitUiEvent
import com.slack.circuit.runtime.CircuitUiState
Expand Down Expand Up @@ -70,6 +75,7 @@ fun HomePresenter(navigator: Navigator): HomeScreen.State {
@Composable
fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) {
var contentComposed by rememberRetained { mutableStateOf(false) }

Scaffold(
modifier = modifier.fillMaxWidth(),
contentWindowInsets = WindowInsets(0, 0, 0, 0),
Expand All @@ -82,13 +88,29 @@ fun HomeContent(state: HomeScreen.State, modifier: Modifier = Modifier) {
}
},
) { paddingValues ->
val saveableStateHolder = rememberSaveableStateHolder()
val currentScreen = state.navItems[state.selectedIndex].screen
saveableStateHolder.SaveableStateProvider(currentScreen) {
val circuit = requireNotNull(LocalCircuit.current)
val ui = rememberUi(currentScreen, factory = circuit::ui)
val presenter =
rememberPresenter(
currentScreen,
navigator =
Navigator.navEventNavigator(currentScreen) { event ->
state.eventSink(ChildNav(event))
},
factory = circuit::presenter,
)

CircuitContent(
screen = currentScreen,
modifier = Modifier.padding(paddingValues),
presenter = presenter!!,
ui = ui!!,
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: loading. I think the solution here is to do something like having a map of nav items to their presenter state that is instantiated and rememberRetained above this section.

And then have a "presenter facade" which switches state to the presenter state that is currently being shown.

This is effectively what I'm doing in my project.

Copy link
Contributor

@chrisbanes chrisbanes Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because we don't retain state for root back stack changes, only back stack additions (effectively).

This has been at the back of my mind for a while (and mildly annoys me every time I use Tivi).

So this change stores the tab index across app starts, but the state of that tab won't be retained if you switch between them.

There is a comment in the code somewhere from when I wrote the retain backstack stuff, let me find it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find it now, but I think this might actually be as simple as making buildCircuitContentProviders a bit smarter, and manually retaining any dropped root records.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on this in #1173

contentComposed = true
val screen = state.navItems[state.selectedIndex].screen
CircuitContent(
screen,
modifier = Modifier.padding(paddingValues),
onNavEvent = { event -> state.eventSink(ChildNav(event)) },
)
}
Platform.ReportDrawnWhen { contentComposed }
}
Expand Down
Loading