From 4475e01570e9ea1c9c26447b7c47cf509cfa84f8 Mon Sep 17 00:00:00 2001 From: Alan Lee Date: Sat, 7 Sep 2024 00:59:47 -0700 Subject: [PATCH] Modal bugfix for statusBarTranslucent prop and Android 15 (#46359) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46359 Remove unneeded code around size calculation and old arch support - updateState was getting called unnecessarily in multiple places --> only call from onSizeChanged() - this is a reliable source for getting the content size area of the dialog used for Modal - remove code checking duplicated update - Old architecture cleanup - Remove Java implementation of ShadowNode - we already have logic to set the node size via UIManagerModule::updateNodeSize(). This code is now group together in updateState() for both new and old architecture This fixes issues with resulting from wrong size calculation: - having gaps at bottom when we set `statusBarTranslucent` to `true` - Modal cut off at bottom on Android 15 (drawn under bottom nav bar) Changelog: [Android][Fixed] - Modal statusBarTranslucent bug, Modal at bottom being cut off in Android 15 (without forced edge-to-edge) [Android][Deprecation] - Deprecating ModalHostShadowNode and ModalHostHelper classes Reviewed By: mdvacca Differential Revision: D62286026 fbshipit-source-id: 03b64a7783c12bebd1457c86a9a2657adc882c79 --- .../ReactAndroid/api/ReactAndroid.api | 6 -- .../react/views/modal/ModalHostHelper.kt | 1 + .../react/views/modal/ModalHostShadowNode.kt | 5 +- .../views/modal/ReactModalHostManager.kt | 9 -- .../react/views/modal/ReactModalHostView.kt | 91 ++++--------------- 5 files changed, 20 insertions(+), 92 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index cfdade228fc..2d7ecf4f3b9 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -6506,13 +6506,10 @@ public final class com/facebook/react/views/modal/ReactModalHostManager : com/fa public static final field REACT_CLASS Ljava/lang/String; public fun ()V public synthetic fun addEventEmitters (Lcom/facebook/react/uimanager/ThemedReactContext;Landroid/view/View;)V - public fun createShadowNodeInstance ()Lcom/facebook/react/uimanager/LayoutShadowNode; - public synthetic fun createShadowNodeInstance ()Lcom/facebook/react/uimanager/ReactShadowNode; public synthetic fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Landroid/view/View; public fun getDelegate ()Lcom/facebook/react/uimanager/ViewManagerDelegate; public fun getExportedCustomDirectEventTypeConstants ()Ljava/util/Map; public fun getName ()Ljava/lang/String; - public fun getShadowNodeClass ()Ljava/lang/Class; public synthetic fun onAfterUpdateTransaction (Landroid/view/View;)V public synthetic fun onDropViewInstance (Landroid/view/View;)V public fun onDropViewInstance (Lcom/facebook/react/views/modal/ReactModalHostView;)V @@ -6579,12 +6576,10 @@ public final class com/facebook/react/views/modal/ReactModalHostView : android/v public final fun setStatusBarTranslucent (Z)V public final fun setTransparent (Z)V public final fun showOrUpdate ()V - public final fun updateState (II)V } public final class com/facebook/react/views/modal/ReactModalHostView$DialogRootViewGroup : com/facebook/react/views/view/ReactViewGroup, com/facebook/react/uimanager/RootView { public fun (Lcom/facebook/react/views/modal/ReactModalHostView;Landroid/content/Context;)V - public fun addView (Landroid/view/View;ILandroid/view/ViewGroup$LayoutParams;)V public fun handleException (Ljava/lang/Throwable;)V public fun onChildEndedNativeGesture (Landroid/view/View;Landroid/view/MotionEvent;)V public fun onChildStartedNativeGesture (Landroid/view/View;Landroid/view/MotionEvent;)V @@ -8055,4 +8050,3 @@ public final class com/facebook/react/views/view/ViewGroupClickEvent : com/faceb public fun canCoalesce ()Z public fun getEventName ()Ljava/lang/String; } - diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostHelper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostHelper.kt index f1698ae9b83..af5dc51a3bc 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostHelper.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostHelper.kt @@ -13,6 +13,7 @@ import android.graphics.Point import android.view.WindowManager /** Helper class for Modals. */ +@Deprecated("This class is no longer used and will be removed soon.") internal object ModalHostHelper { private val MIN_POINT = Point() private val MAX_POINT = Point() diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostShadowNode.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostShadowNode.kt index 4c52f3aa3d5..2466caa3b9b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostShadowNode.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ModalHostShadowNode.kt @@ -9,7 +9,6 @@ package com.facebook.react.views.modal import com.facebook.react.uimanager.LayoutShadowNode import com.facebook.react.uimanager.ReactShadowNodeImpl -import com.facebook.react.views.modal.ModalHostHelper.getModalHostSize /** * We implement the Modal by using an Android Dialog. That will fill the entire window of the @@ -19,6 +18,7 @@ import com.facebook.react.views.modal.ModalHostHelper.getModalHostSize * to be the window size. This will then cause the children of the Modal to layout as if they can * fill the window. */ +@Deprecated("This class is no longer used and will be removed soon.") internal class ModalHostShadowNode : LayoutShadowNode() { /** * We need to set the styleWidth and styleHeight of the one child (represented by the @@ -27,8 +27,5 @@ internal class ModalHostShadowNode : LayoutShadowNode() { */ override fun addChildAt(child: ReactShadowNodeImpl, i: Int) { super.addChildAt(child, i) - val modalSize = getModalHostSize(themedContext) - child.setStyleWidth(modalSize.x.toFloat()) - child.setStyleHeight(modalSize.y.toFloat()) } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostManager.kt index 7bb4bc2ae98..e0e742f7269 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostManager.kt @@ -11,7 +11,6 @@ import android.content.DialogInterface.OnShowListener import com.facebook.react.bridge.ReadableArray import com.facebook.react.common.MapBuilder import com.facebook.react.module.annotations.ReactModule -import com.facebook.react.uimanager.LayoutShadowNode import com.facebook.react.uimanager.ReactStylesDiffMap import com.facebook.react.uimanager.StateWrapper import com.facebook.react.uimanager.ThemedReactContext @@ -21,7 +20,6 @@ import com.facebook.react.uimanager.ViewManagerDelegate import com.facebook.react.uimanager.annotations.ReactProp import com.facebook.react.viewmanagers.ModalHostViewManagerDelegate import com.facebook.react.viewmanagers.ModalHostViewManagerInterface -import com.facebook.react.views.modal.ModalHostHelper.getModalHostSize import com.facebook.react.views.modal.ReactModalHostView.OnRequestCloseListener /** View manager for [ReactModalHostView] components. */ @@ -35,11 +33,6 @@ public class ReactModalHostManager : protected override fun createViewInstance(reactContext: ThemedReactContext): ReactModalHostView = ReactModalHostView(reactContext) - public override fun createShadowNodeInstance(): LayoutShadowNode = ModalHostShadowNode() - - public override fun getShadowNodeClass(): Class = - ModalHostShadowNode::class.java - public override fun onDropViewInstance(view: ReactModalHostView) { super.onDropViewInstance(view) view.onDropInstance() @@ -136,8 +129,6 @@ public class ReactModalHostManager : stateWrapper: StateWrapper ): Any? { view.stateWrapper = stateWrapper - val modalSize = getModalHostSize(view.context) - view.updateState(modalSize.x, modalSize.y) return null } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.kt index f6e0d82fc01..060ca7ea047 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.kt @@ -30,7 +30,6 @@ import com.facebook.react.R import com.facebook.react.bridge.GuardedRunnable import com.facebook.react.bridge.LifecycleEventListener import com.facebook.react.bridge.ReactContext -import com.facebook.react.bridge.ReadableMap import com.facebook.react.bridge.UiThreadUtil import com.facebook.react.bridge.WritableMap import com.facebook.react.bridge.WritableNativeMap @@ -45,9 +44,9 @@ import com.facebook.react.uimanager.ThemedReactContext import com.facebook.react.uimanager.UIManagerModule import com.facebook.react.uimanager.events.EventDispatcher import com.facebook.react.views.common.ContextUtils +import com.facebook.react.views.view.setStatusBarTranslucency import com.facebook.react.views.view.ReactViewGroup import java.util.Objects -import kotlin.math.abs /** * ReactModalHostView is a view that sits in the view hierarchy representing a Modal view. @@ -288,16 +287,7 @@ public class ReactModalHostView(context: ThemedReactContext) : * changed. This has the pleasant side-effect of us not having to preface all Modals with "top: * statusBarHeight", since that margin will be included in the FrameLayout. */ - get() { - val frameLayout = FrameLayout(context) - frameLayout.addView(hostView) - if (statusBarTranslucent) { - frameLayout.systemUiVisibility = SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN - } else { - frameLayout.fitsSystemWindows = true - } - return frameLayout - } + get() = FrameLayout(context).apply { addView(dialogRootViewGroup) } /** * updateProperties will update the properties that do not require us to recreate the dialog @@ -324,6 +314,8 @@ public class ReactModalHostView(context: ThemedReactContext) : } } + dialogWindow.setStatusBarTranslucency(statusBarTranslucent) + if (transparent) { window.clearFlags(WindowManager.LayoutParams.FLAG_DIM_BEHIND) } else { @@ -357,10 +349,6 @@ public class ReactModalHostView(context: ThemedReactContext) : } } - public fun updateState(width: Int, height: Int) { - hostView.updateState(width, height) - } - // This listener is called when the user presses KeyEvent.KEYCODE_BACK // An event is then passed to JS which can either close or not close the Modal by setting the // visible property @@ -386,7 +374,6 @@ public class ReactModalHostView(context: ThemedReactContext) : public inner class DialogRootViewGroup(context: Context?) : ReactViewGroup(context), RootView { internal var stateWrapper: StateWrapper? = null - private var hasAdjustedSize = false private var viewWidth = 0 private var viewHeight = 0 private val jSTouchDispatcher: JSTouchDispatcher = JSTouchDispatcher(this) @@ -406,31 +393,8 @@ public class ReactModalHostView(context: ThemedReactContext) : super.onSizeChanged(w, h, oldw, oldh) viewWidth = w viewHeight = h - updateFirstChildView() - } - private fun updateFirstChildView() { - if (childCount > 0) { - hasAdjustedSize = false - val viewTag: Int = getChildAt(0).id - if (stateWrapper != null) { - // This will only be called under Fabric - updateState(viewWidth, viewHeight) - } else { - // TODO: T44725185 remove after full migration to Fabric - val reactContext: ReactContext = reactContext - reactContext.runOnNativeModulesQueueThread( - object : GuardedRunnable(reactContext) { - override fun runGuarded() { - this@DialogRootViewGroup.reactContext.reactApplicationContext - .getNativeModule(UIManagerModule::class.java) - ?.updateNodeSize(viewTag, viewWidth, viewHeight) - } - }) - } - } else { - hasAdjustedSize = true - } + updateState(viewWidth, viewHeight) } @UiThread @@ -438,43 +402,24 @@ public class ReactModalHostView(context: ThemedReactContext) : val realWidth: Float = PixelUtil.toDIPFromPixel(width.toFloat()) val realHeight: Float = PixelUtil.toDIPFromPixel(height.toFloat()) - // Check incoming state values. If they're already the correct value, return early to prevent - // infinite UpdateState/SetState loop. - val currentState: ReadableMap? = stateWrapper?.getStateData() - if (currentState != null) { - val delta = 0.9f - val stateScreenHeight = - if (currentState.hasKey("screenHeight")) { - currentState.getDouble("screenHeight").toFloat() - } else { - 0f - } - val stateScreenWidth = - if (currentState.hasKey("screenWidth")) { - currentState.getDouble("screenWidth").toFloat() - } else { - 0f - } - - if (abs((stateScreenWidth - realWidth).toDouble()) < delta && - abs((stateScreenHeight - realHeight).toDouble()) < delta) { - return - } - } - stateWrapper?.let { sw -> + // new architecture val newStateData: WritableMap = WritableNativeMap() newStateData.putDouble("screenWidth", realWidth.toDouble()) newStateData.putDouble("screenHeight", realHeight.toDouble()) sw.updateState(newStateData) - } - } - - override fun addView(child: View, index: Int, params: LayoutParams) { - super.addView(child, index, params) - if (hasAdjustedSize) { - updateFirstChildView() - } + } ?: run { + // old architecture + // TODO: T44725185 remove after full migration to Fabric + reactContext.runOnNativeModulesQueueThread( + object : GuardedRunnable(reactContext) { + override fun runGuarded() { + reactContext.reactApplicationContext + .getNativeModule(UIManagerModule::class.java) + ?.updateNodeSize(id, viewWidth, viewHeight) + } + }) + } } override fun handleException(t: Throwable) {