Skip to content

Commit

Permalink
Fix changes of View visibilities
Browse files Browse the repository at this point in the history
Summary:
The purpose of this diff is to ensure that visibility changes are handled correctly when the value of "display" for a View changes from 'flex' to 'none'.

RNTester is nesting several Views with different kind of visibilities. When the user tap on an item there's a state update that changes the visibility styles for some of these views. Fabric does not reflect the right changes of visibility on the screen.

changelog: internal

Reviewed By: shergin

Differential Revision: D25841763

fbshipit-source-id: 769b97afb72939d346a4c6f2669ff938b35596bc
  • Loading branch information
mdvacca authored and facebook-github-bot committed Jan 11, 2021
1 parent 50f2dd9 commit 4076293
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
} else if (mountItemType == CppMountItem::Type::UpdatePadding) {
return 5; // tag, top, left, bottom, right
} else if (mountItemType == CppMountItem::Type::UpdateLayout) {
return 6; // tag, x, y, w, h, layoutDirection
return 7; // tag, x, y, w, h, layoutDirection, DisplayType
} else if (mountItemType == CppMountItem::Type::UpdateEventEmitter) {
return 1; // tag
} else {
Expand Down Expand Up @@ -714,7 +714,7 @@ void Binding::schedulerDidFinishTransaction(
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;
Expand Down Expand Up @@ -879,15 +879,18 @@ void Binding::schedulerDidFinishTransaction(
int h = round(frame.size.height * pointScaleFactor);
int layoutDirection =
toInt(mountItem.newChildShadowView.layoutMetrics.layoutDirection);
int displayType =
toInt(mountItem.newChildShadowView.layoutMetrics.displayType);

temp[0] = mountItem.newChildShadowView.tag;
temp[1] = x;
temp[2] = y;
temp[3] = w;
temp[4] = h;
temp[5] = layoutDirection;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 6, temp);
intBufferPosition += 6;
temp[6] = displayType;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 7, temp);
intBufferPosition += 7;
}
}
if (cppUpdateEventEmitterMountItems.size() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ public void updateProps(int reactTag, @Nullable ReadableMap props) {
}

@UiThread
public void updateLayout(int reactTag, int x, int y, int width, int height) {
public void updateLayout(int reactTag, int x, int y, int width, int height, int displayType) {
UiThreadUtil.assertOnUiThread();

ViewState viewState = getViewState(reactTag);
Expand All @@ -589,6 +589,12 @@ public void updateLayout(int reactTag, int x, int y, int width, int height) {
// 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);

// displayType: 0 represents display: 'none'
int visibility = displayType == 0 ? View.INVISIBLE : View.VISIBLE;
if (viewToUpdate.getVisibility() != visibility) {
viewToUpdate.setVisibility(visibility);
}
}

@UiThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,16 @@ public void execute(@NonNull MountingManager mountingManager) {
} else if (type == INSTRUCTION_UPDATE_STATE) {
mountingManager.updateState(mIntBuffer[i++], castToState(mObjBuffer[j++]));
} else if (type == INSTRUCTION_UPDATE_LAYOUT) {
mountingManager.updateLayout(
mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);

int reactTag = mIntBuffer[i++];
int x = mIntBuffer[i++];
int y = mIntBuffer[i++];
int width = mIntBuffer[i++];
int height = mIntBuffer[i++];
// The final buffer, layoutDirection, seems unused?
i++;
int displayType = mIntBuffer[i++];
mountingManager.updateLayout(reactTag, x, y, width, height, displayType);

} else if (type == INSTRUCTION_UPDATE_PADDING) {
mountingManager.updatePadding(
mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);
Expand Down
11 changes: 11 additions & 0 deletions ReactCommon/react/renderer/core/conversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ inline int toInt(const LayoutDirection &layoutDirection) {
}
}

inline int toInt(const DisplayType &displayType) {
switch (displayType) {
case DisplayType::None:
return 0;
case DisplayType::Flex:
return 1;
case DisplayType::Inline:
return 2;
}
}

inline std::string toString(const DisplayType &displayType) {
switch (displayType) {
case DisplayType::None:
Expand Down

0 comments on commit 4076293

Please sign in to comment.