LeOS-Ice-browser/android-components/docs/rfcs/0010-add-state-based-naviga...

7.9 KiB

layout title permalink
page Add State-based navigation /rfc/0010-add-state-based-navigation
  • Start date: 2023-10-17
  • RFC PR: 4126

Summary

Following the acceptance of 0009 - Remove Interactors and Controllers, Fenix should have a method of navigation that is tied to the lib-state model to provide a method of handling navigation side-effects that is consistent with architectural goals. This architecture is defined at a high-level here and has example code in this folder.

Motivation

Currently, methods of navigation throughout the app are varied. The SessionControlController provides 3 examples alone:

  • HomeActivity::openToBrowserAndLoad
  • Calling a NavController directly
  • Callbacks like showTabTray()

To move to a more consistent Redux-like model, we need a way for features to fire Actions and have that result in navigation. This will help decouple our business logic from the Android platform, where a key example of this would be references to the HomeActivity throughout the app in order to access the openToBrowserAndLoad function.

Proposal

Moving forward, navigation should be initiated through middlewares that respond to Actions. How these middleware handle navigation side-effects can be addressed on a per-case basis, but this proposal includes some generalized advice for 3 common use cases.

1. Screen-based navigation

For screen-based navigation between screens like the settings pages or navigation to the home screen, middlewares should make direct use of a NavController that is hosted by the fragment of the current screen's scope.

For a hypothetical example:


sealed class HistoryAction {
    object HomeButtonClicked : HistoryAction()
    data class HistoryGroupClicked(val group: History.Group) : HistoryAction()
}

class HistoryNavigationMiddleware(private val getNavController: () -> NavController) : Middleware<HistoryState, HistoryAction> {
    override fun invoke(
        context: MiddlewareContext<HistoryState, HistoryAction>,
        next: (HistoryFragmentAction) -> Unit,
        action: HistoryFragmentAction,
    ) {
        next(action)
        when (action) {
            is HomeButtonClicked -> getNavController().navigate(R.id.home_fragment)
            is HistoryGroupClicked -> getNavController().navigate(R.id.history_metadata_fragment)
        }
    }
}

This should translate fairly easily to the Compose world. This example intentionally ignores passing the group through the navigation transition. It should be fairly trivial to convert data types to navigation arguments, or consider creating Stores with a scope large enough to maintain state across these transitions.

Note also the use of a lambda to retrieve the NavController. This should help avoid stale references when Stores outlive their parent fragment by using a StoreProvider.

2. Transient effects

Transient effects can be handled by callbacks provided to a middleware. To build on our previous example:

sealed class HistoryAction {
    object HomeButtonClicked : HistoryAction()
    data class HistoryGroupClicked(val group: HistoryItem.Group) : HistoryAction()
    data class HistoryItemLongClicked(val item: HistoryItem) : HistoryAction()
}

class HistoryUiEffectMiddleware(
    private val displayMenuForItem: (HistoryItem) -> Unit,
) : Middleware<HistoryState, HistoryAction> {
    override fun invoke(
        context: MiddlewareContext<HistoryState, HistoryAction>,
        next: (HistoryFragmentAction) -> Unit,
        action: HistoryFragmentAction,
    ) {
        next(action)
        when (action) {
            is HistoryItemLongClicked -> displayMenuForItem(action.item)
            is HomeButtonClicked, HistoryGroupClicked -> Unit
        }
    }
}

3. The special case of openToBrowserAndLoad

Finally, we want a generally re-usable method of opening a new tab and navigating to the BrowserFragment. Fragment-based Stores can re-use a (theoretical) delegate to do so.

sealed class HistoryAction {
    object HomeButtonClicked : HistoryAction()
    data class HistoryGroupClicked(val group: History.Group) : HistoryAction()
    data class HistoryItemLongClicked(val item: HistoryItem) : HistoryAction()
    data class HistoryItemClicked(val item: History.Item) : HistoryAction()
}

class HistoryNavigationMiddleware(
    private val browserNavigator: BrowserNavigator,
    private val getNavController: () -> NavController,
) : Middleware<HistoryState, HistoryAction> {
    override fun invoke(
        context: MiddlewareContext<HistoryState, HistoryAction>,
        next: (HistoryFragmentAction) -> Unit,
        action: HistoryFragmentAction,
    ) {
        next(action)
        when (action) {
            is HomeButtonClicked -> navController.navigate(R.id.home_fragment)
            is HistoryGroupClicked -> navController.navigate(R.id.history_metadata_fragment)
            is HistoryItemClicked -> browserNavigator.openToBrowserAndLoad(action.item)
            is HistoryItemLongClicked -> Unit
        }
    }
}

This delegate would wrap the current behavior exposed by HomeActivity::openToBrowserAndLoad, looking something roughly like:

class BrowserNavigator(
    private val addTabUseCase: AddNewTabUseCase,
    private val loadTabUseCase: DefaultLoadUrlUseCase,
    private val searchUseCases: SearchUseCases,
    private val navController: () -> NavController,
) {
    // logic to navigate to browser fragment and load a tab
}

Alternatives

1. Observing navigation State from a AppStore through a binding in HomeActivity.

This was the previous proposal for this RFC. An example would roughly be:

sealed class AppAction {
    object NavigateHome : AppAction()
}

data class AppState(
    val currentScreen: Screen
)

fun appReducer(state: AppState, action: AppAction): AppState = when (action) {
    is NavigateHome -> state.copy(currentScreen = Screen.Home)
}

// in HomeActivity
private val navigationObserver by lazy {
    object : AbstractBinding<AppState>(components.appStore) {
        override suspend fun onState(flow: Flow<AppState>) = flow
            .distinctUntilChangedBy { it.screen }
            .collectLatest { /* handleNavigation */ }
    }
}

However, this implies some several issues:

  1. We end up replicating the state of a NavController manually in a our custom State, risking out-of-sync issues.
  2. We lose specificity of Actions by generalizing them globally. For example, instead of a ToolbarAction.HomeClicked, it would encourage re-use of a single AppAction.NavigateHome. Though seemingly convenient at first, it implies downstream problems for things like telemetry. To know where the navigation to home originated from, we would need to include additional properties (like direction) in the Action. Any future changes to the behavior of these Actions would need to be generalized for the whole app.

2. Global navigation middleware attached to the AppStore.

This carries risk of the 2 issue listed above, and runs into immediate technical constraints. When the AppStore is constructed in Core, we do not have reference to an Activity and cannot retrieve a NavController. This could be mitigated by a mutable property or lazy getter that is set as Fragments or the Activity come into and out of scope. The current proposal will localize navigation transitions to feature areas which should keep them isolated in scope.