Create new PietScrollObserver to trigger visibility actions on first draw
PiperOrigin-RevId: 276285964
Change-Id: Ie28b8272e4a90990780c6ef919d1a37c3ec601f0
diff --git a/src/main/java/com/google/android/libraries/feed/basicstream/internal/viewholders/PietViewHolder.java b/src/main/java/com/google/android/libraries/feed/basicstream/internal/viewholders/PietViewHolder.java
index 880fbd5..a614431 100644
--- a/src/main/java/com/google/android/libraries/feed/basicstream/internal/viewholders/PietViewHolder.java
+++ b/src/main/java/com/google/android/libraries/feed/basicstream/internal/viewholders/PietViewHolder.java
@@ -18,7 +18,6 @@
import android.content.Context;
import android.support.annotation.VisibleForTesting;
-import android.support.v7.widget.RecyclerView;
import android.support.v7.widget.RecyclerView.LayoutParams;
import android.view.View;
import android.view.ViewGroup;
@@ -39,7 +38,7 @@
import com.google.android.libraries.feed.sharedstream.logging.VisibilityMonitor;
import com.google.android.libraries.feed.sharedstream.piet.PietEventLogger;
import com.google.android.libraries.feed.sharedstream.publicapi.scroll.ScrollObservable;
-import com.google.android.libraries.feed.sharedstream.publicapi.scroll.ScrollObserver;
+import com.google.android.libraries.feed.sharedstream.scroll.PietScrollObserver;
import com.google.android.libraries.feed.sharedstream.scroll.ScrollListenerNotifier;
import com.google.search.now.ui.action.FeedActionPayloadProto.FeedActionPayload;
import com.google.search.now.ui.piet.PietProto.Frame;
@@ -123,7 +122,8 @@
this.streamActionApi = streamActionApi;
this.swipeAction = swipeAction;
this.actionParser = actionParser;
- scrollObserver = new PietViewActionScrollObserver(frameAdapter, viewport, loggingListener);
+ scrollObserver =
+ new PietViewActionScrollObserver(frameAdapter, viewport, scrollObservable, loggingListener);
// Need to reset padding here. Setting a background can affect padding so if we switch from
// a background which has padding to one that does not, then the padding needs to be removed.
cardView.setPadding(0, 0, 0, 0);
@@ -171,6 +171,7 @@
visibilityMonitor.setListener(null);
if (scrollObserver != null) {
scrollObservable.removeScrollObserver(scrollObserver);
+ scrollObserver.uninstallFirstDrawTrigger();
}
bound = false;
}
@@ -217,33 +218,23 @@
actionParser, "Action parser can only be retrieved once view holder has been bound");
}
- static class PietViewActionScrollObserver implements ScrollObserver {
- private final FrameAdapter frameAdapter;
- private final View viewport;
+ static class PietViewActionScrollObserver extends PietScrollObserver {
private final LoggingListener loggingListener;
PietViewActionScrollObserver(
- FrameAdapter frameAdapter, View viewport, LoggingListener loggingListener) {
- this.frameAdapter = frameAdapter;
- this.viewport = viewport;
+ FrameAdapter frameAdapter,
+ View viewport,
+ ScrollObservable scrollObservable,
+ LoggingListener loggingListener) {
+ super(frameAdapter, viewport, scrollObservable);
this.loggingListener = loggingListener;
}
@Override
public void onScrollStateChanged(View view, String featureId, int newState, long timestamp) {
- // There was logic in here previously ensuring that the state had changed; however, you can
- // have a case where the feed is idle and you fling the feed, and it goes to idle again
- // without triggering the observer during scrolling. Just triggering this every time the feed
- // comes to rest appears to have the desired behavior. We can think about also triggering
- // while the feed is scrolling; not sure how frequently the observer triggers during scroll.
- if (newState == RecyclerView.SCROLL_STATE_IDLE) {
- frameAdapter.triggerViewActions(viewport);
- }
+ super.onScrollStateChanged(view, featureId, newState, timestamp);
loggingListener.onScrollStateChanged(
ScrollListenerNotifier.convertRecyclerViewScrollStateToListenerState(newState));
}
-
- @Override
- public void onScroll(View view, String featureId, int dx, int dy) {}
}
}
diff --git a/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD b/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
index 2919b45..d6e26dc 100644
--- a/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
+++ b/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
@@ -12,6 +12,7 @@
"//src/main/java/com/google/android/libraries/feed/common/concurrent",
"//src/main/java/com/google/android/libraries/feed/common/logging",
"//src/main/java/com/google/android/libraries/feed/common/time",
+ "//src/main/java/com/google/android/libraries/feed/piet",
"//src/main/java/com/google/android/libraries/feed/sharedstream/publicapi/scroll",
"//src/main/proto/com/google/android/libraries/feed/sharedstream/proto:scroll_state_java_proto_lite",
"@com_google_code_findbugs_jsr305//jar",
diff --git a/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserver.java b/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserver.java
new file mode 100644
index 0000000..ba8257a
--- /dev/null
+++ b/src/main/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserver.java
@@ -0,0 +1,111 @@
+// Copyright 2019 The Feed Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.android.libraries.feed.sharedstream.scroll;
+
+import android.support.v7.widget.RecyclerView;
+import android.view.View;
+import android.view.View.OnAttachStateChangeListener;
+import android.view.ViewTreeObserver.OnGlobalLayoutListener;
+import com.google.android.libraries.feed.piet.FrameAdapter;
+import com.google.android.libraries.feed.sharedstream.publicapi.scroll.ScrollObservable;
+import com.google.android.libraries.feed.sharedstream.publicapi.scroll.ScrollObserver;
+
+/** Scroll observer that triggers Piet scroll actions. */
+public class PietScrollObserver implements ScrollObserver {
+
+ private final FrameAdapter frameAdapter;
+ private final View viewport;
+ private final ScrollObservable scrollObservable;
+ private final FirstDrawTrigger firstDrawTrigger;
+
+ /**
+ * Construct a PietScrollObserver.
+ *
+ * @param frameAdapter Piet FrameAdapter which will have view actions triggered by this scroll
+ * observer
+ * @param viewport the view containing the frameAdapter, the bounds of which determine whether the
+ * frame is visible
+ * @param scrollObservable used to get the scroll state to ensure scrolling is idle before
+ * triggering
+ */
+ @SuppressWarnings("initialization") // Doesn't like the OnAttachStateChangeListener.
+ public PietScrollObserver(
+ FrameAdapter frameAdapter, View viewport, ScrollObservable scrollObservable) {
+ this.frameAdapter = frameAdapter;
+ this.viewport = viewport;
+ this.scrollObservable = scrollObservable;
+
+ firstDrawTrigger = new FirstDrawTrigger();
+
+ frameAdapter
+ .getFrameContainer()
+ .addOnAttachStateChangeListener(
+ new OnAttachStateChangeListener() {
+ @Override
+ public void onViewAttachedToWindow(View view) {
+ installFirstDrawTrigger();
+ }
+
+ @Override
+ public void onViewDetachedFromWindow(View view) {
+ uninstallFirstDrawTrigger();
+ }
+ });
+ }
+
+ @Override
+ public void onScrollStateChanged(View view, String featureId, int newState, long timestamp) {
+ // There was logic in here previously ensuring that the state had changed; however, you can
+ // have a case where the feed is idle and you fling the feed, and it goes to idle again
+ // without triggering the observer during scrolling. Just triggering this every time the feed
+ // comes to rest appears to have the desired behavior. We can think about also triggering
+ // while the feed is scrolling; not sure how frequently the observer triggers during scroll.
+ if (newState == RecyclerView.SCROLL_STATE_IDLE) {
+ frameAdapter.triggerViewActions(viewport);
+ }
+ }
+
+ @Override
+ public void onScroll(View view, String featureId, int dx, int dy) {
+ // no-op
+ }
+
+ /** Install the first-draw triggering observer. */
+ public void installFirstDrawTrigger() {
+ frameAdapter
+ .getFrameContainer()
+ .getViewTreeObserver()
+ .addOnGlobalLayoutListener(firstDrawTrigger);
+ }
+
+ /** Uninstall the first-draw triggering observer. */
+ public void uninstallFirstDrawTrigger() {
+ frameAdapter
+ .getFrameContainer()
+ .getViewTreeObserver()
+ .removeOnGlobalLayoutListener(firstDrawTrigger);
+ }
+
+ class FirstDrawTrigger implements OnGlobalLayoutListener {
+ @Override
+ public void onGlobalLayout() {
+ uninstallFirstDrawTrigger();
+
+ if (scrollObservable.getCurrentScrollState() == RecyclerView.SCROLL_STATE_IDLE) {
+ frameAdapter.triggerViewActions(viewport);
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD b/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
index aad383f..8725566 100644
--- a/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
+++ b/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/BUILD
@@ -3,6 +3,28 @@
licenses(["notice"]) # Apache 2
android_local_test(
+ name = "PietScrollObserverTest",
+ size = "small",
+ timeout = "moderate",
+ srcs = ["PietScrollObserverTest.java"],
+ manifest_values = DEFAULT_ANDROID_LOCAL_TEST_MANIFEST,
+ deps = [
+ "//src/main/java/com/google/android/libraries/feed/piet",
+ "//src/main/java/com/google/android/libraries/feed/piet/host",
+ "//src/main/java/com/google/android/libraries/feed/piet/testing",
+ "//src/main/java/com/google/android/libraries/feed/sharedstream/publicapi/scroll",
+ "//src/main/java/com/google/android/libraries/feed/sharedstream/scroll",
+ "//src/main/proto/search/now/ui/piet:piet_java_proto_lite",
+ "//third_party:robolectric",
+ "@com_google_protobuf_javalite//:protobuf_java_lite",
+ "@maven//:com_android_support_recyclerview_v7",
+ "@maven//:com_google_guava_guava",
+ "@maven//:org_mockito_mockito_core",
+ "@robolectric//bazel:android-all",
+ ],
+)
+
+android_local_test(
name = "ScrollListenerNotifierTest",
size = "small",
timeout = "moderate",
diff --git a/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserverTest.java b/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserverTest.java
new file mode 100644
index 0000000..704f25a
--- /dev/null
+++ b/src/test/java/com/google/android/libraries/feed/sharedstream/scroll/PietScrollObserverTest.java
@@ -0,0 +1,147 @@
+// Copyright 2019 The Feed Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.android.libraries.feed.sharedstream.scroll;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.same;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import android.app.Activity;
+import android.support.v7.widget.RecyclerView;
+import android.widget.FrameLayout;
+import android.widget.LinearLayout;
+import android.widget.LinearLayout.LayoutParams;
+import com.google.android.libraries.feed.piet.FrameAdapter;
+import com.google.android.libraries.feed.piet.host.ActionHandler;
+import com.google.android.libraries.feed.piet.host.ActionHandler.ActionType;
+import com.google.android.libraries.feed.piet.testing.FakeFrameAdapter;
+import com.google.android.libraries.feed.sharedstream.publicapi.scroll.ScrollObservable;
+import com.google.common.collect.ImmutableList;
+import com.google.search.now.ui.piet.ActionsProto.Action;
+import com.google.search.now.ui.piet.PietProto.Frame;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+
+/** Tests for {@link PietScrollObserverTest}. */
+@RunWith(RobolectricTestRunner.class)
+public class PietScrollObserverTest {
+
+ @Mock private ScrollObservable scrollObservable;
+ @Mock private ActionHandler actionHandler;
+
+ // .newBuilder().build() creates a unique instance we can check same() against.
+ private static final Action VIEW_ACTION = Action.newBuilder().build();
+
+ private FrameAdapter frameAdapter;
+ private FrameLayout viewport;
+ private LinearLayout frameView;
+ private PietScrollObserver pietScrollObserver;
+
+ @Before
+ public void setUp() {
+ initMocks(this);
+ Activity activity = Robolectric.setupActivity(Activity.class);
+ viewport = new FrameLayout(activity);
+ activity.getWindow().addContentView(viewport, new LayoutParams(100, 100));
+
+ frameAdapter =
+ FakeFrameAdapter.builder(activity)
+ .setActionHandler(actionHandler)
+ .addViewAction(VIEW_ACTION)
+ .build();
+ frameAdapter.bindModel(Frame.getDefaultInstance(), 0, null, ImmutableList.of());
+ frameView = frameAdapter.getFrameContainer();
+
+ pietScrollObserver = new PietScrollObserver(frameAdapter, viewport, scrollObservable);
+ }
+
+ @Test
+ public void testTriggersActionsWhenIdle() {
+ pietScrollObserver.onScrollStateChanged(viewport, "", RecyclerView.SCROLL_STATE_IDLE, 123L);
+
+ verify(actionHandler)
+ .handleAction(
+ same(VIEW_ACTION),
+ eq(ActionType.VIEW),
+ eq(Frame.getDefaultInstance()),
+ eq(frameAdapter.getFrameContainer()),
+ any() /* LogData */);
+ }
+
+ @Test
+ public void testDoesNotTriggerWhenScrolling() {
+ pietScrollObserver.onScrollStateChanged(viewport, "", RecyclerView.SCROLL_STATE_DRAGGING, 123L);
+
+ verifyZeroInteractions(actionHandler);
+ }
+
+ @Test
+ public void testTriggersOnFirstDraw() {
+ pietScrollObserver.installFirstDrawTrigger();
+ when(scrollObservable.getCurrentScrollState()).thenReturn(RecyclerView.SCROLL_STATE_IDLE);
+
+ frameView.getViewTreeObserver().dispatchOnGlobalLayout();
+
+ verify(actionHandler)
+ .handleAction(
+ same(VIEW_ACTION),
+ eq(ActionType.VIEW),
+ eq(Frame.getDefaultInstance()),
+ eq(frameView),
+ any() /* LogData */);
+ }
+
+ @Test
+ public void testDoesNotTriggerOnSecondDraw() {
+ pietScrollObserver.installFirstDrawTrigger();
+ when(scrollObservable.getCurrentScrollState()).thenReturn(RecyclerView.SCROLL_STATE_DRAGGING);
+
+ // trigger on global layout - actions will not trigger because scrolling is not IDLE
+ frameView.getViewTreeObserver().dispatchOnGlobalLayout();
+
+ when(scrollObservable.getCurrentScrollState()).thenReturn(RecyclerView.SCROLL_STATE_IDLE);
+
+ // trigger on global layout - actions will not trigger because observer has been removed.
+ frameView.getViewTreeObserver().dispatchOnGlobalLayout();
+
+ verifyZeroInteractions(actionHandler);
+ }
+
+ @Test
+ public void testTriggersOnAttach() {
+ // Actions will not fire before attached to window
+ frameView.getViewTreeObserver().dispatchOnGlobalLayout();
+ verifyZeroInteractions(actionHandler);
+
+ // Now attach to window and actions should fire
+ viewport.addView(frameView);
+ frameView.getViewTreeObserver().dispatchOnGlobalLayout();
+ verify(actionHandler)
+ .handleAction(
+ same(VIEW_ACTION),
+ eq(ActionType.VIEW),
+ eq(Frame.getDefaultInstance()),
+ eq(frameView),
+ any() /* LogData */);
+ }
+}
