Migrate needsCustomLayoutForChildren check to the new architecture (#34254)

Summary:
Fixes https://github.com/facebook/react-native/issues/34120

The new React Native architecture doesn't check `needsCustomLayoutForChildren` so it wrongly positions native views on Android. In https://github.com/facebook/react-native/issues/34120 there are videos comparing the positioning of a native action view in the old and the new architecture.

This PR passes the parent tag to the `updateLayout` method of the `SurfaceMountingManager`. The `SurfaceMountingManager` calls `needsCustomLayoutForChildren` on the parent view manager (copied the code from the `NativeViewHierarchyManager` in the old architecture).

**NOTE** - I wasn't sure where to get the parent shadow view from so I've put in my best guesses where I could and left it as `{}` otherwise.

## Changelog

[Android] [Fixed] - Migrate `needsCustomLayoutForChildren` check to the new architecture

Pull Request resolved: https://github.com/facebook/react-native/pull/34254

Test Plan:
I checked the fix in the repro from https://github.com/facebook/react-native/issues/34165. Here is a video of the action view closing using the native button that is now visible in the new architecture.

https://user-images.githubusercontent.com/1761227/180607896-35bf477f-4552-4b8a-8e09-9e8c49122c0c.mov

Reviewed By: cipolleschi

Differential Revision: D38153924

Pulled By: javache

fbshipit-source-id: e2c77fa70d725a33ce73fe4a615f6d884312580c
This commit is contained in:
Graham Mendick
2022-07-28 09:57:36 -07:00
committed by Facebook GitHub Bot
parent 1ce23ce435
commit e24ce708ab
10 changed files with 53 additions and 31 deletions
@@ -44,8 +44,10 @@ CppMountItem CppMountItem::UpdatePropsMountItem(
CppMountItem CppMountItem::UpdateStateMountItem(ShadowView const &shadowView) {
return {CppMountItem::Type::UpdateState, {}, {}, shadowView, -1};
}
CppMountItem CppMountItem::UpdateLayoutMountItem(ShadowView const &shadowView) {
return {CppMountItem::Type::UpdateLayout, {}, {}, shadowView, -1};
CppMountItem CppMountItem::UpdateLayoutMountItem(
ShadowView const &shadowView,
ShadowView const &parentView) {
return {CppMountItem::Type::UpdateLayout, parentView, {}, shadowView, -1};
}
CppMountItem CppMountItem::UpdateEventEmitterMountItem(
ShadowView const &shadowView) {
@@ -46,7 +46,9 @@ struct CppMountItem final {
static CppMountItem UpdateStateMountItem(ShadowView const &shadowView);
static CppMountItem UpdateLayoutMountItem(ShadowView const &shadowView);
static CppMountItem UpdateLayoutMountItem(
ShadowView const &shadowView,
ShadowView const &parentView);
static CppMountItem UpdateEventEmitterMountItem(ShadowView const &shadowView);
@@ -79,7 +79,7 @@ static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
case CppMountItem::Type::UpdatePadding:
return 5; // tag, top, left, bottom, right
case CppMountItem::Type::UpdateLayout:
return 6; // tag, x, y, w, h, DisplayType
return 7; // tag, parentTag, x, y, w, h, DisplayType
case CppMountItem::Type::UpdateOverflowInset:
return 5; // tag, left, top, right, bottom
case CppMountItem::Undefined:
@@ -381,7 +381,7 @@ void FabricMountingManager::executeMount(
newChildShadowView.layoutMetrics) {
cppUpdateLayoutMountItems.push_back(
CppMountItem::UpdateLayoutMountItem(
mutation.newChildShadowView));
mutation.newChildShadowView, parentShadowView));
}
// OverflowInset: This is the values indicating boundaries including
@@ -441,7 +441,8 @@ void FabricMountingManager::executeMount(
// Layout
cppUpdateLayoutMountItems.push_back(
CppMountItem::UpdateLayoutMountItem(newChildShadowView));
CppMountItem::UpdateLayoutMountItem(
newChildShadowView, parentShadowView));
// OverflowInset: This is the values indicating boundaries including
// children of the current view. The layout of current view may not
@@ -548,7 +549,7 @@ void FabricMountingManager::executeMount(
int intBufferPosition = 0;
int objBufferPosition = 0;
int prevMountItemType = -1;
jint temp[6];
jint temp[7];
for (int i = 0; i < cppCommonMountItems.size(); i++) {
const auto &mountItem = cppCommonMountItems[i];
const auto &mountItemType = mountItem.type;
@@ -723,13 +724,14 @@ void FabricMountingManager::executeMount(
toInt(mountItem.newChildShadowView.layoutMetrics.displayType);
temp[0] = mountItem.newChildShadowView.tag;
temp[1] = x;
temp[2] = y;
temp[3] = w;
temp[4] = h;
temp[5] = displayType;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 6, temp);
intBufferPosition += 6;
temp[1] = mountItem.parentShadowView.tag;
temp[2] = x;
temp[3] = y;
temp[4] = w;
temp[5] = h;
temp[6] = displayType;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 7, temp);
intBufferPosition += 7;
}
}
if (!cppUpdateOverflowInsetMountItems.empty()) {
@@ -967,7 +967,8 @@ public class SurfaceMountingManager {
}
@UiThread
public void updateLayout(int reactTag, int x, int y, int width, int height, int displayType) {
public void updateLayout(
int reactTag, int parentTag, int x, int y, int width, int height, int displayType) {
if (isStopped()) {
return;
}
@@ -992,9 +993,14 @@ public class SurfaceMountingManager {
parent.requestLayout();
}
// TODO: T31905686 Check if the parent of the view has to layout the view, or the child has
// to lay itself out. see NativeViewHierarchyManager.updateLayout
viewToUpdate.layout(x, y, x + width, y + height);
ViewState parentViewState = getViewState(parentTag);
ViewGroupManager<?> parentViewManager = null;
if (parentViewState.mViewManager != null) {
parentViewManager = parentViewState.mViewManager.getViewGroupManager();
}
if (parentViewManager == null || !parentViewManager.needsCustomLayoutForChildren()) {
viewToUpdate.layout(x, y, x + width, y + height);
}
// displayType: 0 represents display: 'none'
int visibility = displayType == 0 ? View.INVISIBLE : View.VISIBLE;
@@ -149,13 +149,15 @@ public class IntBufferBatchMountItem implements MountItem {
surfaceMountingManager.updateState(mIntBuffer[i++], castToState(mObjBuffer[j++]));
} else if (type == INSTRUCTION_UPDATE_LAYOUT) {
int reactTag = mIntBuffer[i++];
int parentTag = mIntBuffer[i++];
int x = mIntBuffer[i++];
int y = mIntBuffer[i++];
int width = mIntBuffer[i++];
int height = mIntBuffer[i++];
int displayType = mIntBuffer[i++];
surfaceMountingManager.updateLayout(reactTag, x, y, width, height, displayType);
surfaceMountingManager.updateLayout(
reactTag, parentTag, x, y, width, height, displayType);
} else if (type == INSTRUCTION_UPDATE_PADDING) {
surfaceMountingManager.updatePadding(
@@ -57,7 +57,7 @@ void LayoutAnimationDriver::animationMutationsForFrame(
// Create the mutation instruction
mutationsList.emplace_back(ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, mutatedShadowView));
keyframe.viewPrev, mutatedShadowView, keyframe.parentView));
PrintMutationInstruction("Animation Progress:", updateMutation);
@@ -1241,7 +1241,9 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
break;
case ShadowViewMutation::Type::Update:
mutationsList.push_back(ShadowViewMutation::UpdateMutation(
prev, finalMutation.newChildShadowView));
prev,
finalMutation.newChildShadowView,
finalMutation.parentShadowView));
break;
}
if (finalMutation.newChildShadowView.tag > 0) {
@@ -1266,7 +1268,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
auto mutatedShadowView =
createInterpolatedShadowView(1, keyframe.viewStart, keyframe.viewEnd);
auto generatedPenultimateMutation = ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, mutatedShadowView);
keyframe.viewPrev, mutatedShadowView, keyframe.parentView);
react_native_assert(
generatedPenultimateMutation.oldChildShadowView.tag > 0);
react_native_assert(
@@ -1277,7 +1279,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
mutationsList.push_back(generatedPenultimateMutation);
auto generatedMutation = ShadowViewMutation::UpdateMutation(
mutatedShadowView, keyframe.viewEnd);
mutatedShadowView, keyframe.viewEnd, keyframe.parentView);
react_native_assert(generatedMutation.oldChildShadowView.tag > 0);
react_native_assert(generatedMutation.newChildShadowView.tag > 0);
PrintMutationInstruction(
@@ -1286,7 +1288,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
mutationsList.push_back(generatedMutation);
} else {
auto mutation = ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, keyframe.viewEnd);
keyframe.viewPrev, keyframe.viewEnd, keyframe.parentView);
PrintMutationInstruction(
logPrefix +
"Animation Complete: Queuing up Final Synthetic Mutation:",
@@ -574,7 +574,7 @@ static void updateMatchedPair(
if (oldPair.shadowView != newPair.shadowView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldPair.shadowView, newPair.shadowView));
oldPair.shadowView, newPair.shadowView, parentShadowView));
}
}
}
@@ -835,7 +835,9 @@ static void calculateShadowViewMutationsFlattener(
newTreeNodePair.isConcreteView && oldTreeNodePair.isConcreteView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldTreeNodePair.shadowView, newTreeNodePair.shadowView));
oldTreeNodePair.shadowView,
newTreeNodePair.shadowView,
node.shadowView));
}
// Update children if appropriate.
@@ -1161,7 +1163,9 @@ static void calculateShadowViewMutationsV2(
oldChildPair.shadowView != newChildPair.shadowView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldChildPair.shadowView, newChildPair.shadowView));
oldChildPair.shadowView,
newChildPair.shadowView,
parentShadowView));
}
// Recursively update tree if ShadowNode pointers are not equal
@@ -1699,7 +1703,7 @@ ShadowViewMutation::List calculateShadowViewMutations(
if (oldRootShadowView != newRootShadowView) {
mutations.push_back(ShadowViewMutation::UpdateMutation(
oldRootShadowView, newRootShadowView));
oldRootShadowView, newRootShadowView, {}));
}
calculateShadowViewMutationsV2(
@@ -84,10 +84,11 @@ ShadowViewMutation ShadowViewMutation::RemoveDeleteTreeMutation(
ShadowViewMutation ShadowViewMutation::UpdateMutation(
ShadowView oldChildShadowView,
ShadowView newChildShadowView) {
ShadowView newChildShadowView,
ShadowView parentShadowView) {
return {
/* .type = */ Update,
/* .parentShadowView = */ {},
/* .parentShadowView = */ std::move(parentShadowView),
/* .oldChildShadowView = */ std::move(oldChildShadowView),
/* .newChildShadowView = */ std::move(newChildShadowView),
/* .index = */ -1,
@@ -76,7 +76,8 @@ struct ShadowViewMutation final {
*/
static ShadowViewMutation UpdateMutation(
ShadowView oldChildShadowView,
ShadowView newChildShadowView);
ShadowView newChildShadowView,
ShadowView parentShadowView);
#pragma mark - Type