From 32228bcdf1934b1430f0eee7e703b050a7729cf9 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Wed, 5 Aug 2026 20:17:26 +0300 Subject: [PATCH] fix android sheet content width on tablets via native-owned canvas sizing on the new architecture, DialogRootViewGroup's updateNodeSize() path is a no-op, so sheet content was laid out at full window width while material caps the sheet frame at 640dp on tablets, clipping the content. the native side now owns the content canvas size and pushes it into the fabric shadow tree via ExpoView's setViewSize state channel; JS renders unsized flex:1 content on android. the canvas width follows the dialog container's measured width (insets + material's cap); the canvas height is always computed natively (screen minus status bar) and must never come from the container, whose height derives from our own content- driven detent decisions (circular - it froze keyboard and content growth). also fixes stale screen height on rotation while a sheet is open. iOS is unchanged; old arch keeps the legacy updateNodeSize path. Co-Authored-By: Claude Fable 5 --- modules/bottom-sheet/README.md | 25 ++- .../modules/bottomsheet/BottomSheetView.kt | 182 +++++++++++++++--- .../src/BottomSheetNativeComponent.tsx | 39 ++-- 3 files changed, 205 insertions(+), 41 deletions(-) diff --git a/modules/bottom-sheet/README.md b/modules/bottom-sheet/README.md index 49007d97b5..c3653a0298 100644 --- a/modules/bottom-sheet/README.md +++ b/modules/bottom-sheet/README.md @@ -61,7 +61,8 @@ The component uses a class-based approach to expose imperative methods (`present - Preserves status/nav bar appearance from host activity - **DialogRootViewGroup.kt**: Custom ViewGroup acting as RootView for the dialog - Forwards touch events to React Native event system - - Updates shadow node size to match window dimensions + - Reports its measured width to `BottomSheetView` so the content canvas can follow it + - Also carries the legacy `UIManagerModule.updateNodeSize()` shadow node sizing, which only runs on the old architecture - Based on React Native's ReactModalHostView pattern - **SheetManager.kt**: Singleton for tracking sheets (same pattern as iOS) @@ -74,6 +75,24 @@ Both platforms detect content height changes natively without JS bridge round-tr This eliminates layout jank when content changes (e.g., keyboard appearance, dynamic content loading). +### Content Canvas Sizing + +The "canvas" is the size the sheet content is laid out on by Yoga. **On Android the native side owns it**; on iOS it is still sized from JS. + +- **Android**: JS renders unsized `flex: 1` content and `BottomSheetView` pushes the canvas size into the Fabric shadow tree through `ExpoView`'s `setViewSize` state channel (`shadowNodeProxy.setViewSize()`). Only native knows the real sheet frame - Material caps the frame at 640dp on tablets and centers it, and it changes on rotation. +- **iOS**: `BottomSheetNativeComponent` sets `height: screenHeight - insets.top` and `width: '100%'` on the native view. Moving iOS onto the same state channel is deferred: it needs on-device iteration on iOS 26 sheet geometry (large-detent and floating-card metrics, where the visible sheet is shorter than the window minus the top inset). + +How the Android path works: + +- The JS style on the native view **must not set `width` or `height`** on Android. `ExpoViewComponentDescriptor::adopt()` only applies the state size on an axis where the style leaves that dimension undefined, so a style dimension would silently win. +- The two axes come from different places, and the distinction is load-bearing: + - **Width** is authoritatively the dialog container's measured width, reported through `DialogRootViewGroup`'s size-change listener - that is the real sheet width, with the horizontal window insets and Material's 640dp cap already applied. It is seeded from `min(window width, material_bottom_sheet_max_width)` on the first `onLayout` so content has something to lay out in before the dialog exists. + - **Height** is always computed natively as `screenHeight - statusBarHeight` (matching the behavior's `expandedOffset`) - the whole expanded frame, **never** the dialog's measured height. The canvas has to be room for the content to grow *into*, because the content's height is what drives the snap points. Sizing it from the dialog's own height is circular: `BottomSheetBehavior` measures the container against the sheet, so the canvas collapses onto the content height and the content is then pinned - extra `ScrollView` padding (the Android keyboard path) or a longer list becomes scroll extent instead of a height change, `OnLayoutChangeListener` never fires, and the sheet stops responding to its content. +- Seeding runs once per open cycle - re-seeding would fight the width the dialog reported and the two would push each other back and forth. +- Because the content measures 0x0 until that first state commit lands, `present()` bails out early when the content height is still zero. The commit resizes the native view, which re-fires `onLayout`, which re-enters `present()` - so presentation self-retries rather than needing an explicit callback. Full-height sheets skip the check, since they don't need a content measurement. +- Rotation is handled by the container push: the RN activity handles configuration changes itself, so the view is never recreated. `screenHeight` is read per access so the computed height follows the rotation, and the container reports the new width (plus a deferred `updateLayout()` to reposition the sheet). +- On the **old architecture** there is no state channel (`stateWrapper` is null, so `setViewSize` no-ops) and Android falls back to `DialogRootViewGroup`'s legacy `UIManagerModule.updateNodeSize()` path. The `present()` gate is skipped there for the same reason - nothing would ever resize the view. + ## Props ```typescript @@ -213,6 +232,10 @@ BottomSheetNativeComponent.dismissAll() 4. **Layout Updates During Gestures**: Content height changes are deferred during drag gestures to prevent fighting the user's input. +5. **Tablet Width**: Material caps the sheet frame at 640dp (`material_bottom_sheet_max_width`, the `android:maxWidth` on `Widget.MaterialComponents.BottomSheet`) and centers it horizontally, so on tablets the sheet is narrower than the screen. `BottomSheetView` reads that cap from resources when seeding the canvas width, and the dialog container's measured width then corrects it - see [Content Canvas Sizing](#content-canvas-sizing). + +6. **Rotation**: The RN activity handles configuration changes itself, so a rotation resizes the display without recreating `BottomSheetView`. Screen height is therefore read per access rather than cached, and `maxHeight` is stored unclamped and clamped against the current screen at use time. + ### Platform Differences - **cornerRadius**: Applied to sheet on iOS, to content wrapper on Android (Android clips with `overflow: hidden`) diff --git a/modules/bottom-sheet/android/src/main/java/expo/modules/bottomsheet/BottomSheetView.kt b/modules/bottom-sheet/android/src/main/java/expo/modules/bottomsheet/BottomSheetView.kt index 1b9224c6e7..38a1437247 100644 --- a/modules/bottom-sheet/android/src/main/java/expo/modules/bottomsheet/BottomSheetView.kt +++ b/modules/bottom-sheet/android/src/main/java/expo/modules/bottomsheet/BottomSheetView.kt @@ -21,6 +21,12 @@ import expo.modules.kotlin.AppContext import expo.modules.kotlin.viewevent.EventDispatcher import expo.modules.kotlin.views.ExpoView +/** + * Fallback for Material's `material_bottom_sheet_max_width` dimen (in dp), used only + * if the resource lookup fails. 640dp is the value Material ships. + */ +private const val FALLBACK_MAX_SHEET_WIDTH_DP = 640f + class BottomSheetView( context: Context, appContext: AppContext, @@ -38,26 +44,30 @@ class BottomSheetView( private var lastObservedContentHeight: Float = 0f private var pendingLayoutUpdate: Boolean = false - private val screenHeight: Float = - if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.VANILLA_ICE_CREAM) { - // API 35+: edge-to-edge is mandatory, heightPixels is the full display - context.resources.displayMetrics.heightPixels - .toFloat() - } else if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.R) { - // API 30-34: heightPixels may exclude nav bar, use currentWindowMetrics - val wm = context.getSystemService(Context.WINDOW_SERVICE) as android.view.WindowManager - wm.currentWindowMetrics.bounds - .height() - .toFloat() - } else { - // API < 30: currentWindowMetrics not available, use getRealSize - // which includes system bars (heightPixels may exclude them) - val wm = context.getSystemService(Context.WINDOW_SERVICE) as android.view.WindowManager - val size = android.graphics.Point() - @Suppress("DEPRECATION") - wm.defaultDisplay.getRealSize(size) - size.y.toFloat() - } + // Computed per read rather than cached at construction: the RN activity handles + // configuration changes itself, so a rotation resizes the display without + // recreating this view and a cached value would stay stale for the sheet's life. + private val screenHeight: Float + get() = + if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.VANILLA_ICE_CREAM) { + // API 35+: edge-to-edge is mandatory, heightPixels is the full display + context.resources.displayMetrics.heightPixels + .toFloat() + } else if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.R) { + // API 30-34: heightPixels may exclude nav bar, use currentWindowMetrics + val wm = context.getSystemService(Context.WINDOW_SERVICE) as android.view.WindowManager + wm.currentWindowMetrics.bounds + .height() + .toFloat() + } else { + // API < 30: currentWindowMetrics not available, use getRealSize + // which includes system bars (heightPixels may exclude them) + val wm = context.getSystemService(Context.WINDOW_SERVICE) as android.view.WindowManager + val size = android.graphics.Point() + @Suppress("DEPRECATION") + wm.defaultDisplay.getRealSize(size) + size.y.toFloat() + } private fun getNavigationBarHeight(): Int { val resourceId = resources.getIdentifier("navigation_bar_height", "dimen", "android") @@ -73,6 +83,11 @@ class BottomSheetView( private val onSnapPointChange by EventDispatcher() private val onStateChange by EventDispatcher() + // Last canvas size (in dp) pushed into the shadow tree, so repeated layout + // passes don't spam state updates + private var lastPushedCanvasWidth: Float = -1f + private var lastPushedCanvasHeight: Float = -1f + var disableDrag = false set(value) { field = value @@ -99,10 +114,11 @@ class BottomSheetView( field = if (value < 0) 0f else dpToPx(value) } - var maxHeight = this.screenHeight + // Stored unclamped (in px) because screenHeight can change under us on rotation. + // The clamp against the screen happens at use time, in getTargetHeight(). + var maxHeight = Float.MAX_VALUE set(value) { - val px = dpToPx(value) - field = if (px > this.screenHeight) this.screenHeight else px + field = dpToPx(value) } private var isOpen: Boolean = false @@ -140,6 +156,38 @@ class BottomSheetView( this.eventDispatcher = UIManagerHelper.getEventDispatcherForReactTag(it, this.id) this.dialogRootViewGroup = DialogRootViewGroup(context) this.dialogRootViewGroup.eventDispatcher = this.eventDispatcher + + // The dialog container's measured WIDTH is the authoritative canvas width: it + // already accounts for the window's horizontal insets, Material's max-width cap on + // tablets and the current rotation. DialogRootViewGroup's own updateNodeSize() path + // is a no-op on the new architecture (getNativeModule(UIManagerModule) returns null + // under Fabric), so this state channel is what actually gets the width across there. + // + // Its measured HEIGHT is deliberately ignored - see canvasHeight. + this.dialogRootViewGroup.setOnSizeChangeListener( + object : DialogRootViewGroup.OnSizeChangeListener { + override fun onSizeChange( + width: Int, + height: Int, + ) { + val density = context.resources.displayMetrics.density + pushCanvasSize(width / density, canvasHeight / density) + + // onSizeChanged fires from inside a layout pass, so defer the reposition: + // updateLayout() reads child heights that aren't final yet. This is what + // makes the sheet settle back into place after a rotation. It no-ops for + // fullHeight sheets, which is correct - those are pinned to the expanded + // offset either way. + if ((isOpen || isOpening) && !isClosing) { + post { + if ((isOpen || isOpening) && !isClosing) { + updateLayout() + } + } + } + } + }, + ) } SheetManager.add(this) } @@ -151,9 +199,84 @@ class BottomSheetView( r: Int, b: Int, ) { + this.seedCanvasSize() this.present() } + /** + * The height, in px, of the canvas the sheet content is laid out on. This is the whole + * expanded frame (the behavior's expandedOffset is the status bar height), NOT the + * sheet's current height. + * + * That distinction is the whole ballgame. The content's height is what drives the snap + * points, so the canvas has to be room to grow *into*. Sizing the canvas from the + * dialog's own measured height is circular - BottomSheetBehavior measures the dialog + * container against the sheet, so the canvas collapses onto the content height, and from + * then on the content is pinned: extra ScrollView padding (the Android keyboard path) or + * a longer list just becomes scroll extent instead of a height change, the + * OnLayoutChangeListener never fires, and the sheet stops responding to its content. + */ + private val canvasHeight: Float + get() = screenHeight - getStatusBarHeight() + + /** + * JS renders the sheet content unsized, so before the first state commit it measures + * 0x0 and present() has no content height to derive snap points from. Seed the canvas + * here to kick that off - the dialog container reports the authoritative width later, + * via its OnSizeChangeListener. + * + * Runs at most once per open cycle. It has to: each state commit re-fires onLayout, so + * re-seeding would fight the width the dialog reported and the two would push each other + * back and forth forever. + * + * stateWrapper is assigned while Fabric mounts the view, before the first layout pass, + * so setViewSize() should already reach the shadow tree from here. On the old + * architecture it is null and this no-ops, which is fine: DialogRootViewGroup's legacy + * updateNodeSize() path still sizes the content there. + */ + private fun seedCanvasSize() { + if (lastPushedCanvasWidth > 0f) return + val density = context.resources.displayMetrics.density + val widthPx = + minOf( + context.resources.displayMetrics.widthPixels + .toFloat(), + getMaxSheetWidth(), + ) + this.pushCanvasSize(widthPx / density, canvasHeight / density) + } + + /** + * Sets the size of this view's shadow node, which is the canvas the sheet content is + * laid out on. Deduped because both onLayout and the dialog container's size changes + * can re-report an unchanged size. + */ + private fun pushCanvasSize( + widthDp: Float, + heightDp: Float, + ) { + if (widthDp <= 0f || heightDp <= 0f) return + if (widthDp == lastPushedCanvasWidth && heightDp == lastPushedCanvasHeight) return + lastPushedCanvasWidth = widthDp + lastPushedCanvasHeight = heightDp + this.shadowNodeProxy.setViewSize(widthDp.toDouble(), heightDp.toDouble()) + } + + /** + * Material caps the sheet frame at `material_bottom_sheet_max_width` (the + * `android:maxWidth` on `Widget.MaterialComponents.BottomSheet`, which our dialog theme + * inherits from) and centers it horizontally, so on tablets the sheet is narrower than + * the display. Returns the cap in px. + */ + private fun getMaxSheetWidth(): Float = + try { + resources + .getDimensionPixelSize(com.google.android.material.R.dimen.material_bottom_sheet_max_width) + .toFloat() + } catch (e: android.content.res.Resources.NotFoundException) { + FALLBACK_MAX_SHEET_WIDTH_DP * context.resources.displayMetrics.density + } + private fun destroy() { this.stopObservingContentHeight() this.isClosing = false @@ -178,6 +301,15 @@ class BottomSheetView( val contentHeight = this.getContentHeight() + // The content is unsized until the canvas size we pushed lands in the shadow tree, + // so bail and let this retry itself: the state commit resizes this view, that + // re-fires onLayout, and onLayout re-enters present(). Full-height sheets don't + // need a content measurement, so they can go ahead immediately. + // + // Only gate when there is a state channel to wait on. Without one (old architecture) + // nothing would ever resize this view, and the sheet would never present. + if (stateWrapper != null && !fullHeight && contentHeight <= 0f) return + var activityWindow: Window? = null var currentContext = context while (currentContext != null) { @@ -425,8 +557,10 @@ class BottomSheetView( private fun getTargetHeight(): Float { val contentHeight = this.getContentHeight() + // maxHeight is stored unclamped, so clamp it against the current screen here + val effectiveMaxHeight = minOf(this.maxHeight, this.screenHeight) return when { - contentHeight > maxHeight -> maxHeight + contentHeight > effectiveMaxHeight -> effectiveMaxHeight contentHeight < minHeight -> minHeight else -> contentHeight } diff --git a/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx b/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx index f2955395e6..f030633667 100644 --- a/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx +++ b/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx @@ -34,10 +34,6 @@ const IS_IOS15 = Platform.OS === 'ios' && // semvar - can be 3 segments, so can't use Number(Platform.Version) Number(Platform.Version.split('.').at(0)) < 16 -// older android versions (15 and below) aren't naturally edge-to-edge -// and behave a little differently -const IS_NON_E2E_ANDROID = - Platform.OS === 'android' && Number(Platform.Version) < 35 export class BottomSheetNativeComponent extends Component< BottomSheetViewProps, @@ -148,24 +144,35 @@ function BottomSheetNativeComponentInner({ const {height: screenHeight} = useWindowDimensions() const isHeightConstrained = maxHeight != null || rest.fullHeight === true - // sigh... on older Android versions, screenHeight does not include safe area insets - // on newer Androids + iOS, it does. we need to find the inner bit + the bottom inset - // for the sheet content - const sheetHeight = IS_NON_E2E_ANDROID - ? screenHeight + insets.bottom - : screenHeight - insets.top - return (