Fix possible crash using .fixed grid item layouts

skiptools · Aug 22, 2024 · b2235b0 · b2235b0
1 parent a533180
commit b2235b0
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 19 deletions.
diff --git a/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift b/Sources/SkipUI/SkipUI/Containers/LazyHGrid.swift
@@ -37,17 +37,17 @@ 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
         }
 
         let (gridCells, cellAlignment, verticalSpacing) = GridItem.asGridCells(items: rows)
         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
@@ -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
@@ -40,17 +40,17 @@ 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
         }
 
         let (gridCells, cellAlignment, horizontalSpacing) = GridItem.asGridCells(items: columns)
         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
@@ -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
@@ -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
@@ -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 }) }