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.

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

WIP RetainedStateHolder #1168

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
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
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
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
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

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