Skip to content

Commit 2e76fc8

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 fbshipit-source-id: d7afb4fa5d2f43a7e33da3860432fa6dfe0dc8d7
1 parent 5fc23d7 commit 2e76fc8

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
@@ -16,6 +16,7 @@ import androidx.core.view.WindowInsetsCompat
1616
import androidx.window.layout.WindowMetricsCalculator
1717
import com.facebook.react.bridge.WritableMap
1818
import com.facebook.react.bridge.WritableNativeMap
19+
import com.facebook.react.uimanager.PixelUtil.pxToDp
1920
import com.facebook.react.views.view.isEdgeToEdgeFeatureFlagOn
2021

2122
/**
@@ -140,4 +141,33 @@ public object DisplayMetricsHolder {
140141
WindowInsetsCompat.Type.displayCutout())
141142
.top
142143
}
144+
145+
/**
146+
* Returns the encoded screen size without vertical insets.
147+
*
148+
* This is needed to render components that needs to be correctly positioned on the screen on
149+
* their first frame. Modal is one of such components.
150+
*
151+
* @param activity the [Activity] to get the insets from.
152+
* @return the encoded screen size as a [Long] value, where the first 32 bits represent the width
153+
* and the last 32 bits represent the height in dp (density-independent pixels).
154+
*/
155+
// This annotation can be removed once FabricUIManager is migrated to Kotlin
156+
@JvmName("getEncodedScreenSizeWithoutVerticalInsets")
157+
@JvmStatic
158+
internal fun getEncodedScreenSizeWithoutVerticalInsets(activity: Activity?): Long {
159+
val windowInsets = activity?.window?.decorView?.let(ViewCompat::getRootWindowInsets) ?: return 0
160+
val insets =
161+
windowInsets.getInsets(
162+
WindowInsetsCompat.Type.statusBars() or
163+
WindowInsetsCompat.Type.navigationBars() or
164+
WindowInsetsCompat.Type.displayCutout())
165+
val verticalInsets = insets.top + insets.bottom
166+
return encodeFloatsToLong(
167+
(checkNotNull(screenDisplayMetrics).widthPixels).toFloat().pxToDp(),
168+
(checkNotNull(screenDisplayMetrics).heightPixels - verticalInsets).toFloat().pxToDp())
169+
}
170+
171+
private fun encodeFloatsToLong(width: Float, height: Float): Long =
172+
(width.toRawBits().toLong()) shl 32 or (height.toRawBits().toLong())
143173
}

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
@@ -41,8 +41,6 @@ import com.facebook.react.common.annotations.UnstableReactNativeAPI
4141
import com.facebook.react.common.annotations.VisibleForTesting
4242
import com.facebook.react.common.build.ReactBuildConfig
4343
import com.facebook.react.config.ReactFeatureFlags
44-
import com.facebook.react.uimanager.DisplayMetricsHolder
45-
import com.facebook.react.uimanager.DisplayMetricsHolder.getStatusBarHeightPx
4644
import com.facebook.react.uimanager.JSPointerDispatcher
4745
import com.facebook.react.uimanager.JSTouchDispatcher
4846
import com.facebook.react.uimanager.PixelUtil.pxToDp
@@ -52,13 +50,11 @@ import com.facebook.react.uimanager.ThemedReactContext
5250
import com.facebook.react.uimanager.UIManagerModule
5351
import com.facebook.react.uimanager.events.EventDispatcher
5452
import com.facebook.react.views.common.ContextUtils
55-
import com.facebook.react.views.modal.ReactModalHostView.DialogRootViewGroup
5653
import com.facebook.react.views.view.ReactViewGroup
5754
import com.facebook.react.views.view.disableEdgeToEdge
5855
import com.facebook.react.views.view.enableEdgeToEdge
5956
import com.facebook.react.views.view.isEdgeToEdgeFeatureFlagOn
6057
import com.facebook.react.views.view.setStatusBarTranslucency
61-
import com.facebook.yoga.annotations.DoNotStrip
6258

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

@@ -132,7 +127,6 @@ public class ReactModalHostView(context: ThemedReactContext) :
132127
private var createNewDialog = false
133128

134129
init {
135-
initStatusBarHeight(context)
136130
dialogRootViewGroup = DialogRootViewGroup(context)
137131
}
138132

@@ -485,26 +479,6 @@ public class ReactModalHostView(context: ThemedReactContext) :
485479

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

510484
/**
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)