Skip to content

Commit

Permalink
Fix visibilityChanged event MountState calculations
Browse files Browse the repository at this point in the history
Summary:
The visibilityChanged event is using the incorrect rectangles to calculate percent visibility. This is visible in the test added and also affects nested LithoView situations (like when used in a VeriticalScroll component).

The old calculation used the layoutState and localVisibleRect

The new calculation uses the visibilityOutputBounds and the intersection of the visibilityOutputBounds and localVisibleRect

See D7377488 where I made a similar fix for the visibleRatio calculation
 ---
`localVisibleRect` represents the full screen (in the test, Rect(left=0, top=0, right=10, bottom=1000))
`visibilityOutputBounds` represents the renderedComponent (in the test, Rect(left=0, top=0, right=10, bottom=10)

`sTempRect` represents the intersection of localVisibleRect and visibilityOutputBounds, essentially the part of the component that is visible on screen (in the initial mount of the test, Rect(left=0, top=0, right=10, bottom=10)

Therefore it follows if we want to calculate the amount on screen, we use `sTempRect` to get the width/height of the visible component and then divide by visibilityOutputBounds to get the percent of the component that is visible.

Reviewed By: mihaelao

Differential Revision: D14377550

fbshipit-source-id: 8e98c53f07bbbf1023ea71fbd8dfe82dee02dce1
  • Loading branch information
Brock Overcash authored and facebook-github-bot committed Mar 8, 2019
1 parent a0d2378 commit 66d65fe
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
8 changes: 4 additions & 4 deletions litho-core/src/main/java/com/facebook/litho/MountState.java
Original file line number Diff line number Diff line change
Expand Up @@ -637,14 +637,14 @@ void processVisibilityOutputs(
}

if (visibilityChangedHandler != null) {
final int visibleWidth = localVisibleRect.right - localVisibleRect.left;
final int visibleHeight = localVisibleRect.bottom - localVisibleRect.top;
final int visibleWidth = sTempRect.right - sTempRect.left;
final int visibleHeight = sTempRect.bottom - sTempRect.top;
EventDispatcherUtils.dispatchOnVisibilityChanged(
visibilityChangedHandler,
visibleWidth,
visibleHeight,
100f * visibleWidth / layoutState.getWidth(),
100f * visibleHeight / layoutState.getHeight());
100f * visibleWidth / visibilityOutputBounds.width(),
100f * visibleHeight / visibilityOutputBounds.height());
}
}
if (isDoingPerfLog) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,60 @@ public void testVisibleRectChangedEventItemNotVisible() {
assertThat(visibilityChangedEvent.percentVisibleWidth).isEqualTo(0.0f);
}

@Test
public void testVisibleRectChangedEventLargeView() {
final TestComponent content = create(mContext).build();
final EventHandler<VisibilityChangedEvent> visibilityChangedHandler =
new EventHandler<>(content, 3);

final LithoView lithoView =
mountComponent(
mContext,
mLithoView,
Column.create(mContext)
.child(
Wrapper.create(mContext)
.delegate(content)
.visibilityChangedHandler(visibilityChangedHandler)
.widthPx(10)
.heightPx(10))
.build(),
true,
10,
1000);

VisibilityChangedEvent visibilityChangedEvent =
(VisibilityChangedEvent) content.getEventState(visibilityChangedHandler);
assertThat(visibilityChangedEvent.visibleHeight).isEqualTo(10);
assertThat(visibilityChangedEvent.visibleWidth).isEqualTo(10);
assertThat(visibilityChangedEvent.percentVisibleHeight).isEqualTo(100f);
assertThat(visibilityChangedEvent.percentVisibleWidth).isEqualTo(100f);

content.getDispatchedEventHandlers().clear();

lithoView.performIncrementalMount(new Rect(LEFT, 0, RIGHT, 4), true);
assertThat(content.getDispatchedEventHandlers()).contains(visibilityChangedHandler);

visibilityChangedEvent =
(VisibilityChangedEvent) content.getEventState(visibilityChangedHandler);

assertThat(visibilityChangedEvent.visibleHeight).isEqualTo(4);
assertThat(visibilityChangedEvent.visibleWidth).isEqualTo(10);
assertThat(visibilityChangedEvent.percentVisibleHeight).isEqualTo(40f);
assertThat(visibilityChangedEvent.percentVisibleWidth).isEqualTo(100f);

lithoView.performIncrementalMount(new Rect(LEFT, 0, RIGHT, 5), true);
assertThat(content.getDispatchedEventHandlers()).contains(visibilityChangedHandler);

visibilityChangedEvent =
(VisibilityChangedEvent) content.getEventState(visibilityChangedHandler);

assertThat(visibilityChangedEvent.visibleHeight).isEqualTo(5);
assertThat(visibilityChangedEvent.visibleWidth).isEqualTo(10);
assertThat(visibilityChangedEvent.percentVisibleHeight).isEqualTo(50f);
assertThat(visibilityChangedEvent.percentVisibleWidth).isEqualTo(100f);
}

@Test
public void testVisibleAndInvisibleEvents() {
final TestComponent content = create(mContext).build();
Expand Down

0 comments on commit 66d65fe

Please sign in to comment.