Skip to content

Commit 3799da0

Browse files
cortinicofacebook-github-bot
authored andcommitted
Correctly create the first modal state (#52835)
Summary: There is currently a bug with Modals with New Architecture where the first frame is rendered incorrectly, specifically not accounting for all the vertical insets (only the status bar). This fixes it. Specifically: 1. I've removed the caching of the statusbar height from `ReactModalHostView` as that was not working correctly. Sometimes the value returned `0` meaning that it was not yet computed when Fabric was asking for it. In the updated implementation we now query `FabricUIManager` given the `surfaceId` of the modal. 2. I've modified the logic to account for all the vertical insets, not just the status bar. ## Changelog: [ANDROID] [FIXED] - Correctly account for insets on first render of Modals on New Arch Pull Request resolved: #52835 Test Plan: Tested on Marketplace Location Picker and the picker is still working correctly: https://pxl.cl/7NjtJ Reviewed By: mdvacca Differential Revision: D78975126 Pulled By: cortinico
1 parent e1f6a19 commit 3799da0

7 files changed

Lines changed: 108 additions & 66 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import com.facebook.react.internal.interop.InteropEventEmitter;
7171
import com.facebook.react.modules.core.ReactChoreographer;
7272
import com.facebook.react.modules.i18nmanager.I18nUtil;
73+
import com.facebook.react.uimanager.DisplayMetricsHolder;
7374
import com.facebook.react.uimanager.GuardedFrameCallback;
7475
import com.facebook.react.uimanager.IllegalViewOperationException;
7576
import com.facebook.react.uimanager.PixelUtil;
@@ -97,6 +98,7 @@
9798
import java.util.HashSet;
9899
import java.util.List;
99100
import java.util.Map;
101+
import java.util.Objects;
100102
import java.util.Queue;
101103
import java.util.Set;
102104
import java.util.concurrent.CopyOnWriteArrayList;
@@ -725,6 +727,23 @@ public boolean getThemeData(int surfaceId, float[] defaultTextInputPadding) {
725727
return true;
726728
}
727729

730+
/**
731+
* This method is used to get the encoded screen size without vertical insets for a given surface.
732+
* It's used by the Modal component to determine the size of the screen without vertical insets.
733+
* The method is private as it's accessed via JNI from C++.
734+
*
735+
* @param surfaceId The surface ID of the surface for which the Modal is going to render.
736+
* @return The encoded screen size as a long (both width and height) are represented without
737+
* vertical insets.
738+
*/
739+
private long getEncodedScreenSizeWithoutVerticalInsets(int surfaceId) {
740+
SurfaceMountingManager surfaceMountingManager = mMountingManager.getSurfaceManager(surfaceId);
741+
Objects.requireNonNull(surfaceMountingManager);
742+
ThemedReactContext context = Objects.requireNonNull(surfaceMountingManager.getContext());
743+
return DisplayMetricsHolder.getEncodedScreenSizeWithoutVerticalInsets(
744+
context.getCurrentActivity());
745+
}
746+
728747
@Override
729748
public void addUIManagerEventListener(UIManagerListener listener) {
730749
mListeners.add(listener);

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/DisplayMetricsHolder.kt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import androidx.core.view.ViewCompat
1515
import androidx.core.view.WindowInsetsCompat
1616
import com.facebook.react.bridge.WritableMap
1717
import com.facebook.react.bridge.WritableNativeMap
18+
import com.facebook.react.uimanager.PixelUtil.pxToDp
1819

1920
/**
2021
* Holds an instance of the current DisplayMetrics so we don't have to thread it through all the
@@ -111,4 +112,33 @@ public object DisplayMetricsHolder {
111112
WindowInsetsCompat.Type.displayCutout())
112113
.top
113114
}
115+
116+
/**
117+
* Returns the encoded screen size without vertical insets.
118+
*
119+
* This is needed to render components that needs to be correctly positioned on the screen on
120+
* their first frame. Modal is one of such components.
121+
*
122+
* @param activity the [Activity] to get the insets from.
123+
* @return the encoded screen size as a [Long] value, where the first 32 bits represent the width
124+
* and the last 32 bits represent the height in dp (density-independent pixels).
125+
*/
126+
// This annotation can be removed once FabricUIManager is migrated to Kotlin
127+
@JvmName("getEncodedScreenSizeWithoutVerticalInsets")
128+
@JvmStatic
129+
internal fun getEncodedScreenSizeWithoutVerticalInsets(activity: Activity?): Long {
130+
val windowInsets = activity?.window?.decorView?.let(ViewCompat::getRootWindowInsets) ?: return 0
131+
val insets =
132+
windowInsets.getInsets(
133+
WindowInsetsCompat.Type.statusBars() or
134+
WindowInsetsCompat.Type.navigationBars() or
135+
WindowInsetsCompat.Type.displayCutout())
136+
val verticalInsets = insets.top + insets.bottom
137+
return encodeFloatsToLong(
138+
(checkNotNull(screenDisplayMetrics).widthPixels).toFloat().pxToDp(),
139+
(checkNotNull(screenDisplayMetrics).heightPixels - verticalInsets).toFloat().pxToDp())
140+
}
141+
142+
private fun encodeFloatsToLong(width: Float, height: Float): Long =
143+
(width.toRawBits().toLong()) shl 32 or (height.toRawBits().toLong())
114144
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.kt

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ import com.facebook.react.bridge.WritableNativeMap
4040
import com.facebook.react.common.annotations.VisibleForTesting
4141
import com.facebook.react.common.build.ReactBuildConfig
4242
import com.facebook.react.config.ReactFeatureFlags
43-
import com.facebook.react.uimanager.DisplayMetricsHolder
44-
import com.facebook.react.uimanager.DisplayMetricsHolder.getStatusBarHeightPx
4543
import com.facebook.react.uimanager.JSPointerDispatcher
4644
import com.facebook.react.uimanager.JSTouchDispatcher
4745
import com.facebook.react.uimanager.PixelUtil.pxToDp
@@ -51,13 +49,11 @@ import com.facebook.react.uimanager.ThemedReactContext
5149
import com.facebook.react.uimanager.UIManagerModule
5250
import com.facebook.react.uimanager.events.EventDispatcher
5351
import com.facebook.react.views.common.ContextUtils
54-
import com.facebook.react.views.modal.ReactModalHostView.DialogRootViewGroup
5552
import com.facebook.react.views.view.ReactViewGroup
5653
import com.facebook.react.views.view.disableEdgeToEdge
5754
import com.facebook.react.views.view.enableEdgeToEdge
5855
import com.facebook.react.views.view.isEdgeToEdgeFeatureFlagOn
5956
import com.facebook.react.views.view.setStatusBarTranslucency
60-
import com.facebook.yoga.annotations.DoNotStrip
6157

6258
/**
6359
* ReactModalHostView is a view that sits in the view hierarchy representing a Modal view.
@@ -72,7 +68,6 @@ import com.facebook.yoga.annotations.DoNotStrip
7268
* addition and removal of views to the DialogRootViewGroup.
7369
*/
7470
@SuppressLint("ViewConstructor")
75-
@DoNotStrip
7671
public class ReactModalHostView(context: ThemedReactContext) :
7772
ViewGroup(context), LifecycleEventListener {
7873

@@ -131,7 +126,6 @@ public class ReactModalHostView(context: ThemedReactContext) :
131126
private var createNewDialog = false
132127

133128
init {
134-
initStatusBarHeight(context)
135129
dialogRootViewGroup = DialogRootViewGroup(context)
136130
}
137131

@@ -484,26 +478,6 @@ public class ReactModalHostView(context: ThemedReactContext) :
484478

485479
private companion object {
486480
private const val TAG = "ReactModalHost"
487-
488-
// We store the status bar height to be able to properly position
489-
// the modal on the first render.
490-
private var statusBarHeight = 0
491-
492-
private fun initStatusBarHeight(reactContext: ReactContext) {
493-
statusBarHeight = getStatusBarHeightPx(reactContext.currentActivity)
494-
}
495-
496-
@JvmStatic
497-
@DoNotStrip
498-
private fun getScreenDisplayMetricsWithoutInsets(): Long {
499-
val displayMetrics = DisplayMetricsHolder.getScreenDisplayMetrics()
500-
return encodeFloatsToLong(
501-
displayMetrics.widthPixels.toFloat().pxToDp(),
502-
(displayMetrics.heightPixels - statusBarHeight).toFloat().pxToDp())
503-
}
504-
505-
private fun encodeFloatsToLong(width: Float, height: Float): Long =
506-
(width.toRawBits().toLong()) shl 32 or (height.toRawBits().toLong())
507481
}
508482

509483
/**
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include "ModalHostViewComponentDescriptor.h"
9+
10+
namespace facebook::react {
11+
12+
#ifdef ANDROID
13+
State::Shared ModalHostViewComponentDescriptor::createInitialState(
14+
const Props::Shared& props,
15+
const ShadowNodeFamily::Shared& family) const {
16+
// For Android, we need to get the size of the screen without the vertical
17+
// insets to correctly position the modal on the first rendering.
18+
// For this reason we provide the `createInitialState` implementation
19+
// that will query FabricUIManager for the size of the screen without
20+
// vertical insets.
21+
22+
int surfaceId = family->getSurfaceId();
23+
24+
const jni::global_ref<jobject>& fabricUIManager =
25+
contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");
26+
27+
static auto getEncodedScreenSizeWithoutVerticalInsets =
28+
jni::findClassStatic(UIManagerJavaDescriptor)
29+
->getMethod<jlong(jint)>("getEncodedScreenSizeWithoutVerticalInsets");
30+
31+
auto result =
32+
getEncodedScreenSizeWithoutVerticalInsets(fabricUIManager, surfaceId);
33+
34+
// Inspired from yogaMeasureToSize from conversions.h
35+
int32_t wBits = 0xFFFFFFFF & (result >> 32);
36+
int32_t hBits = 0xFFFFFFFF & result;
37+
38+
auto* measuredWidth = reinterpret_cast<float*>(&wBits);
39+
auto* measuredHeight = reinterpret_cast<float*>(&hBits);
40+
41+
return std::make_shared<ModalHostViewShadowNode::ConcreteState>(
42+
std::make_shared<const ModalHostViewState>(ModalHostViewState(
43+
Size{.width = *measuredWidth, .height = *measuredHeight})),
44+
family);
45+
}
46+
#endif // ANDROID
47+
48+
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/components/modal/ModalHostViewComponentDescriptor.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ class ModalHostViewComponentDescriptor final
3737

3838
ConcreteComponentDescriptor::adopt(shadowNode);
3939
}
40+
41+
#ifdef ANDROID
42+
State::Shared createInitialState(
43+
const Props::Shared& props,
44+
const ShadowNodeFamily::Shared& family) const override;
45+
#endif // ANDROID
46+
47+
private:
48+
constexpr static auto UIManagerJavaDescriptor =
49+
"com/facebook/react/fabric/FabricUIManager";
4050
};
4151

4252
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/components/modal/platform/android/JReactModalHostView.h

Lines changed: 0 additions & 38 deletions
This file was deleted.

packages/react-native/ReactCommon/react/renderer/components/modal/platform/android/ModalHostViewUtils.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@
77

88
#include <react/renderer/components/modal/ModalHostViewUtils.h>
99
#include <react/renderer/graphics/Size.h>
10-
#include "JReactModalHostView.h"
1110

1211
namespace facebook::react {
1312

1413
Size ModalHostViewScreenSize() {
15-
return JReactModalHostView::getDisplayMetrics();
14+
return Size{0, 0};
1615
}
1716

1817
} // namespace facebook::react

0 commit comments

Comments
 (0)