From b2235b081aea5cfa889fdb0061fa1ad2f0514a81 Mon Sep 17 00:00:00 2001 From: Abe White Date: Thu, 22 Aug 2024 12:08:47 -0500 Subject: [PATCH] Fix possible crash using .fixed grid item layouts --- Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift | 8 ++++---- Sources/SkipUI/SkipUI/Containers/LazyHStack.swift | 8 ++++---- Sources/SkipUI/SkipUI/Containers/LazyVGrid.swift | 8 ++++---- Sources/SkipUI/SkipUI/Containers/LazyVStack.swift | 8 ++++---- Sources/SkipUI/SkipUI/Containers/ScrollView.swift | 13 ++++++++++--- .../SkipUI/Environment/EnvironmentValues.swift | 6 ++++++ 6 files changed, 32 insertions(+), 19 deletions(-) diff --git a/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift b/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift index 3d50cde1..7cb7b176 100644 --- a/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift +++ b/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift @@ -37,9 +37,8 @@ public struct LazyHGrid: View { @Composable override func ComposeContent(context: ComposeContext) { // Let any parent scroll view know about our builtin scrolling. If there is a parent scroll // view that didn't already know, abort and wait for recompose to avoid fatal nested scroll error - let builtinScrollAxisSet = PreferenceValues.shared.collector(key: BuiltinScrollAxisSetPreferenceKey.self)?.state.value.reduced as? Axis.Set PreferenceValues.shared.contribute(context: context, key: BuiltinScrollAxisSetPreferenceKey.self, value: Axis.Set.horizontal) - guard builtinScrollAxisSet?.contains(Axis.Set.horizontal) != false else { + guard !EnvironmentValues.shared._scrollAxes.contains(Axis.Set.horizontal) else { return } @@ -47,7 +46,8 @@ public struct LazyHGrid: View { let boxAlignment = cellAlignment?.asComposeAlignment() ?? androidx.compose.ui.Alignment.Center let verticalArrangement = Arrangement.spacedBy((verticalSpacing ?? 8.0).dp, alignment: alignment.asComposeAlignment()) let horizontalArrangement = Arrangement.spacedBy((spacing ?? 8.0).dp) - let isScrollEnabled = EnvironmentValues.shared._scrollAxes.contains(.horizontal) + let isScrollEnabled = EnvironmentValues.shared._scrollViewAxes.contains(.horizontal) + let scrollAxes: Axis.Set = isScrollEnabled ? Axis.Set.horizontal : [] // Collect all top-level views to compose. The LazyHorizontalGrid itself is not a composable context, so we have to execute // our content's Compose function to collect its views before entering the LazyHorizontalGrid body, then use LazyHorizontalGrid's @@ -58,7 +58,7 @@ public struct LazyHGrid: View { let itemContext = context.content() let factoryContext = remember { mutableStateOf(LazyItemFactoryContext()) } - ComposeContainer(axis: .vertical, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in + ComposeContainer(axis: .vertical, scrollAxes: scrollAxes, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in // Integrate with our scroll-to-top and ScrollViewReader let gridState = rememberLazyGridState() let coroutineScope = rememberCoroutineScope() diff --git a/Sources/SkipUI/SkipUI/Containers/LazyHStack.swift b/Sources/SkipUI/SkipUI/Containers/LazyHStack.swift index 63dbbf28..bac51f68 100644 --- a/Sources/SkipUI/SkipUI/Containers/LazyHStack.swift +++ b/Sources/SkipUI/SkipUI/Containers/LazyHStack.swift @@ -33,15 +33,15 @@ public struct LazyHStack : View { @Composable override func ComposeContent(context: ComposeContext) { // Let any parent scroll view know about our builtin scrolling. If there is a parent scroll // view that didn't already know, abort and wait for recompose to avoid fatal nested scroll error - let builtinScrollAxisSet = PreferenceValues.shared.collector(key: BuiltinScrollAxisSetPreferenceKey.self)?.state.value.reduced as? Axis.Set PreferenceValues.shared.contribute(context: context, key: BuiltinScrollAxisSetPreferenceKey.self, value: Axis.Set.horizontal) - guard builtinScrollAxisSet?.contains(Axis.Set.horizontal) != false else { + guard !EnvironmentValues.shared._scrollAxes.contains(Axis.Set.horizontal) else { return } let rowAlignment = alignment.asComposeAlignment() let rowArrangement = Arrangement.spacedBy((spacing ?? 8.0).dp, alignment: androidx.compose.ui.Alignment.CenterHorizontally) - let isScrollEnabled = EnvironmentValues.shared._scrollAxes.contains(.horizontal) + let isScrollEnabled = EnvironmentValues.shared._scrollViewAxes.contains(.horizontal) + let scrollAxes: Axis.Set = isScrollEnabled ? Axis.Set.horizontal : [] // Collect all top-level views to compose. The LazyRow itself is not a composable context, so we have to execute // our content's Compose function to collect its views before entering the LazyRow body, then use LazyRow's @@ -52,7 +52,7 @@ public struct LazyHStack : View { let itemContext = context.content() let factoryContext = remember { mutableStateOf(LazyItemFactoryContext()) } - ComposeContainer(axis: .horizontal, modifier: context.modifier, fillWidth: true, fillHeight: false) { modifier in + ComposeContainer(axis: .horizontal, scrollAxes: scrollAxes, modifier: context.modifier, fillWidth: true, fillHeight: false) { modifier in // Integrate with ScrollViewReader let listState = rememberLazyListState() let coroutineScope = rememberCoroutineScope() diff --git a/Sources/SkipUI/SkipUI/Containers/LazyVGrid.swift b/Sources/SkipUI/SkipUI/Containers/LazyVGrid.swift index 1354726e..860c7119 100644 --- a/Sources/SkipUI/SkipUI/Containers/LazyVGrid.swift +++ b/Sources/SkipUI/SkipUI/Containers/LazyVGrid.swift @@ -40,9 +40,8 @@ public struct LazyVGrid: View { @Composable override func ComposeContent(context: ComposeContext) { // Let any parent scroll view know about our builtin scrolling. If there is a parent scroll // view that didn't already know, abort and wait for recompose to avoid fatal nested scroll error - let builtinScrollAxisSet = PreferenceValues.shared.collector(key: BuiltinScrollAxisSetPreferenceKey.self)?.state.value.reduced as? Axis.Set PreferenceValues.shared.contribute(context: context, key: BuiltinScrollAxisSetPreferenceKey.self, value: Axis.Set.vertical) - guard builtinScrollAxisSet?.contains(Axis.Set.vertical) != false else { + guard !EnvironmentValues.shared._scrollAxes.contains(Axis.Set.vertical) else { return } @@ -50,7 +49,8 @@ public struct LazyVGrid: View { let boxAlignment = cellAlignment?.asComposeAlignment() ?? androidx.compose.ui.Alignment.Center let horizontalArrangement = Arrangement.spacedBy((horizontalSpacing ?? 8.0).dp, alignment: alignment.asComposeAlignment()) let verticalArrangement = Arrangement.spacedBy((spacing ?? 8.0).dp) - let isScrollEnabled = EnvironmentValues.shared._scrollAxes.contains(.vertical) + let isScrollEnabled = EnvironmentValues.shared._scrollViewAxes.contains(.vertical) + let scrollAxes: Axis.Set = isScrollEnabled ? Axis.Set.vertical : [] // Collect all top-level views to compose. The LazyVerticalGrid itself is not a composable context, so we have to execute // our content's Compose function to collect its views before entering the LazyVerticalGrid body, then use LazyVerticalGrid's @@ -64,7 +64,7 @@ public struct LazyVGrid: View { let itemContext = context.content() let factoryContext = remember { mutableStateOf(LazyItemFactoryContext()) } - ComposeContainer(axis: .vertical, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in + ComposeContainer(axis: .vertical, scrollAxes: scrollAxes, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in // Integrate with our scroll-to-top and ScrollViewReader let gridState = rememberLazyGridState(initialFirstVisibleItemIndex = isSearchable ? 1 : 0) let coroutineScope = rememberCoroutineScope() diff --git a/Sources/SkipUI/SkipUI/Containers/LazyVStack.swift b/Sources/SkipUI/SkipUI/Containers/LazyVStack.swift index 7dcc0bb6..087f0d1d 100644 --- a/Sources/SkipUI/SkipUI/Containers/LazyVStack.swift +++ b/Sources/SkipUI/SkipUI/Containers/LazyVStack.swift @@ -37,15 +37,15 @@ public struct LazyVStack : View { @Composable override func ComposeContent(context: ComposeContext) { // Let any parent scroll view know about our builtin scrolling. If there is a parent scroll // view that didn't already know, abort and wait for recompose to avoid fatal nested scroll error - let builtinScrollAxisSet = PreferenceValues.shared.collector(key: BuiltinScrollAxisSetPreferenceKey.self)?.state.value.reduced as? Axis.Set PreferenceValues.shared.contribute(context: context, key: BuiltinScrollAxisSetPreferenceKey.self, value: Axis.Set.vertical) - guard builtinScrollAxisSet?.contains(Axis.Set.vertical) != false else { + guard !EnvironmentValues.shared._scrollAxes.contains(Axis.Set.vertical) else { return } let columnAlignment = alignment.asComposeAlignment() let columnArrangement = Arrangement.spacedBy((spacing ?? 8.0).dp, alignment: androidx.compose.ui.Alignment.CenterVertically) - let isScrollEnabled = EnvironmentValues.shared._scrollAxes.contains(.vertical) + let isScrollEnabled = EnvironmentValues.shared._scrollViewAxes.contains(.vertical) + let scrollAxes: Axis.Set = isScrollEnabled ? Axis.Set.vertical : [] // Collect all top-level views to compose. The LazyColumn itself is not a composable context, so we have to execute // our content's Compose function to collect its views before entering the LazyColumn body, then use LazyColumn's @@ -59,7 +59,7 @@ public struct LazyVStack : View { let itemContext = context.content() let factoryContext = remember { mutableStateOf(LazyItemFactoryContext()) } - ComposeContainer(axis: .vertical, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in + ComposeContainer(axis: .vertical, scrollAxes: scrollAxes, modifier: context.modifier, fillWidth: true, fillHeight: true) { modifier in // Integrate with our scroll-to-top and ScrollViewReader let listState = rememberLazyListState(initialFirstVisibleItemIndex = isSearchable ? 1 : 0) let coroutineScope = rememberCoroutineScope() diff --git a/Sources/SkipUI/SkipUI/Containers/ScrollView.swift b/Sources/SkipUI/SkipUI/Containers/ScrollView.swift index 8f8e5e4b..c7a3da57 100644 --- a/Sources/SkipUI/SkipUI/Containers/ScrollView.swift +++ b/Sources/SkipUI/SkipUI/Containers/ScrollView.swift @@ -52,8 +52,10 @@ public struct ScrollView : View { let isVerticalScroll = axes.contains(.vertical) && !builtinScrollAxisSet.value.reduced.contains(Axis.Set.vertical) let isHorizontalScroll = axes.contains(.horizontal) && !builtinScrollAxisSet.value.reduced.contains(Axis.Set.horizontal) var scrollModifier: Modifier = Modifier + var effectiveScrollAxes: Axis.Set = [] if isVerticalScroll { scrollModifier = scrollModifier.verticalScroll(scrollState) + effectiveScrollAxes.insert(Axis.Set.vertical) if !axes.contains(.horizontal) { // Integrate with our scroll-to-top navigation bar taps PreferenceValues.shared.contribute(context: context, key: ScrollToTopPreferenceKey.self, value: { @@ -65,9 +67,10 @@ public struct ScrollView : View { } if isHorizontalScroll { scrollModifier = scrollModifier.horizontalScroll(scrollState) + effectiveScrollAxes.insert(Axis.Set.horizontal) } let contentContext = context.content() - ComposeContainer(scrollAxes: axes, modifier: context.modifier, fillWidth: axes.contains(.horizontal), fillHeight: axes.contains(.vertical), then: scrollModifier) { modifier in + ComposeContainer(scrollAxes: effectiveScrollAxes, modifier: context.modifier, fillWidth: axes.contains(.horizontal), fillHeight: axes.contains(.vertical), then: scrollModifier) { modifier in let containerModifier: Modifier let refreshing = remember { mutableStateOf(false) } let refreshAction = EnvironmentValues.shared.refresh @@ -96,8 +99,12 @@ public struct ScrollView : View { SearchField(state: searchableState, context: context.content(modifier: Modifier.padding(horizontal: 16.dp, vertical: 8.dp))) } } - PreferenceValues.shared.collectPreferences([builtinScrollAxisSetCollector]) { - content.Compose(context: contentContext) + EnvironmentValues.shared.setValues { + $0.set_scrollViewAxes(axes) + } in: { + PreferenceValues.shared.collectPreferences([builtinScrollAxisSetCollector]) { + content.Compose(context: contentContext) + } } } if let refreshState { diff --git a/Sources/SkipUI/SkipUI/Environment/EnvironmentValues.swift b/Sources/SkipUI/SkipUI/Environment/EnvironmentValues.swift index 32e2d2cc..909c1b48 100644 --- a/Sources/SkipUI/SkipUI/Environment/EnvironmentValues.swift +++ b/Sources/SkipUI/SkipUI/Environment/EnvironmentValues.swift @@ -515,6 +515,12 @@ extension EnvironmentValues { set { setBuiltinValue(key: "_scrollContentBackground", value: newValue, defaultValue: { nil }) } } + /// While `_scrollAxes` contains the effective scroll directions, this property contains the nominal directions of any ancestor scroll view. + var _scrollViewAxes: Axis.Set { + get { builtinValue(key: "_scrollViewAxes", defaultValue: { Axis.Set(rawValue: 0) }) as! Axis.Set } + set { setBuiltinValue(key: "_scrollViewAxes", value: newValue, defaultValue: { Axis.Set(rawValue: 0) }) } + } + var _searchableState: SearchableState? { get { builtinValue(key: "_searchableState", defaultValue: { nil }) as! SearchableState? } set { setBuiltinValue(key: "_searchableState", value: newValue, defaultValue: { nil }) }