Skip to content

Commit

Permalink
Fix not processing Visibility events on parent LithoView when it has …
Browse files Browse the repository at this point in the history
…transient child LithoViews

Summary:
We process our visibility evens in 2 cases: `Extension.afterMount` or `Extension.onVisibleBoundsChanged`. In both cases if the view is in transient state, we skip processing until it's not. We rely on `setHasTransientState(false)` to be called eventually to return the view back to non transient state. But in realty `setHasTransientState(true)` is not the only was to make a view transient. It transitively becomes transient, when any of its children becomes transient. This was the case in the issue that uncovered this bug — child LithoViews sometimes were in transient state in `afterMount` or `onVisibleBoundsChanged`, resulting in parent LithoView skipping processing visibility. But since the parent view was never marked as transient itself, it never had `setHasTransientState(false)` called as well, and we didn't process visibility until the next  `afterMount` or `onVisibleBoundsChanged` when all children were back to non-transient state.

View has a dedicated `childHasTransientStateChanged` to be notified about its children transient state changes, which is being hooked up in this diff to trigger visibility processing when parent's effective transient state is back to `false`.

Reviewed By: adityasharat

Differential Revision: D47841143

fbshipit-source-id: 9a65469ac09af23bb2f2586539b074f5cc5cb1eb
  • Loading branch information
colriot authored and facebook-github-bot committed Jul 28, 2023
1 parent 224bf06 commit dc8a478
Showing 1 changed file with 28 additions and 0 deletions.
28 changes: 28 additions & 0 deletions litho-core/src/main/java/com/facebook/litho/BaseMountingView.java
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,34 @@ public void setAnimatedHeight(int height) {
requestLayout();
}

@Override
public void childHasTransientStateChanged(View child, boolean childHasTransientState) {
final boolean oldHasTransientState = hasTransientState();
super.childHasTransientStateChanged(child, childHasTransientState);
final boolean newHasTransientState = hasTransientState();

if (oldHasTransientState == newHasTransientState) {
return;
}

if (mTransientStateCount != 0) {
return;
}

if (!hasTree()) {
return;
}

if (newHasTransientState) {
notifyVisibleBoundsChanged(new Rect(0, 0, getWidth(), getHeight()), false);
} else {
// We mounted everything when the transient state was set on this view. We need to do this
// partly to unmount content that is not visible but mostly to get the correct visibility
// events to be fired.
notifyVisibleBoundsChanged();
}
}

@Override
public void setHasTransientState(boolean hasTransientState) {
super.setHasTransientState(hasTransientState);
Expand Down

0 comments on commit dc8a478

Please sign in to comment.