Add view visibility checks to EpoxyVisibilityItem and decouple Recycl…

…erView (#1052) * Add view visibility checks for EpoxyVisibilityItem Make EpoxyVisibilityItem more generic so it can work with a ViewGroup instead of just a RecyclerView Fix docs in OnVisibilityStateChanged * Add more tests * Add last visibility check Co-authored-by: Peter Elliott <[email protected]>
airbnb · Sep 17, 2020 · 6b243e0 · 6b243e0
1 parent f5f3dce
commit 6b243e0
Show file tree

Hide file tree

Showing 7 changed files with 482 additions and 20 deletions.
diff --git a/blessedDeps.gradle b/blessedDeps.gradle
@@ -66,7 +66,8 @@ rootProject.ext.deps = [
     androidArchCoreTesting : "android.arch.core:core-testing:$ANDROID_ARCH_TESTING",
     androidTestRunner      : "com.android.support.test:runner:$ANDROID_TEST_RUNNER",
     androidAnnotations     : "androidx.annotation:annotation:$ANDROIDX_ANNOTATION",
-    androidTestCore        : "androidx.test:core:1.2.0",
+    androidTestCore        : "androidx.test:core:1.3.0",
+    androidTestExtJunitKtx : "androidx.test.ext:junit-ktx:1.1.2",
     androidLegacy          : "androidx.legacy:legacy-support-v4:$ANDROIDX_LEGACY",
     versionedParcelable    : "androidx.versionedparcelable:versionedparcelable:$ANDROIDX_VERSIONED_PARCELABLE",
     dataBindingAdapters    : "androidx.databinding:databinding-adapters:$ANDROIDX_DATABINDING_ADAPTERS",

diff --git a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityItem.java b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityItem.java
@@ -2,16 +2,17 @@
 
 import android.graphics.Rect;
 import android.view.View;
+import android.view.ViewGroup;
 
 import androidx.annotation.IntRange;
 import androidx.annotation.NonNull;
 import androidx.annotation.Px;
 import androidx.recyclerview.widget.RecyclerView;
 
 /**
- * This class represent an item in the {@link androidx.recyclerview.widget.RecyclerView} and it is
+ * This class represent an item in a {@link android.view.ViewGroup} 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 androidx.recyclerview.widget.RecyclerView}.
+ * EpoxyVisibilityItem and a child within the {@link android.view.ViewGroup}.
  *
  * 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}
@@ -46,10 +47,15 @@ class EpoxyVisibilityItem {
   private boolean fullyVisible = false;
   private boolean visible = false;
   private boolean focusedVisible = false;
+  private int viewVisibility = View.GONE;
 
   /** Store last value for de-duping */
   private int lastVisibleHeightNotified = NOT_NOTIFIED;
   private int lastVisibleWidthNotified = NOT_NOTIFIED;
+  private int lastVisibilityNotified = NOT_NOTIFIED;
+
+  EpoxyVisibilityItem() {
+  }
 
   EpoxyVisibilityItem(int adapterPosition) {
     reset(adapterPosition);
@@ -59,10 +65,10 @@ class EpoxyVisibilityItem {
    * Update the visibility item according the current layout.
    *
    * @param view        the current {@link com.airbnb.epoxy.EpoxyViewHolder}'s itemView
-   * @param parent      the {@link androidx.recyclerview.widget.RecyclerView}
+   * @param parent      the {@link android.view.ViewGroup}
    * @return true if the view has been measured
    */
-  boolean update(@NonNull View view, @NonNull RecyclerView parent, boolean detachEvent) {
+  boolean update(@NonNull View view, @NonNull ViewGroup parent, boolean detachEvent) {
     // Clear the rect before calling getLocalVisibleRect
     localVisibleRect.setEmpty();
     boolean viewDrawn = view.getLocalVisibleRect(localVisibleRect) && !detachEvent;
@@ -72,6 +78,7 @@ boolean update(@NonNull View view, @NonNull RecyclerView parent, boolean detachE
     viewportWidth = parent.getWidth();
     visibleHeight = viewDrawn ? localVisibleRect.height() : 0;
     visibleWidth = viewDrawn ? localVisibleRect.width() : 0;
+    viewVisibility = view.getVisibility();
     return height > 0 && width > 0;
   }
 
@@ -86,6 +93,7 @@ void reset(int newAdapterPosition) {
     adapterPosition = newAdapterPosition;
     lastVisibleHeightNotified = NOT_NOTIFIED;
     lastVisibleWidthNotified = NOT_NOTIFIED;
+    lastVisibilityNotified = NOT_NOTIFIED;
   }
 
   void handleVisible(@NonNull EpoxyViewHolder epoxyHolder, boolean detachEvent) {
@@ -137,23 +145,29 @@ void handleFullImpressionVisible(EpoxyViewHolder epoxyHolder, boolean detachEven
 
   boolean handleChanged(EpoxyViewHolder epoxyHolder, boolean visibilityChangedEnabled) {
     boolean changed = false;
-    if (visibleHeight != lastVisibleHeightNotified || visibleWidth != lastVisibleWidthNotified) {
+    if (visibleHeight != lastVisibleHeightNotified || visibleWidth != lastVisibleWidthNotified
+        || viewVisibility != lastVisibilityNotified) {
       if (visibilityChangedEnabled) {
-        epoxyHolder.visibilityChanged(
-            100.f / height * visibleHeight,
-            100.f / width * visibleWidth,
-            visibleHeight, visibleWidth
-        );
+        if (viewVisibility == View.GONE) {
+          epoxyHolder.visibilityChanged(0f, 0f, 0, 0);
+        } else {
+          epoxyHolder.visibilityChanged(
+              100.f / height * visibleHeight,
+              100.f / width * visibleWidth,
+              visibleHeight, visibleWidth
+          );
+        }
       }
       lastVisibleHeightNotified = visibleHeight;
       lastVisibleWidthNotified = visibleWidth;
+      lastVisibilityNotified = viewVisibility;
       changed = true;
     }
     return changed;
   }
 
   private boolean isVisible() {
-    return visibleHeight > 0 && visibleWidth > 0;
+    return viewVisibility == View.VISIBLE && visibleHeight > 0 && visibleWidth > 0;
   }
 
   private boolean isInFocusVisible() {
@@ -163,9 +177,10 @@ private boolean isInFocusVisible() {
     // The model has entered the focused range either if it is larger than half of the viewport
     // and it occupies at least half of the viewport or if it is smaller than half of the viewport
     // and it is fully visible.
-    return (totalArea >= halfViewportArea)
-        ? (visibleArea >= halfViewportArea)
-        : totalArea == visibleArea;
+    return viewVisibility == View.VISIBLE &&
+        ((totalArea >= halfViewportArea)
+            ? (visibleArea >= halfViewportArea)
+            : totalArea == visibleArea);
   }
 
   private boolean isPartiallyVisible(@IntRange(from = 0, to = 100) int thresholdPercentage) {
@@ -176,11 +191,11 @@ private boolean isPartiallyVisible(@IntRange(from = 0, to = 100) int thresholdPe
     final int visibleArea = visibleHeight * visibleWidth;
     final float visibleAreaPercentage = (visibleArea / (float) totalArea) * 100;
 
-    return visibleAreaPercentage >= thresholdPercentage;
+    return viewVisibility == View.VISIBLE && visibleAreaPercentage >= thresholdPercentage;
   }
 
   private boolean isFullyVisible() {
-    return visibleHeight == height && visibleWidth == width;
+    return viewVisibility == View.VISIBLE && visibleHeight == height && visibleWidth == width;
   }
 
   void shiftBy(int offsetPosition) {

diff --git a/epoxy-annotations/src/main/java/com/airbnb/epoxy/OnVisibilityStateChanged.java b/epoxy-annotations/src/main/java/com/airbnb/epoxy/OnVisibilityStateChanged.java
@@ -10,10 +10,10 @@
  * with this annotation will be called when the visibility state is changed.
  * <p>
  * Annotated methods should follow this signature :
- * `@OnVisibilityStateChange
- * public void method(@VisibilityState int state)`
+ * `@OnVisibilityStateChanged
+ * public void method(@Visibility int state)`
  * <p>
- * Possible States are declared in {@link com.airbnb.epoxy.OnModelVisibilityStateChangedListener}.
+ * Possible States are declared in {@link com.airbnb.epoxy.VisibilityState}.
  * <p>
  * The equivalent methods on the model is
  * {@link com.airbnb.epoxy.EpoxyModel#onVisibilityStateChanged}

diff --git a/epoxy-integrationtest/build.gradle b/epoxy-integrationtest/build.gradle
@@ -56,6 +56,10 @@ dependencies {
   testImplementation rootProject.deps.mockito
   testImplementation rootProject.deps.mockito_inline
   testImplementation rootProject.deps.androidTestCore
+  testImplementation rootProject.deps.androidTestExtJunitKtx
+  testImplementation('androidx.test.espresso:espresso-core:3.1.0-alpha2', {
+    exclude group: 'com.android.support', module: 'support-annotations'
+  })
 }
 
 repositories {

diff --git a/epoxy-integrationtest/src/main/AndroidManifest.xml b/epoxy-integrationtest/src/main/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:label="@string/app_name"
         android:theme="@style/Theme.AppCompat.Light">
 
+        <activity android:name=".TestActivity"/>
+
     </application>
 
 </manifest>
diff --git a/epoxy-integrationtest/src/main/java/com/airbnb/epoxy/integrationtest/TestActivity.kt b/epoxy-integrationtest/src/main/java/com/airbnb/epoxy/integrationtest/TestActivity.kt
@@ -0,0 +1,8 @@
+package com.airbnb.epoxy.integrationtest
+
+import android.app.Activity
+
+/**
+ * Empty activity used for view testing.
+ */
+class TestActivity : Activity()