Remove focus from bookmark search box as scroll happens.

Bug: 1467376 Change-Id: Iaed3abeffcc2d286081c2aca7c69b25445293e47 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4776129 Commit-Queue: Sky Malice <skym@chromium.org> Auto-Submit: Sky Malice <skym@chromium.org> Reviewed-by: Theresa Sullivan <twellington@chromium.org> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#1184754}
chromium · Aug 17, 2023 · d64bc2d · d64bc2d
1 parent f891d0c
commit d64bc2d
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 13 deletions.
diff --git a/...me/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerCoordinator.java b/...me/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerCoordinator.java
@@ -18,6 +18,7 @@
 import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.RecyclerView;
 import androidx.recyclerview.widget.RecyclerView.ItemAnimator;
+import androidx.recyclerview.widget.RecyclerView.OnScrollListener;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.metrics.RecordUserAction;
@@ -51,6 +52,8 @@
 import org.chromium.ui.modaldialog.ModalDialogManager.ModalDialogType;
 import org.chromium.ui.modelutil.MVCListAdapter.ModelList;
 
+import java.util.function.Consumer;
+
 /** Responsible for setting up sub-components and routing incoming/outgoing signals */
 // TODO(crbug.com/1446506): Add a new coordinator so this class doesn't own everything.
 public class BookmarkManagerCoordinator
@@ -168,12 +171,15 @@ public BookmarkManagerCoordinator(Context context, ComponentName openBookmarkCom
 
         BookmarkUndoController bookmarkUndoController =
                 new BookmarkUndoController(context, mBookmarkModel, snackbarManager);
+        Consumer<OnScrollListener> onScrollListenerConsumer =
+                onScrollListener -> mRecyclerView.addOnScrollListener(onScrollListener);
         mMediator = new BookmarkManagerMediator(context, mBookmarkModel, mBookmarkOpener,
                 mSelectableListLayout, mSelectionDelegate, mRecyclerView,
                 dragReorderableRecyclerViewAdapter, largeIconBridge, isDialogUi, isIncognito,
                 mBackPressStateSupplier, mProfile, bookmarkUndoController, modelList,
                 mBookmarkUiPrefs, this::hideKeyboard, bookmarkImageFetcher,
-                ShoppingServiceFactory.getForProfile(mProfile), mSnackbarManager);
+                ShoppingServiceFactory.getForProfile(mProfile), mSnackbarManager,
+                onScrollListenerConsumer);
         mPromoHeaderManager = mMediator.getPromoHeaderManager();
 
         bookmarkDelegateSupplier.set(/*bookmarkDelegate=*/mMediator);

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerMediator.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerMediator.java
@@ -14,6 +14,7 @@
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.recyclerview.widget.RecyclerView;
+import androidx.recyclerview.widget.RecyclerView.OnScrollListener;
 
 import org.chromium.base.Callback;
 import org.chromium.base.ObserverList;
@@ -66,6 +67,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.Stack;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 
 /** Responsible for BookmarkManager business logic. */
@@ -349,7 +351,7 @@ public void onBookmarkRowSortOrderChanged(@BookmarkRowSortOrder int sortOrder) {
             BookmarkUndoController bookmarkUndoController, ModelList modelList,
             BookmarkUiPrefs bookmarkUiPrefs, Runnable hideKeyboardRunnable,
             BookmarkImageFetcher bookmarkImageFetcher, ShoppingService shoppingService,
-            SnackbarManager snackbarManager) {
+            SnackbarManager snackbarManager, Consumer<OnScrollListener> onScrollListenerConsumer) {
         mContext = context;
         mBookmarkModel = bookmarkModel;
         mBookmarkModel.addObserver(mBookmarkModelObserver);
@@ -387,6 +389,18 @@ public void onBookmarkRowSortOrderChanged(@BookmarkRowSortOrder int sortOrder) {
         mShoppingService = shoppingService;
         mSnackbarManager = snackbarManager;
 
+        if (BookmarkFeatures.isAndroidImprovedBookmarksEnabled()) {
+            onScrollListenerConsumer.accept(new OnScrollListener() {
+                @Override
+                public void onScrolled(@NonNull RecyclerView recyclerView, int dx, int dy) {
+                    if (dy > 0) {
+                        mHideKeyboardRunnable.run();
+                        clearSearchBoxFocus();
+                    }
+                }
+            });
+        }
+
         // Previously we were waiting for BookmarkModel to be loaded, but it's not necessary.
         PartnerBookmarksReader.addFaviconUpdateObserver(this);
 
@@ -845,6 +859,17 @@ int getPositionForBookmark(BookmarkId bookmark) {
         return position;
     }
 
+    private void clearSearchBoxFocus() {
+        assertIsAndroidImprovedBookmarksEnabled();
+        getSearchBoxPropertyModel().set(BookmarkSearchBoxRowProperties.HAS_FOCUS, false);
+    }
+
+    private PropertyModel getSearchBoxPropertyModel() {
+        assertIsAndroidImprovedBookmarksEnabled();
+        int index = getCurrentSearchBoxIndex();
+        return index < 0 ? null : mModelList.get(index).model;
+    }
+
     @SuppressWarnings("NotifyDataSetChanged")
     private void setBookmarks(List<BookmarkListEntry> bookmarkListEntryList) {
         clearHighlight();
@@ -1087,6 +1112,8 @@ private ListItem buildSearchBoxRow() {
                                 R.string.price_tracking_bookmarks_filter_title)
                         .with(BookmarkSearchBoxRowProperties.SHOPPING_CHIP_TOGGLE_CALLBACK,
                                 this::onShoppingFilterToggle)
+                        .with(BookmarkSearchBoxRowProperties.FOCUS_CHANGE_CALLBACK,
+                                this::onSearchBoxFocusChange)
                         .build();
         return new ListItem(ViewType.SEARCH_BOX, propertyModel);
     }
@@ -1292,6 +1319,10 @@ private void onQueryCallback(String query) {
         onSearchChange(query, getCurrentSearchPowerFilter());
     }
 
+    private void onSearchBoxFocusChange(Boolean hasFocus) {
+        getSearchBoxPropertyModel().set(BookmarkSearchBoxRowProperties.HAS_FOCUS, hasFocus);
+    }
+
     private void onShoppingFilterToggle(boolean isFiltering) {
         final @NonNull Set<PowerBookmarkType> powerFilter = isFiltering
                 ? Collections.singleton(PowerBookmarkType.SHOPPING)
@@ -1336,6 +1367,10 @@ void changeSelectionMode(boolean selectionEnabled) {
             model.set(ImprovedBookmarkRowProperties.SELECTION_ACTIVE, mIsSelectionEnabled);
         }
     }
+    @SuppressWarnings("AssertionSideEffect")
+    private void assertIsAndroidImprovedBookmarksEnabled() {
+        assert BookmarkFeatures.isAndroidImprovedBookmarksEnabled();
+    }
 
     // Testing methods.
 

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRow.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRow.java
@@ -24,7 +24,7 @@
  * bookmarks.
  */
 public class BookmarkSearchBoxRow extends LinearLayout {
-    private Callback<String> mQueryCallback;
+    private EditText mSearchText;
 
     public BookmarkSearchBoxRow(Context context, @Nullable AttributeSet attrs) {
         super(context, attrs);
@@ -33,26 +33,42 @@ public BookmarkSearchBoxRow(Context context, @Nullable AttributeSet attrs) {
     @Override
     protected void onFinishInflate() {
         super.onFinishInflate();
+        mSearchText = findViewById(R.id.search_text);
+        mSearchText.setOnEditorActionListener(this::onEditorAction);
+    }
 
-        EditText searchText = findViewById(R.id.search_text);
-        searchText.addTextChangedListener(new EmptyTextWatcher() {
+    void setQueryCallback(Callback<String> queryCallback) {
+        mSearchText.addTextChangedListener(new EmptyTextWatcher() {
             @Override
             public void onTextChanged(CharSequence charSequence, int i, int i1, int i2) {
-                mQueryCallback.onResult(charSequence.toString());
+                queryCallback.onResult(charSequence.toString());
             }
         });
-        searchText.setOnEditorActionListener(this::onEditorAction);
     }
 
-    void setQueryCallback(Callback<String> queryCallback) {
-        mQueryCallback = queryCallback;
+    void setFocusChangeCallback(Callback<Boolean> focusChangeCallback) {
+        mSearchText.setOnFocusChangeListener((view, hasFocus) -> {
+            assert view == mSearchText;
+            focusChangeCallback.onResult(hasFocus);
+        });
+    }
+
+    void setHasFocus(boolean modelHasFocus) {
+        boolean viewHasFocus = mSearchText.hasFocus();
+        if (modelHasFocus == viewHasFocus) return;
+        if (modelHasFocus) {
+            mSearchText.requestFocus();
+        } else {
+            mSearchText.clearFocus();
+        }
     }
 
     private boolean onEditorAction(TextView textView, int actionId, KeyEvent keyEvent) {
+        assert textView == mSearchText;
         if (actionId == EditorInfo.IME_ACTION_SEARCH
                 || keyEvent != null && keyEvent.getKeyCode() == KeyEvent.KEYCODE_ENTER) {
             KeyboardVisibilityDelegate.getInstance().hideKeyboard(textView);
-            clearFocus();
+            mSearchText.clearFocus();
             return true;
         } else {
             return false;

diff --git a/...ndroid/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowProperties.java b/...ndroid/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowProperties.java
@@ -8,11 +8,17 @@
 import org.chromium.ui.modelutil.PropertyKey;
 import org.chromium.ui.modelutil.PropertyModel.ReadableObjectPropertyKey;
 import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey;
+import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;
 
 /** Responsible for hosting properties for {@link BookmarkSearchBoxRow}. */
 class BookmarkSearchBoxRowProperties {
     public static final ReadableObjectPropertyKey<Callback<String>> QUERY_CALLBACK =
             new ReadableObjectPropertyKey<>();
+    public static final ReadableObjectPropertyKey<Callback<Boolean>> FOCUS_CHANGE_CALLBACK =
+            new ReadableObjectPropertyKey<>();
+    public static final WritableObjectPropertyKey<Boolean> HAS_FOCUS =
+            new WritableObjectPropertyKey<>();
+
     public static final ReadableObjectPropertyKey<Callback<Boolean>> SHOPPING_CHIP_TOGGLE_CALLBACK =
             new ReadableObjectPropertyKey<>();
     public static final WritableBooleanPropertyKey SHOPPING_CHIP_VISIBILITY =
@@ -23,6 +29,6 @@ class BookmarkSearchBoxRowProperties {
             new ReadableObjectPropertyKey<>();
 
     static final PropertyKey[] ALL_KEYS = {BookmarkManagerProperties.BOOKMARK_LIST_ENTRY,
-            QUERY_CALLBACK, SHOPPING_CHIP_TOGGLE_CALLBACK, SHOPPING_CHIP_VISIBILITY,
-            SHOPPING_CHIP_START_ICON_RES, SHOPPING_CHIP_TEXT_RES};
+            QUERY_CALLBACK, FOCUS_CHANGE_CALLBACK, HAS_FOCUS, SHOPPING_CHIP_TOGGLE_CALLBACK,
+            SHOPPING_CHIP_VISIBILITY, SHOPPING_CHIP_START_ICON_RES, SHOPPING_CHIP_TEXT_RES};
 }
diff --git a/...ndroid/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowViewBinder.java b/...ndroid/java/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowViewBinder.java
@@ -22,6 +22,11 @@ static void bind(PropertyModel model, View view, PropertyKey key) {
         ChipView shoppingChip = view.findViewById(R.id.shopping_filter_chip);
         if (key == BookmarkSearchBoxRowProperties.QUERY_CALLBACK) {
             row.setQueryCallback(model.get(BookmarkSearchBoxRowProperties.QUERY_CALLBACK));
+        } else if (key == BookmarkSearchBoxRowProperties.FOCUS_CHANGE_CALLBACK) {
+            row.setFocusChangeCallback(
+                    model.get(BookmarkSearchBoxRowProperties.FOCUS_CHANGE_CALLBACK));
+        } else if (key == BookmarkSearchBoxRowProperties.HAS_FOCUS) {
+            row.setHasFocus(model.get(BookmarkSearchBoxRowProperties.HAS_FOCUS));
         } else if (key == BookmarkSearchBoxRowProperties.SHOPPING_CHIP_VISIBILITY) {
             boolean isVisible = model.get(BookmarkSearchBoxRowProperties.SHOPPING_CHIP_VISIBILITY);
             ((View) shoppingChip.getParent()).setVisibility(isVisible ? View.VISIBLE : View.GONE);

diff --git a/...android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowTest.java b/...android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkSearchBoxRowTest.java
@@ -61,6 +61,8 @@ public class BookmarkSearchBoxRowTest {
     @Mock
     private Callback<String> mQueryCallback;
     @Mock
+    private Callback<Boolean> mFocusChangeCallback;
+    @Mock
     private Callback<Boolean> mToggleCallback;
 
     private BookmarkSearchBoxRow mBookmarkSearchBoxRow;
@@ -95,6 +97,8 @@ public void setUp() throws Exception {
                     new PropertyModel.Builder(BookmarkSearchBoxRowProperties.ALL_KEYS)
                             .with(BookmarkSearchBoxRowProperties.SHOPPING_CHIP_VISIBILITY, false)
                             .with(BookmarkSearchBoxRowProperties.QUERY_CALLBACK, mQueryCallback)
+                            .with(BookmarkSearchBoxRowProperties.FOCUS_CHANGE_CALLBACK,
+                                    mFocusChangeCallback)
                             .with(BookmarkSearchBoxRowProperties.SHOPPING_CHIP_TOGGLE_CALLBACK,
                                     mToggleCallback)
                             .build();
@@ -126,6 +130,16 @@ public void testQueryCallback() {
         Mockito.verify(mQueryCallback).onResult(Mockito.eq(query));
     }
 
+    @Test
+    @MediumTest
+    public void testFocusChangeCallback() {
+        TestThreadUtils.runOnUiThreadBlocking(() -> mBookmarkSearchBoxRow.setHasFocus(true));
+        Mockito.verify(mFocusChangeCallback).onResult(true);
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> mBookmarkSearchBoxRow.setHasFocus(false));
+        Mockito.verify(mFocusChangeCallback).onResult(true);
+    }
+
     @Test
     @MediumTest
     public void testShoppingChipVisibility() {

diff --git a/.../android/junit/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerMediatorTest.java b/.../android/junit/src/org/chromium/chrome/browser/bookmarks/BookmarkManagerMediatorTest.java
@@ -23,6 +23,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 import static org.chromium.ui.test.util.MockitoHelper.doCallback;
@@ -38,6 +39,7 @@
 import androidx.annotation.Nullable;
 import androidx.annotation.StringRes;
 import androidx.recyclerview.widget.RecyclerView;
+import androidx.recyclerview.widget.RecyclerView.OnScrollListener;
 import androidx.test.ext.junit.rules.ActivityScenarioRule;
 
 import org.junit.Before;
@@ -117,6 +119,7 @@
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.function.Consumer;
 
 /** Unit tests for {@link BookmarkManagerMediator}. */
 @Batch(Batch.UNIT_TESTS)
@@ -183,6 +186,8 @@ public class BookmarkManagerMediatorTest {
     private PriceTrackingUtils.Natives mPriceTrackingUtilsJniMock;
     @Mock
     private ListObservable.ListObserver<Void> mListObserver;
+    @Mock
+    private Consumer<OnScrollListener> mOnScrollListenerConsumer;
 
     @Captor
     private ArgumentCaptor<BookmarkModelObserver> mBookmarkModelObserverArgumentCaptor;
@@ -194,6 +199,8 @@ public class BookmarkManagerMediatorTest {
     private ArgumentCaptor<SyncStateChangedListener> mSyncStateChangedListenerCaptor;
     @Captor
     private ArgumentCaptor<Runnable> mFinishLoadingBookmarkModelCaptor;
+    @Captor
+    private ArgumentCaptor<OnScrollListener> mOnScrollListenerCaptor;
 
     private final ObservableSupplierImpl<Boolean> mBackPressStateSupplier =
             new ObservableSupplierImpl<>();
@@ -364,7 +371,8 @@ public void setUp() {
                     mDragReorderableRecyclerViewAdapter, mLargeIconBridge, /*isDialogUi=*/true,
                     /*isIncognito=*/false, mBackPressStateSupplier, mProfile,
                     mBookmarkUndoController, mModelList, mBookmarkUiPrefs, mHideKeyboardRunnable,
-                    mBookmarkImageFetcher, mShoppingService, mSnackbarManager);
+                    mBookmarkImageFetcher, mShoppingService, mSnackbarManager,
+                    mOnScrollListenerConsumer);
             mMediator.addUiObserver(mBookmarkUiObserver);
         });
     }
@@ -1271,4 +1279,28 @@ public void testChangeSelectionMode() {
         assertTrue(mModelList.get(1).model.get(ImprovedBookmarkRowProperties.SELECTION_ACTIVE));
         assertTrue(mModelList.get(2).model.get(ImprovedBookmarkRowProperties.SELECTION_ACTIVE));
     }
+
+    @Test
+    @EnableFeatures(ChromeFeatureList.ANDROID_IMPROVED_BOOKMARKS)
+    public void testClearFocusOnScroll() {
+        finishLoading();
+        mMediator.openFolder(mFolderId1);
+
+        assertEquals(ViewType.SEARCH_BOX, mModelList.get(0).type);
+        verify(mOnScrollListenerConsumer).accept(mOnScrollListenerCaptor.capture());
+        OnScrollListener onScrollListener = mOnScrollListenerCaptor.getValue();
+
+        PropertyModel searchBoxRowPropertyModel = mModelList.get(0).model;
+        searchBoxRowPropertyModel.get(BookmarkSearchBoxRowProperties.FOCUS_CHANGE_CALLBACK)
+                .onResult(true);
+        assertTrue(searchBoxRowPropertyModel.get(BookmarkSearchBoxRowProperties.HAS_FOCUS));
+
+        onScrollListener.onScrolled(mRecyclerView, 0, -1);
+        verifyNoInteractions(mHideKeyboardRunnable);
+        assertTrue(searchBoxRowPropertyModel.get(BookmarkSearchBoxRowProperties.HAS_FOCUS));
+
+        onScrollListener.onScrolled(mRecyclerView, 0, 1);
+        verify(mHideKeyboardRunnable).run();
+        assertFalse(searchBoxRowPropertyModel.get(BookmarkSearchBoxRowProperties.HAS_FOCUS));
+    }
 }
diff --git a/...a/src/org/chromium/components/browser_ui/widget/selectable_list/SelectableListLayout.java b/...a/src/org/chromium/components/browser_ui/widget/selectable_list/SelectableListLayout.java
@@ -181,6 +181,9 @@ public void onScrolled(RecyclerView recyclerView, int dx, int dy) {
                 setToolbarShadowVisibility();
             }
         });
+        mRecyclerView.addOnLayoutChangeListener(
+                (View v, int left, int top, int right, int bottom, int oldLeft, int oldTop,
+                        int oldRight, int oldBottom) -> { setToolbarShadowVisibility(); });
 
         mItemAnimator = mRecyclerView.getItemAnimator();
     }