Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements visibility events for EpoxyRecyclerView/EpoxyViewHolder. #560

Merged
merged 42 commits into from
Oct 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
ef42810
Implements visibility events for EpoxyRecyclerView/EpoxyViewHolder.
eboudrant Sep 27, 2018
a0fe6a4
checkstyle failures
eboudrant Oct 2, 2018
03fc799
Fix existing tests
eboudrant Oct 2, 2018
5b175d2
Send visibleSize instead of size, prevent sending duplicate events.
eboudrant Oct 3, 2018
3b29fe8
Remove useless private methods
eboudrant Oct 3, 2018
33757a9
Add visibility events in sample app.
eboudrant Oct 3, 2018
7727d1f
Replace `@OnVisibilityEvent(event = ...)` by `@OnVisibilityChanged` a…
eboudrant Oct 4, 2018
625b76d
Add the visibility listener to the model code generation
eboudrant Oct 4, 2018
a080b51
Update test resources (not sure why but the ruby script reformatted s…
eboudrant Oct 4, 2018
51fb99f
Send unfocused event on detach event
eboudrant Oct 8, 2018
16d71f6
Add float ranges
eboudrant Oct 10, 2018
62478ba
Rename to sizeInScrollingDirection;
eboudrant Oct 10, 2018
aa39ae2
Can be private
eboudrant Oct 10, 2018
cd5619f
typo
eboudrant Oct 10, 2018
25ce009
Remove dependency to LinearLayoutManager
eboudrant Oct 10, 2018
c3f5e2e
Remove dependency to EpoxyRecyclerView. Make visibility tracker public.
eboudrant Oct 10, 2018
b1bcc9f
parameter names inverted
eboudrant Oct 10, 2018
22d5126
Add toto regarding nested list (carousel)
eboudrant Oct 10, 2018
4f2e2eb
pr comments
eboudrant Oct 10, 2018
38267bb
visibility...() -> onVisibility...()
eboudrant Oct 10, 2018
85fa891
visibilityStateParam -> visibilityObjectParam
eboudrant Oct 10, 2018
a965ab1
Add optional checkTypeParameters in validateExecutableElement, use it…
eboudrant Oct 10, 2018
f96ce25
Merge pull request #4 from eboudrant/feature/visibility_changed_annot…
eboudrant Oct 10, 2018
ecc85c2
throws if not instanceof EpoxyViewHolder
eboudrant Oct 10, 2018
f3f4440
javadoc + renaming on EpoxyVisibilityItem
eboudrant Oct 10, 2018
0f81812
EpoxyModel<?> -> EpoxyModel<V>
eboudrant Oct 10, 2018
323dc69
Javadoc, links to the epoxy model methods
eboudrant Oct 10, 2018
2d26aaf
Remove mention to "You may clear the listener by..." in javadoc
eboudrant Oct 10, 2018
f54178c
pr comments
eboudrant Oct 11, 2018
e780c0a
More javadoc
eboudrant Oct 11, 2018
e2d675e
Move state constants to top level class VisibilityState
eboudrant Oct 11, 2018
b3b40d3
pr comments
eboudrant Oct 11, 2018
0ca9771
Unused import
eboudrant Oct 11, 2018
ad083de
Add 3 tests : load RV, scroll to, scroll by.
eboudrant Oct 11, 2018
7a1b97e
Fix bugs (thanks to unit tests)
eboudrant Oct 11, 2018
7db96a9
Missing annotations (FloatRange, Px)
eboudrant Oct 12, 2018
e150b9d
Tests for @ModelView's @OnVisibilityChanged @OnVisibilityStateChanged…
eboudrant Oct 12, 2018
bae5693
Rename `is...()` to `checkAndUpdate...()`
eboudrant Oct 12, 2018
4eda5c9
Use constant
eboudrant Oct 12, 2018
44ed7e5
Add basic javadoc to EpoxyModel + todo
eboudrant Oct 12, 2018
d042c68
Prepare unit test for insert/delete case (disabled as failing)
eboudrant Oct 12, 2018
e975337
remove dup comment
eboudrant Oct 12, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions epoxy-adapter/build.gradle
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
apply plugin: 'com.android.library'
apply plugin: 'kotlin-android'

android {
compileSdkVersion rootProject.COMPILE_SDK_VERSION
Expand All @@ -25,6 +26,7 @@ dependencies {

annotationProcessor project(':epoxy-processor')

testImplementation rootProject.deps.kotlin
testCompile rootProject.deps.junit
testCompile rootProject.deps.robolectric
testCompile rootProject.deps.mockito
Expand Down
23 changes: 23 additions & 0 deletions epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyModel.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package com.airbnb.epoxy;

import android.support.annotation.FloatRange;
import android.support.annotation.LayoutRes;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.Px;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;

import com.airbnb.epoxy.EpoxyController.ModelInterceptorCallback;
import com.airbnb.epoxy.VisibilityState.Visibility;

import java.util.List;

Expand Down Expand Up @@ -155,6 +158,26 @@ public void bind(@NonNull T view, @NonNull EpoxyModel<?> previouslyBoundModel) {
public void unbind(@NonNull T view) {
}

/**
* TODO link to the wiki
* @see OnVisibilityStateChanged annotation
*/
public void onVisibilityStateChanged(@Visibility int visibilityState, @NonNull T view) {
eboudrant marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* TODO link to the wiki
* @see OnVisibilityChanged annotation
*/
public void onVisibilityChanged(
@FloatRange(from = 0.0f, to = 100.0f) float percentVisibleHeight,
@FloatRange(from = 0.0f, to = 100.0f) float percentVisibleWidth,
@Px int visibleHeight,
@Px int visibleWidth,
@NonNull T view
) {
}

public long id() {
return id;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.airbnb.epoxy;

import android.support.annotation.FloatRange;
import android.support.annotation.NonNull;
import android.support.annotation.Px;

import com.airbnb.epoxy.VisibilityState.Visibility;

import java.util.List;

Expand Down Expand Up @@ -40,6 +44,24 @@ public void unbind(@NonNull T holder) {
super.unbind(holder);
}


@Override
public void onVisibilityStateChanged(@Visibility int visibilityState, @NonNull T view) {
super.onVisibilityStateChanged(visibilityState, view);
}

@Override
public void onVisibilityChanged(
@FloatRange(from = 0, to = 100) float percentVisibleHeight,
@FloatRange(from = 0, to = 100) float percentVisibleWidth,
@Px int visibleHeight, @Px int visibleWidth,
@NonNull T view) {
super.onVisibilityChanged(
percentVisibleHeight, percentVisibleWidth,
visibleHeight, visibleWidth,
view);
}

@Override
public boolean onFailedToRecycleView(T holder) {
return super.onFailedToRecycleView(holder);
Expand Down
22 changes: 22 additions & 0 deletions epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyViewHolder.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@

package com.airbnb.epoxy;

import android.support.annotation.FloatRange;
import android.support.annotation.Nullable;
import android.support.annotation.Px;
import android.support.v7.widget.RecyclerView;
import android.view.View;

import com.airbnb.epoxy.ViewHolderState.ViewState;
import com.airbnb.epoxy.VisibilityState.Visibility;

import java.util.List;

Expand Down Expand Up @@ -82,6 +85,25 @@ public void unbind() {
payloads = null;
}

public void visibilityStateChanged(@Visibility int visibilityState) {
assertBound();
// noinspection unchecked
epoxyModel.onVisibilityStateChanged(visibilityState, objectToBind());
}

public void visibilityChanged(
@FloatRange(from = 0.0f, to = 100.0f) float percentVisibleHeight,
@FloatRange(from = 0.0f, to = 100.0f) float percentVisibleWidth,
@Px int visibleHeight,
@Px int visibleWidth
) {
assertBound();
// noinspection unchecked
epoxyModel.onVisibilityChanged(percentVisibleHeight, percentVisibleWidth, visibleHeight,
visibleWidth, objectToBind());
}


public List<Object> getPayloads() {
assertBound();
return payloads;
Expand Down
177 changes: 177 additions & 0 deletions epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityItem.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
package com.airbnb.epoxy;

import android.graphics.Rect;
import android.support.annotation.NonNull;
import android.support.annotation.Px;
import android.support.v7.widget.RecyclerView;
import android.view.View;

/**
* This class represent an item in the {@link android.support.v7.widget.RecyclerView} and it is
* being reused with multiple model via the update method. There is 1:1 relationship between an
* EpoxyVisibilityItem and a child within the {@link android.support.v7.widget.RecyclerView}.
*
* It contains the logic to compute the visibility state of an item. It will also invoke the
* visibility callbacks on {@link com.airbnb.epoxy.EpoxyViewHolder}
*
* This class should remain non-public and is intended to be used by {@link EpoxyVisibilityTracker}
* only.
*/
class EpoxyVisibilityItem {
eboudrant marked this conversation as resolved.
Show resolved Hide resolved

private static final int NOT_NOTIFIED = -1;

private final Rect localVisibleRect = new Rect();

private int adapterPosition = RecyclerView.NO_POSITION;

@Px
private int sizeInScrollingDirection;

private int sizeNotInScrollingDirection;

private boolean verticalScrolling;

private float percentVisibleSize = 0.f;

private int visibleSize;

private int viewportSize;

private boolean fullyVisible = false;
private boolean visible = false;
private boolean focusedVisible = false;

/** Store last value for de-duping */
private int lastVisibleSizeNotified = NOT_NOTIFIED;

/**
* Update the visibility item according the current layout.
*
* @param view the current {@link com.airbnb.epoxy.EpoxyViewHolder}'s itemView
* @param parent the {@link android.support.v7.widget.RecyclerView}
* @param vertical true if it scroll vertically
* @return true if the view has been measured
*/
boolean update(@NonNull View view, @NonNull RecyclerView parent,
boolean vertical, boolean detachEvent) {
view.getLocalVisibleRect(localVisibleRect);
this.verticalScrolling = vertical;
if (vertical) {
sizeInScrollingDirection = view.getMeasuredHeight();
sizeNotInScrollingDirection = view.getMeasuredWidth();
viewportSize = parent.getMeasuredHeight();
visibleSize = detachEvent ? 0 : localVisibleRect.height();
} else {
sizeNotInScrollingDirection = view.getMeasuredHeight();
sizeInScrollingDirection = view.getMeasuredWidth();
viewportSize = parent.getMeasuredWidth();
visibleSize = detachEvent ? 0 : localVisibleRect.width();
}
percentVisibleSize = detachEvent ? 0 : 100.f / sizeInScrollingDirection * visibleSize;
if (visibleSize != sizeInScrollingDirection) {
fullyVisible = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: i realized that fullyVisible is tracked so we don't notify the event twice (at least that is my understanding) - maybe that could be renamed to hasNotifiedFullyVisible?

additionally it seems more clear and safer to set this field explicitly where we do the notify call

original:
fullyVisible is also set in isFullImpressionVisible. seems it should be only set in one place

even better, isFullImpressionVisible seems to be derived only from visibleSize != sizeInScrollingDirection which are also stored in fields, instead of storing data derived from that that could get out of sync can we just do this?

boolean isFullImpressionVisible() {
 return  visibleSize == sizeInScrollingDirection
}

}
return sizeInScrollingDirection > 0;
}

int getAdapterPosition() {
return adapterPosition;
}

void reset(int newAdapterPosition) {
fullyVisible = false;
visible = false;
focusedVisible = false;
adapterPosition = newAdapterPosition;
lastVisibleSizeNotified = NOT_NOTIFIED;
}

void handleVisible(@NonNull EpoxyViewHolder epoxyHolder, boolean detachEvent) {
if (visible && checkAndUpdateInvisible(detachEvent)) {
epoxyHolder.visibilityStateChanged(VisibilityState.INVISIBLE);
} else if (!visible && checkAndUpdateVisible()) {
epoxyHolder.visibilityStateChanged(VisibilityState.VISIBLE);
}
}

void handleFocus(EpoxyViewHolder epoxyHolder, boolean detachEvent) {
if (focusedVisible && checkAndUpdateUnfocusedVisible(detachEvent)) {
epoxyHolder.visibilityStateChanged(VisibilityState.UNFOCUSED_VISIBLE);
} else if (!focusedVisible && checkAndUpdateFocusedVisible()) {
epoxyHolder.visibilityStateChanged(VisibilityState.FOCUSED_VISIBLE);
}
}

void handleFullImpressionVisible(EpoxyViewHolder epoxyHolder, boolean detachEvent) {
if (!fullyVisible && checkAndUpdateFullImpressionVisible()) {
epoxyHolder
.visibilityStateChanged(VisibilityState.FULL_IMPRESSION_VISIBLE);
}
}

void handleChanged(EpoxyViewHolder epoxyHolder) {
if (visibleSize != lastVisibleSizeNotified) {
if (verticalScrolling) {
epoxyHolder.visibilityChanged(percentVisibleSize, 100.f, visibleSize,
sizeNotInScrollingDirection);
} else {
epoxyHolder.visibilityChanged(100.f, percentVisibleSize,
sizeNotInScrollingDirection, visibleSize);
}
lastVisibleSizeNotified = visibleSize;
}
}

/**
* @return true when at least one pixel of the component is visible
*/
private boolean checkAndUpdateVisible() {
return visible = visibleSize > 0;
}

/**
* @param detachEvent true if initiated from detach event
* @return true when when the component no longer has any pixels on the screen
*/
private boolean checkAndUpdateInvisible(boolean detachEvent) {
boolean invisible = visibleSize <= 0 || detachEvent;
if (invisible) {
visible = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not ideal that what would appear to be a simple getter has the side effect of updating the visible field. maybe this method can be named to reflect that?

alternatively I feel that tracking both visible and visibleSize could be redundant - what if we could derive isVisible() purely from visibleSize? -1 or null could represent detached.

I see that update already sets visibleSize to 0 if detachEvent is true, so the logic in this method feels unnecessary

Same thoughts apply to isUnfocusedVisible

}
return !visible;
}

/**
* @return true when either the component occupies at least half of the viewport, or, if the
* component is smaller than half the viewport, when it is fully visible.
*/
private boolean checkAndUpdateFocusedVisible() {
return focusedVisible =
sizeInScrollingDirection >= viewportSize / 2 || (visibleSize == sizeInScrollingDirection
&& sizeInScrollingDirection < viewportSize / 2);
}

/**
* @param detachEvent true if initiated from detach event
* @return true when the component is no longer focused, i.e. it is not fully visible and does
* not occupy at least half the viewport.
*/
private boolean checkAndUpdateUnfocusedVisible(boolean detachEvent) {
boolean unfocusedVisible = detachEvent
|| !(sizeInScrollingDirection >= viewportSize / 2 || (
visibleSize == sizeInScrollingDirection && sizeInScrollingDirection < viewportSize / 2));
if (unfocusedVisible) {
focusedVisible = false;
}
return !focusedVisible;
}

/**
* @return true when the entire component has passed through the viewport at some point.
*/
private boolean checkAndUpdateFullImpressionVisible() {
return fullyVisible = visibleSize == sizeInScrollingDirection;
}
}

Loading