From e46977ddcac215e621fbb36e1158b19b71c45ef0 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Sun, 1 Mar 2026 19:48:31 +0200 Subject: [PATCH] fix android bottom sheet height observation and content clipping - getContentHeight: use max of children heights instead of DFS index 1 (old approach measured the drag handle, not the content) - observe height via OnLayoutChangeListener on each direct child instead of OnGlobalLayoutListener (RN bypasses requestLayout, so global listener never fires for RN-driven layout changes) - defer state changes during DRAGGING/SETTLING to avoid interrupting dismiss swipes, apply when gesture settles - subtract insets.top from NativeView height on android (matching iOS) to prevent scroll content clipping at bottom of full-height sheets Co-Authored-By: Claude Opus 4.6 (1M context) --- .../modules/bottomsheet/BottomSheetView.kt | 79 +++++++++++++------ .../src/BottomSheetNativeComponent.tsx | 5 +- src/components/Dialog/index.tsx | 35 ++++---- 3 files changed, 75 insertions(+), 44 deletions(-) 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 196c6dd292..a68736fa44 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 @@ -11,7 +11,6 @@ import android.widget.FrameLayout import androidx.core.view.ViewCompat import androidx.core.view.WindowInsetsCompat import androidx.core.view.WindowInsetsControllerCompat -import androidx.core.view.allViews import com.facebook.react.bridge.LifecycleEventListener import com.facebook.react.bridge.ReactContext import com.facebook.react.bridge.UiThreadUtil @@ -38,7 +37,9 @@ class BottomSheetView( // Native content height observation (eliminates JS bridge round-trip) private var contentLayoutListener: View.OnLayoutChangeListener? = null - private var observedContentView: View? = null + private var observedChildren: List = emptyList() + private var lastObservedContentHeight: Float = 0f + private var pendingLayoutUpdate: Boolean = false private val screenHeight = context.resources.displayMetrics.heightPixels @@ -237,6 +238,13 @@ class BottomSheetView( BottomSheetBehavior.STATE_HALF_EXPANDED -> selectedSnapPoint = 1 BottomSheetBehavior.STATE_HIDDEN -> selectedSnapPoint = 0 } + // Apply deferred layout update after gesture completes + if (newState != BottomSheetBehavior.STATE_DRAGGING && + newState != BottomSheetBehavior.STATE_SETTLING && + pendingLayoutUpdate) { + pendingLayoutUpdate = false + updateLayout() + } } override fun onSlide( @@ -298,6 +306,15 @@ class BottomSheetView( val availableHeight = screenHeight - getStatusBarHeight() - getNavigationBarHeight() val shouldBeExpanded = targetHeight >= availableHeight + // Don't force state changes during user gestures — the ratio and maxHeight + // are already updated above, so when the gesture settles the sheet will land + // at the correct position. Forcing a state change mid-drag interrupts + // dismiss swipes and causes visual glitches. + if (currentState == BottomSheetBehavior.STATE_DRAGGING || currentState == BottomSheetBehavior.STATE_SETTLING) { + pendingLayoutUpdate = true + return + } + if (isKeyboardVisible) { if (behavior.state != BottomSheetBehavior.STATE_EXPANDED) { behavior.state = BottomSheetBehavior.STATE_EXPANDED @@ -316,53 +333,67 @@ class BottomSheetView( this.dialog?.dismiss() } - // Observe the content view's layout changes so that height updates are detected - // purely on the native side, without a JS bridge round-trip through onLayout. + // Observe each direct child of innerView via OnLayoutChangeListener so that + // height updates are detected purely on the native side. We use OnLayoutChangeListener + // (not OnGlobalLayoutListener) because React Native calls view.layout() directly + // via Yoga, bypassing requestLayout()/performTraversals(). OnLayoutChangeListener + // fires from setFrame() which IS called by layout(), so it catches RN updates. private fun startObservingContentHeight() { stopObservingContentHeight() val innerViewGroup = this.innerView as? ViewGroup ?: return - val contentView = innerViewGroup.getChildAt(0) ?: return - val listener = View.OnLayoutChangeListener { _, _, top, _, bottom, _, oldTop, _, oldBottom -> + val listener = View.OnLayoutChangeListener { _, _, top, _, bottom, _, _, oldTop, oldBottom -> val newHeight = bottom - top val oldHeight = oldBottom - oldTop - if (newHeight != oldHeight && newHeight > 0 && (isOpen || isOpening) && !isClosing) { - updateLayout() + if (newHeight != oldHeight) { + val contentHeight = getContentHeight() + if (contentHeight != lastObservedContentHeight && contentHeight > 0 && (isOpen || isOpening) && !isClosing) { + lastObservedContentHeight = contentHeight + updateLayout() + } } } - contentView.addOnLayoutChangeListener(listener) - this.contentLayoutListener = listener - this.observedContentView = contentView + val children = mutableListOf() + for (i in 0 until innerViewGroup.childCount) { + val child = innerViewGroup.getChildAt(i) + child.addOnLayoutChangeListener(listener) + children.add(child) + } - // The listener only fires on future changes. If content already laid out - // (e.g. dialog.show() triggered layout synchronously), pick up that height now. - if (contentView.height > 0) { + this.contentLayoutListener = listener + this.observedChildren = children + + // Pick up current height if content is already laid out + val contentHeight = getContentHeight() + if (contentHeight > 0 && contentHeight != lastObservedContentHeight) { + lastObservedContentHeight = contentHeight updateLayout() } } private fun stopObservingContentHeight() { contentLayoutListener?.let { listener -> - observedContentView?.removeOnLayoutChangeListener(listener) + observedChildren.forEach { it.removeOnLayoutChangeListener(listener) } } contentLayoutListener = null - observedContentView = null + observedChildren = emptyList() + lastObservedContentHeight = 0f } // Util private fun getContentHeight(): Float { - val innerView = this.innerView ?: return 0f - var index = 0 - innerView.allViews.forEach { - if (index == 1) { - return it.height.toFloat() - } - index++ + val innerView = this.innerView as? ViewGroup ?: return 0f + // Use the tallest direct child's height. The handle is absolutely positioned + // (overlaps the content), so summing would double-count its height as padding. + var maxChildHeight = 0f + for (i in 0 until innerView.childCount) { + val h = innerView.getChildAt(i).height.toFloat() + if (h > maxChildHeight) maxChildHeight = h } - return 0f + return maxChildHeight } private fun getTargetHeight(): Float { diff --git a/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx b/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx index 451ba6a388..f0035c9505 100644 --- a/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx +++ b/modules/bottom-sheet/src/BottomSheetNativeComponent.tsx @@ -12,7 +12,6 @@ import { import {useSafeAreaInsets} from 'react-native-safe-area-context' import {requireNativeModule, requireNativeViewManager} from 'expo-modules-core' -import {IS_IOS} from '#/env' import { type BottomSheetState, type BottomSheetViewProps, @@ -145,8 +144,6 @@ function BottomSheetNativeComponentInner({ const cornerRadius = rest.cornerRadius ?? 0 const {height: screenHeight} = useWindowDimensions() - const sheetHeight = IS_IOS ? screenHeight - insets.top : screenHeight - return ( diff --git a/src/components/Dialog/index.tsx b/src/components/Dialog/index.tsx index 7cb870d86d..5af626aa53 100644 --- a/src/components/Dialog/index.tsx +++ b/src/components/Dialog/index.tsx @@ -207,19 +207,8 @@ export const ScrollableInner = React.forwardRef( ref, ) { const {nativeSnapPoint, disableDrag, setDisableDrag} = useDialogContext() - const insets = useSafeAreaInsets() const isAtMaxSnapPoint = nativeSnapPoint === BottomSheetSnapPoint.Full - - let paddingBottom = 0 - if (IS_IOS) { - paddingBottom = tokens.space._2xl - } else { - paddingBottom = - Math.max(insets.bottom, tokens.space._5xl) + tokens.space._2xl - if (isAtMaxSnapPoint) { - paddingBottom += insets.top - } - } + const insets = useSafeAreaInsets() const onScroll = (e: NativeSyntheticEvent) => { if (!IS_ANDROID) { @@ -238,8 +227,14 @@ export const ScrollableInner = React.forwardRef( contentContainerStyle={[ a.pt_2xl, IS_LIQUID_GLASS ? a.px_2xl : a.px_xl, - {paddingBottom}, + platform({ + ios: a.pb_2xl, + android: { + paddingBottom: insets.bottom + tokens.space.xl, + }, + }), contentContainerStyle, + a.debug, ]} ref={ref} showsVerticalScrollIndicator={IS_ANDROID ? false : undefined} @@ -250,14 +245,19 @@ export const ScrollableInner = React.forwardRef( {...props} bounces={isAtMaxSnapPoint} scrollEventThrottle={50} - onScroll={IS_ANDROID ? onScroll : undefined} + // set drag state based on scroll on android. + // we want to detect if it's at the top or not, so watch + // scrollEndDrag and momentumScrollEnd as well + onScroll={android(onScroll)} + onScrollEndDrag={android(onScroll)} + onMomentumScrollEnd={android(onScroll)} keyboardShouldPersistTaps="handled" // TODO: figure out why this positions the header absolutely (rather than stickily) // on Android. fine to disable for now, because we don't have any // dialogs that use this that actually scroll -sfn stickyHeaderIndices={ios(header ? [0] : undefined)}> {header} - {children} + {children} ) }, @@ -293,7 +293,10 @@ export const InnerFlatList = React.forwardRef< } return ( - +