Skip to content

Revert "Fix crash on state reset when preferences are queried" #441

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

Open
dfabulich wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Revert "Fix crash on state reset when preferences are queried" #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 24 additions & 12 deletions Sources/SkipUI/SkipUI/Containers/Navigation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,14 @@ public struct NavigationStack : View, Renderable {

#if SKIP
@Composable public override func Render(context: ComposeContext) {
// Have to use rememberSaveable for e.g. a nav stack in each tab. Make the collectors non-erasable so that
// destinations defined at e.g. the root nav stack layer don't disappear when you push.
let (destinations, destinationsCollector) = rememberSaveablePreferenceCollector(key: NavigationDestinationsPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<NavigationDestinations>, Any>, isErasable: false)
let (destinationLayoutHints, destinationLayoutHintsCollector) = rememberSaveablePreferenceCollector(key: NavigationDestinationLayoutHintsPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<NavigationDestinationLayoutHintsMap>, Any>, isErasable: false)
// Have to use rememberSaveable for e.g. a nav stack in each tab
let destinations = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<NavigationDestinations>, Any>) { mutableStateOf(Preference<NavigationDestinations>(key: NavigationDestinationsPreferenceKey.self))
}
// Make this collector non-erasable so that destinations defined at e.g. the root nav stack layer don't disappear when you push
let destinationsCollector = PreferenceCollector<NavigationDestinations>(key: NavigationDestinationsPreferenceKey.self, state: destinations, isErasable: false)
let destinationLayoutHints = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<NavigationDestinationLayoutHintsMap>, Any>) { mutableStateOf(Preference<NavigationDestinationLayoutHintsMap>(key: NavigationDestinationLayoutHintsPreferenceKey.self))
}
let destinationLayoutHintsCollector = PreferenceCollector<NavigationDestinationLayoutHintsMap>(key: NavigationDestinationLayoutHintsPreferenceKey.self, state: destinationLayoutHints, isErasable: false)
let reducedDestinations = destinations.value.reduced
let reducedDestinationLayoutHints = destinationLayoutHints.value.reduced
let mergedDestinations = mergeNavigationDestinationsWithLayoutHints(reducedDestinations, layoutHints: reducedDestinationLayoutHints)
Expand All @@ -165,9 +169,12 @@ public struct NavigationStack : View, Renderable {
}
// These preferences are per-entry, but if we put them in RenderEntry then their initial values don't show
// during the navigation animation. We have to collect them here
let (title, titleCollector) = rememberSaveablePreferenceCollector(key: NavigationTitlePreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<Text>, Any>)
let (toolbarPreferences, toolbarPreferencesCollector) = rememberSaveablePreferenceCollector(key: ToolbarPreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<ToolbarPreferences>, Any>)
let (toolbarContentPreferences, toolbarContentPreferencesCollector) = rememberSaveablePreferenceCollector(key: ToolbarContentPreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<ToolbarContentPreferences>, Any>)
let title = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<Text>, Any>) { mutableStateOf(Preference<Text>(key: NavigationTitlePreferenceKey.self)) }
let titleCollector = PreferenceCollector<Text>(key: NavigationTitlePreferenceKey.self, state: title)
let toolbarPreferences = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<ToolbarPreferences>, Any>) { mutableStateOf(Preference<ToolbarPreferences>(key: ToolbarPreferenceKey.self)) }
let toolbarPreferencesCollector = PreferenceCollector<ToolbarPreferences>(key: ToolbarPreferenceKey.self, state: toolbarPreferences)
let toolbarContentPreferences = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<ToolbarContentPreferences>, Any>) { mutableStateOf(Preference<ToolbarContentPreferences>(key: ToolbarContentPreferenceKey.self)) }
let toolbarContentPreferencesCollector = PreferenceCollector<ToolbarContentPreferences>(key: ToolbarContentPreferenceKey.self, state: toolbarContentPreferences)
let arguments = NavigationEntryArguments(isRoot: true, state: state, safeArea: safeArea, ignoresSafeAreaEdges: ignoresSafeAreaEdges, title: title.value.reduced, toolbarPreferences: toolbarPreferences.value.reduced)
PreferenceValues.shared.collectPreferences([titleCollector, toolbarPreferencesCollector, toolbarContentPreferencesCollector, destinationsCollector, destinationLayoutHintsCollector]) {
RenderEntry(navigator: navigator, toolbarContent: toolbarContentPreferences, arguments: arguments, context: context) { context in
Expand All @@ -181,9 +188,12 @@ public struct NavigationStack : View, Renderable {
}
// These preferences are per-entry, but if we put them in RenderEntry then their initial values don't show
// during the navigation animation. We have to collect them here
let (title, titleCollector) = rememberSaveablePreferenceCollector(key: NavigationTitlePreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<Text>, Any>)
let (toolbarPreferences, toolbarPreferencesCollector) = rememberSaveablePreferenceCollector(key: ToolbarPreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<ToolbarPreferences>, Any>)
let (toolbarContentPreferences, toolbarContentPreferencesCollector) = rememberSaveablePreferenceCollector(key: ToolbarContentPreferenceKey.self, stateSaver: state.stateSaver as! Saver<Preference<ToolbarContentPreferences>, Any>)
let title = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<Text>, Any>) { mutableStateOf(Preference<Text>(key: NavigationTitlePreferenceKey.self)) }
let titleCollector = PreferenceCollector<Text>(key: NavigationTitlePreferenceKey.self, state: title)
let toolbarPreferences = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<ToolbarPreferences>, Any>) { mutableStateOf(Preference<ToolbarPreferences>(key: ToolbarPreferenceKey.self)) }
let toolbarPreferencesCollector = PreferenceCollector<ToolbarPreferences>(key: ToolbarPreferenceKey.self, state: toolbarPreferences)
let toolbarContentPreferences = rememberSaveable(stateSaver: state.stateSaver as! Saver<Preference<ToolbarContentPreferences>, Any>) { mutableStateOf(Preference<ToolbarContentPreferences>(key: ToolbarContentPreferenceKey.self)) }
let toolbarContentPreferencesCollector = PreferenceCollector<ToolbarContentPreferences>(key: ToolbarContentPreferenceKey.self, state: toolbarContentPreferences)
EnvironmentValues.shared.setValues {
$0.setdismiss(DismissAction(action: { navigator.value.navigateBack() }))
return ComposeResult.ok
Expand Down Expand Up @@ -267,9 +277,11 @@ public struct NavigationStack : View, Renderable {
let searchFieldOffsetPx = rememberSaveable(stateSaver: context.stateSaver as! Saver<Float, Any>) { mutableStateOf(Float(0.0)) }
let searchFieldScrollConnection = remember { SearchFieldScrollConnection(heightPx: searchFieldHeightPx, offsetPx: searchFieldOffsetPx) }

let (searchableStatePreference, searchableStateCollector) = rememberSaveablePreferenceCollector(key: SearchableStatePreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<SearchableState?>, Any>)
let searchableStatePreference = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<SearchableState?>, Any>) { mutableStateOf(Preference<SearchableState?>(key: SearchableStatePreferenceKey.self)) }
let searchableStateCollector = PreferenceCollector<SearchableState?>(key: SearchableStatePreferenceKey.self, state: searchableStatePreference)

let (scrollToTop, scrollToTopCollector) = rememberSaveablePreferenceCollector(key: ScrollToTopPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<ScrollToTopAction>, Any>)
let scrollToTop = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<ScrollToTopAction>, Any>) { mutableStateOf(Preference<ScrollToTopAction>(key: ScrollToTopPreferenceKey.self)) }
let scrollToTopCollector = PreferenceCollector<ScrollToTopAction>(key: ScrollToTopPreferenceKey.self, state: scrollToTop)

let initialScrollBehavior = isInlineTitleDisplayMode ? TopAppBarDefaults.pinnedScrollBehavior() : TopAppBarDefaults.exitUntilCollapsedScrollBehavior()
// Determine the final scrollBehavior early by checking if the environment value would modify it
Expand Down
3 changes: 2 additions & 1 deletion Sources/SkipUI/SkipUI/Containers/PresentationRoot.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import androidx.compose.ui.platform.LocalLayoutDirection
@Composable public func PresentationRoot(defaultColorScheme: ColorScheme? = nil, absoluteSystemBarEdges systemBarEdges: Edge.Set = .all, context: ComposeContext, content: @Composable (ComposeContext) -> Void) {
launchUIApplicationActivity()

let (preferredColorScheme, preferredColorSchemeCollector) = rememberSaveablePreferenceCollector(key: PreferredColorSchemePreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<PreferredColorScheme>, Any>)
let preferredColorScheme = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<PreferredColorScheme>, Any>) { mutableStateOf(Preference<PreferredColorScheme>(key: PreferredColorSchemePreferenceKey.self)) }
let preferredColorSchemeCollector = PreferenceCollector<PreferredColorScheme>(key: PreferredColorSchemePreferenceKey.self, state: preferredColorScheme)
PreferenceValues.shared.collectPreferences([preferredColorSchemeCollector]) {
let materialColorScheme = preferredColorScheme.value.reduced.colorScheme?.asMaterialTheme() ?? defaultColorScheme?.asMaterialTheme() ?? MaterialTheme.colorScheme
MaterialTheme(colorScheme: materialColorScheme) {
Expand Down
3 changes: 2 additions & 1 deletion Sources/SkipUI/SkipUI/Containers/ScrollView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public struct ScrollView : View, Renderable {
// SKIP INSERT: @OptIn(ExperimentalMaterialApi::class)
@Composable override func Render(context: ComposeContext) {
// Some components in Compose have their own scrolling built in
let (builtinScrollAxisSet, builtinScrollAxisSetCollector) = rememberSaveablePreferenceCollector(key: BuiltinScrollAxisSetPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<Axis.Set>, Any>)
let builtinScrollAxisSet = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<Axis.Set>, Any>) { mutableStateOf(Preference<Axis.Set>(key: BuiltinScrollAxisSetPreferenceKey.self)) }
let builtinScrollAxisSetCollector = PreferenceCollector<Axis.Set>(key: BuiltinScrollAxisSetPreferenceKey.self, state: builtinScrollAxisSet)

let scrollState = rememberScrollState()
let coroutineScope = rememberCoroutineScope()
Expand Down
3 changes: 2 additions & 1 deletion Sources/SkipUI/SkipUI/Containers/TabView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ public struct TabView : View, Renderable {
// Isolate access to current route within child Composable so route nav does not force us to recompose
navigateToCurrentRoute(tabBackStacks: tabBackStacks, selectedTabIndex: selectedTabIndex, tabRenderables: tabRenderables)

let (tabBarPreferences, tabBarPreferencesCollector) = rememberSaveablePreferenceCollector(key: TabBarPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<ToolbarBarPreferences>, Any>)
let tabBarPreferences = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<ToolbarBarPreferences>, Any>) { mutableStateOf(Preference<ToolbarBarPreferences>(key: TabBarPreferenceKey.self)) }
let tabBarPreferencesCollector = PreferenceCollector<ToolbarBarPreferences>(key: TabBarPreferenceKey.self, state: tabBarPreferences)

let safeArea = EnvironmentValues.shared._safeArea
/// Latest TabView-scope safe area; use inside long-lived nav entry closures so inset updates (e.g. status bar hide) propagate without relying on lexical capture of `safeArea`.
Expand Down
28 changes: 2 additions & 26 deletions Sources/SkipUI/SkipUI/Environment/PreferenceKey.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,30 +166,6 @@ struct PreferenceCollector<Value> {
}
}

/// Wraps `rememberSaveable` for a `Preference<V>` with defensive null handling.
///
/// After certain Android configuration changes (e.g. a system font scale change) the activity is recreated and the
/// in-memory map backing our `ComposeStateSaver` is lost, so saved keys no longer resolve to their values and the
/// restored `MutableState` ends up holding null. Reading `.reduced` on a null `Preference` then crashes. Reset to
/// the value produced by `initial` whenever the state value is null. See https://github.com/skiptools/skip-ui/issues/300.
@Composable func rememberSaveablePreference<V>(stateSaver: Saver<Preference<V>, Any>, initial: () -> Preference<V>) -> MutableState<Preference<V>> {
let state = rememberSaveable(stateSaver: stateSaver) { mutableStateOf(initial()) }
if (state.value as Any?) == nil {
state.value = initial()
}
return state
}

/// Combines `rememberSaveablePreference` with the matching `PreferenceCollector` so callers don't have to repeat
/// the value type or the key. The generic `V` is supplied once on the `stateSaver` cast and inferred elsewhere.
/// Pass `collectorKey` for cases where producers contribute under a different key than the `PreferenceKey` companion
/// (e.g. when the producer keys on the value type itself rather than the `PreferenceKey` type).
@Composable func rememberSaveablePreferenceCollector<V>(key: Any.Type, stateSaver: Saver<Preference<V>, Any>, collectorKey: Any? = nil, isErasable: Bool = true) -> (state: MutableState<Preference<V>>, collector: PreferenceCollector<V>) {
let state = rememberSaveablePreference(stateSaver: stateSaver) { Preference<V>(key: key) }
let collector = PreferenceCollector<V>(key: collectorKey ?? key, state: state, isErasable: isErasable)
return (state: state, collector: collector)
}

struct PreferenceNode<Value>: Equatable {
let id: Int
let value: Value
Expand Down Expand Up @@ -242,8 +218,8 @@ extension View {
public func onPreferenceChange(key: Any, defaultValue: Any?, reducer: @escaping (Any?, Any?) -> Any?, action: @escaping (Any?) -> Void) -> any View {
#if SKIP
return ModifiedContent(content: self, modifier: RenderModifier { renderable, context in
let preference = rememberSaveablePreference(stateSaver: context.stateSaver as! Saver<Preference<Any?>, Any>) {
Preference<Any?>(key: key, initialValue: defaultValue, reducer: reducer)
let preference = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<Any?>, Any>) {
mutableStateOf(Preference<Any?>(key: key, initialValue: defaultValue, reducer: reducer))
}
let preferenceCollector = PreferenceCollector<Any?>(key: key, state: preference)
let currentAction = rememberUpdatedState(action)
Expand Down
7 changes: 4 additions & 3 deletions Sources/SkipUI/SkipUI/Layout/Presentation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ private let AlertDialogMaxWidth: Dp = 560.dp

// SKIP INSERT: @OptIn(ExperimentalMaterial3Api::class)
@Composable func SheetPresentation(isPresented: Binding<Bool>, isFullScreen: Bool, context: ComposeContext, content: () -> any View, onDismiss: (() -> Void)?) {
let (interactiveDismissDisabledPreference, interactiveDismissDisabledCollector) = rememberSaveablePreferenceCollector(key: InteractiveDismissDisabledPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<Bool>, Any>)
let interactiveDismissDisabledPreference = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<Bool>, Any>) { mutableStateOf(Preference<Bool>(key: InteractiveDismissDisabledPreferenceKey.self)) }
let interactiveDismissDisabledCollector = PreferenceCollector<Bool>(key: InteractiveDismissDisabledPreferenceKey.self, state: interactiveDismissDisabledPreference)

let sheetState = rememberModalBottomSheetState(skipPartiallyExpanded: true)
let isPresentedValue = isPresented.get()
Expand Down Expand Up @@ -129,8 +130,8 @@ private let AlertDialogMaxWidth: Dp = 560.dp
let sheetDepth = EnvironmentValues.shared._sheetDepth
var systemBarEdges: Edge.Set = isFullScreen ? .all : [.top, .bottom]

// Producers contribute under the value type rather than the `PreferenceKey` type, so override `collectorKey`.
let (detentPreferences, detentPreferencesCollector) = rememberSaveablePreferenceCollector(key: PresentationDetentPreferenceKey.self, stateSaver: context.stateSaver as! Saver<Preference<PresentationDetentPreferences>, Any>, collectorKey: PresentationDetentPreferences.self)
let detentPreferences = rememberSaveable(stateSaver: context.stateSaver as! Saver<Preference<PresentationDetentPreferences>, Any>) { mutableStateOf(Preference<PresentationDetentPreferences>(key: PresentationDetentPreferenceKey.self)) }
let detentPreferencesCollector = PreferenceCollector<PresentationDetentPreferences>(key: PresentationDetentPreferences.self, state: detentPreferences)
let reducedDetentPreferences = detentPreferences.value.reduced

if !isFullScreen && verticalSizeClass != .compact {
Expand Down