From 6b243e0b23ae75441bd6cfaaed17b0932951ba76 Mon Sep 17 00:00:00 2001 From: Peter Elliott Date: Thu, 17 Sep 2020 10:37:56 -0600 Subject: [PATCH] Add view visibility checks to EpoxyVisibilityItem and decouple RecyclerView (#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 --- blessedDeps.gradle | 3 +- .../com/airbnb/epoxy/EpoxyVisibilityItem.java | 47 +- .../epoxy/OnVisibilityStateChanged.java | 6 +- epoxy-integrationtest/build.gradle | 4 + .../src/main/AndroidManifest.xml | 2 + .../epoxy/integrationtest/TestActivity.kt | 8 + .../airbnb/epoxy/EpoxyVisibilityItemTest.kt | 432 ++++++++++++++++++ 7 files changed, 482 insertions(+), 20 deletions(-) create mode 100644 epoxy-integrationtest/src/main/java/com/airbnb/epoxy/integrationtest/TestActivity.kt create mode 100644 epoxy-integrationtest/src/test/java/com/airbnb/epoxy/EpoxyVisibilityItemTest.kt diff --git a/blessedDeps.gradle b/blessedDeps.gradle index 2a1fd00941..4941d9f42d 100644 --- 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 index cf545eeb9a..e516de0108 100644 --- a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityItem.java +++ b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyVisibilityItem.java @@ -2,6 +2,7 @@ import android.graphics.Rect; import android.view.View; +import android.view.ViewGroup; import androidx.annotation.IntRange; import androidx.annotation.NonNull; @@ -9,9 +10,9 @@ 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 index abf8450025..579e968d0e 100644 --- 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. *

* Annotated methods should follow this signature : - * `@OnVisibilityStateChange - * public void method(@VisibilityState int state)` + * `@OnVisibilityStateChanged + * public void method(@Visibility int state)` *

- * Possible States are declared in {@link com.airbnb.epoxy.OnModelVisibilityStateChangedListener}. + * Possible States are declared in {@link com.airbnb.epoxy.VisibilityState}. *

* 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 index 8c5893179f..1102936868 100644 --- 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 index cdd755cc52..14a0365730 100644 --- 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"> + + 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 new file mode 100644 index 0000000000..ee09fa357f --- /dev/null +++ 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() diff --git a/epoxy-integrationtest/src/test/java/com/airbnb/epoxy/EpoxyVisibilityItemTest.kt b/epoxy-integrationtest/src/test/java/com/airbnb/epoxy/EpoxyVisibilityItemTest.kt new file mode 100644 index 0000000000..e72475ed8d --- /dev/null +++ b/epoxy-integrationtest/src/test/java/com/airbnb/epoxy/EpoxyVisibilityItemTest.kt @@ -0,0 +1,432 @@ +package com.airbnb.epoxy + +import android.app.Activity +import android.view.View +import android.widget.FrameLayout +import androidx.test.ext.junit.rules.activityScenarioRule +import androidx.test.runner.AndroidJUnit4 +import com.airbnb.epoxy.integrationtest.TestActivity +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.eq +import org.mockito.Mock +import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoInteractions +import org.mockito.Mockito.verifyNoMoreInteractions +import org.mockito.MockitoAnnotations +import org.robolectric.annotation.LooperMode + +@RunWith(AndroidJUnit4::class) +@LooperMode(LooperMode.Mode.LEGACY) +class EpoxyVisibilityItemTest { + + @get:Rule + var activityRule = activityScenarioRule() + + @Mock + lateinit var mockEpoxyHolder: EpoxyViewHolder + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + } + + @Test + fun testUpdate() { + activityRule.scenario.onActivity { + val frameLayout = FrameLayout(it).apply { + it.setContentView(this) + } + val view = View(frameLayout.context).apply { + layoutParams = FrameLayout.LayoutParams(100, 100) + frameLayout.addView(this) + } + + val item = EpoxyVisibilityItem() + val measured = item.update(view, frameLayout, false) + assertTrue(measured) + } + } + + @Test + fun testUpdate_visibilityGone() { + activityRule.scenario.onActivity { + val frameLayout = FrameLayout(it).apply { + it.setContentView(this) + } + val view = View(frameLayout.context).apply { + visibility = View.GONE + layoutParams = FrameLayout.LayoutParams(100, 100) + frameLayout.addView(this) + } + + val item = EpoxyVisibilityItem() + val measured = item.update(view, frameLayout, false) + assertFalse(measured) + } + } + + @Test + fun testUpdate_visibilityInvisible() { + activityRule.scenario.onActivity { + val frameLayout = FrameLayout(it).apply { + it.setContentView(this) + } + val view = View(frameLayout.context).apply { + visibility = View.INVISIBLE + layoutParams = FrameLayout.LayoutParams(100, 100) + frameLayout.addView(this) + } + + val item = EpoxyVisibilityItem() + val measured = item.update(view, frameLayout, false) + assertTrue(measured) + } + } + + @Test + fun testUpdate_noSize() { + activityRule.scenario.onActivity { + val frameLayout = FrameLayout(it).apply { + it.setContentView(this) + } + val view = View(frameLayout.context).apply { + layoutParams = FrameLayout.LayoutParams(0, 0) + frameLayout.addView(this) + } + + val item = EpoxyVisibilityItem() + val measured = item.update(view, frameLayout, false) + assertFalse(measured) + } + } + + @Test + fun testHandleVisible() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleVisible(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.VISIBLE)) + } + } + + @Test + fun testHandleVisible_subsequentCall() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleVisible(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.VISIBLE)) + item.handleVisible(mockEpoxyHolder, false) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleVisible_viewInitiallyGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPreViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleVisible(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleVisible_viewTransitionToGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleVisible(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFocus() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleFocus(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.FOCUSED_VISIBLE)) + } + } + + @Test + fun testHandleFocus_subsequentCall() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleFocus(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.FOCUSED_VISIBLE)) + item.handleFocus(mockEpoxyHolder, false) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFocus_viewInitiallyGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPreViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleFocus(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFocus_viewTransitionToGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleFocus(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandlePartialImpressionVisible() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handlePartialImpressionVisible(mockEpoxyHolder, false, 50) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.PARTIAL_IMPRESSION_VISIBLE)) + } + } + + @Test + fun testHandlePartialImpressionVisible_subsequentCall() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handlePartialImpressionVisible(mockEpoxyHolder, false, 50) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.PARTIAL_IMPRESSION_VISIBLE)) + item.handlePartialImpressionVisible(mockEpoxyHolder, false, 50) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandlePartialImpressionVisible_viewInitiallyGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPreViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handlePartialImpressionVisible(mockEpoxyHolder, false, 100) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandlePartialImpressionVisible_viewTransitionToGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handlePartialImpressionVisible(mockEpoxyHolder, false, 100) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFullImpressionVisible() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleFullImpressionVisible(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.FULL_IMPRESSION_VISIBLE)) + } + } + + @Test + fun testHandleFullImpressionVisible_subsequentCall() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleFullImpressionVisible(mockEpoxyHolder, false) + verify(mockEpoxyHolder).visibilityStateChanged(eq(VisibilityState.FULL_IMPRESSION_VISIBLE)) + item.handleFullImpressionVisible(mockEpoxyHolder, false) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFullImpressionVisible_viewInitiallyGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPreViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleFullImpressionVisible(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleFullImpressionVisible_viewTransitionToGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleFullImpressionVisible(mockEpoxyHolder, false) + verifyNoInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleChanged() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(100f), eq(100f), eq(100), eq(100)) + } + } + + @Test + fun testHandleChanged_subsequentCall() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate() + + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(100f), eq(100f), eq(100), eq(100)) + item.handleChanged(mockEpoxyHolder, true) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + @Test + fun testHandleChanged_viewInitiallyGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPreViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(0f), eq(0f), eq(0), eq(0)) + } + } + + @Test + fun testHandleChanged_viewTransitionToGone() { + activityRule.scenario.onActivity { + val item = it.createViewAndUpdate( + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(0f), eq(0f), eq(0), eq(0)) + } + } + + @Test + fun testHandleChanged_goneTransitionsToVisible() { + activityRule.scenario.onActivity { + val wrapper = EpoxyVisibilityItemWrapper( + activity = it, + onPostViewAdded = { view -> + view.visibility = View.GONE + } + ) + val item = wrapper.item + + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(0f), eq(0f), eq(0), eq(0)) + + // Change visibility and update the item + wrapper.view.visibility = View.VISIBLE + item.update(wrapper.view, wrapper.rootLayout, false) + + // Ensure holder is is notified + item.handleChanged(mockEpoxyHolder, true) + verify(mockEpoxyHolder).visibilityChanged(eq(100f), eq(100f), eq(100), eq(100)) + verifyNoMoreInteractions(mockEpoxyHolder) + } + } + + /** + * This class wraps an [EpoxyVisibilityItem] and contains references to the views needed to + * perform subsequent [EpoxyVisibilityItem.update] calls. This will: + * * Create a [FrameLayout] and set it as the content view in the activity. + * * Create a [View] and add it to the [FrameLayout]. Default size is 100px x 100px. + * * Create an [EpoxyVisibilityItem] and call [EpoxyVisibilityItem.update] on it. + * + * @param onPreViewAdded invoked before the view is added to the [FrameLayout]. Use this to + * customize the view (e.g set visibility, change size, etc). + * @param onPostViewAdded invoked after the view is added to the [FrameLayout]. Use this to + * customize the view (e.g set visibility, change size, etc). + */ + private class EpoxyVisibilityItemWrapper( + activity: Activity, + detachEvent: Boolean = false, + onPreViewAdded: (View) -> (Unit) = {}, + onPostViewAdded: (View) -> (Unit) = {}, + ) { + + val rootLayout: FrameLayout = FrameLayout(activity) + val view: View + val item: EpoxyVisibilityItem + + init { + activity.setContentView(rootLayout) + view = View(rootLayout.context).apply { + layoutParams = FrameLayout.LayoutParams(100, 100) + onPreViewAdded.invoke(this) + rootLayout.addView(this) + onPostViewAdded.invoke(this) + } + + item = EpoxyVisibilityItem().apply { + update(view, rootLayout, detachEvent) + } + } + } + + /** + * Helper function that gets the [EpoxyVisibilityItem] from the [EpoxyVisibilityItemWrapper] + * for simple tests. + */ + private fun TestActivity.createViewAndUpdate( + detachEvent: Boolean = false, + onPreViewAdded: (View) -> (Unit) = {}, + onPostViewAdded: (View) -> (Unit) = {} + ): EpoxyVisibilityItem { + return EpoxyVisibilityItemWrapper(this, detachEvent, onPreViewAdded, onPostViewAdded).item + } +}