Skip to content

Commit

Permalink
[Journeys] Add more progress spinner/button.
Browse files Browse the repository at this point in the history
This displays a spinner when content is loading, and a load more button
when scroll to load is disabled. Scroll to load is disabled when
accessibility is enabled or a hardware keyboard is attached. In this
state, "infinite scroll" is disabled.

Bug: 1303171, 1346662
Change-Id: Ic5f10e18f80c432f031045ed4dc1762d0d4fef88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3781705
Reviewed-by: Gang Wu <gangwu@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030608}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed Aug 2, 2022
1 parent 0b8a051 commit 86f3d60
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 30 deletions.
Expand Up @@ -52,6 +52,7 @@
import org.chromium.chrome.browser.ui.messages.snackbar.Snackbar;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager.SnackbarController;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.components.browser_ui.settings.SettingsLauncher;
import org.chromium.components.browser_ui.widget.selectable_list.SelectableItemView;
import org.chromium.components.browser_ui.widget.selectable_list.SelectableListLayout;
Expand Down Expand Up @@ -256,9 +257,9 @@ public void onOptOut() {
}
};

mHistoryClustersCoordinator =
new HistoryClustersCoordinator(Profile.getLastUsedRegularProfile(), activity,
TemplateUrlServiceFactory.get(), historyClustersDelegate);
mHistoryClustersCoordinator = new HistoryClustersCoordinator(
Profile.getLastUsedRegularProfile(), activity, TemplateUrlServiceFactory.get(),
historyClustersDelegate, ChromeAccessibilityUtil.get());
}

// 1. Create selectable components.
Expand Down
Expand Up @@ -128,6 +128,7 @@
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.ui.system.StatusBarColorController;
import org.chromium.chrome.browser.ui.system.StatusBarColorController.StatusBarColorProvider;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.chrome.browser.vr.VrModuleProvider;
import org.chromium.chrome.features.start_surface.StartSurface;
import org.chromium.components.browser_ui.accessibility.PageZoomCoordinator;
Expand Down Expand Up @@ -824,8 +825,9 @@ public ViewGroup getToggleView(ViewGroup parent) {
}
};

mHistoryClustersCoordinator = new HistoryClustersCoordinator(
profile, mActivity, TemplateUrlServiceFactory.get(), historyClustersDelegate);
mHistoryClustersCoordinator = new HistoryClustersCoordinator(profile, mActivity,
TemplateUrlServiceFactory.get(), historyClustersDelegate,
ChromeAccessibilityUtil.get());
mHistoryClustersCoordinatorSupplier.set(mHistoryClustersCoordinator);
}
}
Expand Down
Expand Up @@ -59,6 +59,7 @@
import org.chromium.components.favicon.LargeIconBridgeJni;
import org.chromium.components.search_engines.TemplateUrlService;
import org.chromium.ui.display.DisplayAndroidManager;
import org.chromium.ui.util.AccessibilityUtil;
import org.chromium.url.GURL;

import java.io.Serializable;
Expand Down Expand Up @@ -178,6 +179,8 @@ public void markVisitForRemoval(ClusterVisit clusterVisit) {
private GURL mGurl2;
@Mock
private HistoryClustersMetricsLogger mMetricsLogger;
@Mock
private AccessibilityUtil mAccessibilityUtil;

private ActivityScenario<ChromeTabbedActivity> mActivityScenario;
private HistoryClustersCoordinator mHistoryClustersCoordinator;
Expand Down Expand Up @@ -214,9 +217,9 @@ public void setUp() {
mActivityScenario =
ActivityScenario.launch(ChromeTabbedActivity.class).onActivity(activity -> {
mActivity = activity;
mHistoryClustersCoordinator =
new HistoryClustersCoordinator(mProfile, activity, mTemplateUrlService,
mHistoryClustersDelegate, mMetricsLogger, mSelectionDelegate);
mHistoryClustersCoordinator = new HistoryClustersCoordinator(mProfile, activity,
mTemplateUrlService, mHistoryClustersDelegate, mMetricsLogger,
mSelectionDelegate, mAccessibilityUtil);
});
}

Expand Down
Expand Up @@ -5,6 +5,7 @@
package org.chromium.chrome.browser.history_clusters;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.AdditionalMatchers.geq;
Expand All @@ -18,6 +19,7 @@

import android.content.Context;
import android.content.Intent;
import android.content.res.Configuration;
import android.content.res.Resources;
import android.graphics.Typeface;
import android.net.Uri;
Expand All @@ -42,6 +44,7 @@
import org.junit.runner.RunWith;
import org.mockito.ArgumentMatcher;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.robolectric.annotation.Config;
Expand All @@ -59,6 +62,7 @@
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.TabCreator;
import org.chromium.components.browser_ui.widget.MoreProgressButton.State;
import org.chromium.components.browser_ui.widget.selectable_list.SelectionDelegate;
import org.chromium.components.favicon.LargeIconBridge;
import org.chromium.components.search_engines.TemplateUrlService;
Expand All @@ -67,6 +71,7 @@
import org.chromium.ui.modelutil.MVCListAdapter.ModelList;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.shadows.ShadowAppCompatResources;
import org.chromium.ui.util.AccessibilityUtil;
import org.chromium.url.GURL;
import org.chromium.url.JUnitTestGURLs;
import org.chromium.url.ShadowGURL;
Expand Down Expand Up @@ -125,6 +130,10 @@ public class HistoryClustersMediatorTest {
private TabCreator mTabCreator;
@Mock
private HistoryClustersMetricsLogger mMetricsLogger;
@Mock
private AccessibilityUtil mAccessibilityUtil;
@Mock
private Configuration mConfiguration;

private ClusterVisit mVisit1;
private ClusterVisit mVisit2;
Expand Down Expand Up @@ -155,6 +164,8 @@ public void setUp() {
doReturn(mResources).when(mContext).getResources();
doReturn(ITEM_URL_SPEC).when(mMockGurl).getSpec();
doReturn(mLayoutManager).when(mRecyclerView).getLayoutManager();
mConfiguration.keyboard = Configuration.KEYBOARD_NOKEYS;
doReturn(mConfiguration).when(mResources).getConfiguration();
mModelList = new ModelList();
mToolbarModel = new PropertyModel(HistoryClustersToolbarProperties.ALL_KEYS);

Expand Down Expand Up @@ -238,7 +249,7 @@ public void markVisitForRemoval(ClusterVisit clusterVisit) {

mMediator = new HistoryClustersMediator(mBridge, mLargeIconBridge, mContext, mResources,
mModelList, mToolbarModel, mHistoryClustersDelegate, mClock, mTemplateUrlService,
mSelectionDelegate, mMetricsLogger);
mSelectionDelegate, mMetricsLogger, mAccessibilityUtil);
mVisit1 = new ClusterVisit(1.0F, mGurl1, "Title 1", "url1.com/", new ArrayList<>(),
new ArrayList<>(), mGurl1, 123L, new ArrayList<>());
mVisit2 = new ClusterVisit(1.0F, mGurl2, "Title 2", "url2.com/", new ArrayList<>(),
Expand Down Expand Up @@ -281,7 +292,11 @@ public void testQuery() {
// call both.
mMediator.setQueryState(QueryState.forQuery("query", ""));
mMediator.startQuery("query");
assertEquals(mModelList.size(), 0);
assertEquals(1, mModelList.size());
ListItem spinnerItem = mModelList.get(0);
assertEquals(spinnerItem.type, ItemType.MORE_PROGRESS);
assertEquals(spinnerItem.model.get(HistoryClustersItemProperties.PROGRESS_BUTTON_STATE),
State.LOADING);

fulfillPromise(promise, mHistoryClustersResultWithQuery);

Expand Down Expand Up @@ -315,6 +330,55 @@ public void testQuery() {
HistoryClustersItemProperties.RELATED_SEARCHES)));
}

@Test
public void testScrollToLoadDisabled() {
mConfiguration.keyboard = Configuration.KEYBOARD_12KEY;
mMediator = new HistoryClustersMediator(mBridge, mLargeIconBridge, mContext, mResources,
mModelList, mToolbarModel, mHistoryClustersDelegate, mClock, mTemplateUrlService,
mSelectionDelegate, mMetricsLogger, mAccessibilityUtil);

Promise<HistoryClustersResult> promise = new Promise<>();
doReturn(promise).when(mBridge).queryClusters("query");
Promise<HistoryClustersResult> secondPromise = new Promise();
doReturn(secondPromise).when(mBridge).loadMoreClusters("query");

mMediator.setQueryState(QueryState.forQuery("query", ""));
mMediator.startQuery("query");

assertEquals(1, mModelList.size());
ListItem spinnerItem = mModelList.get(0);
assertEquals(spinnerItem.type, ItemType.MORE_PROGRESS);
assertEquals(spinnerItem.model.get(HistoryClustersItemProperties.PROGRESS_BUTTON_STATE),
State.LOADING);

fulfillPromise(promise, mHistoryClustersResultWithQuery);

spinnerItem = mModelList.get(mModelList.size() - 1);
assertEquals(spinnerItem.type, ItemType.MORE_PROGRESS);
assertEquals(spinnerItem.model.get(HistoryClustersItemProperties.PROGRESS_BUTTON_STATE),
State.BUTTON);

mMediator.onScrolled(mRecyclerView, 1, 1);
verify(mBridge, Mockito.never()).loadMoreClusters("query");

spinnerItem.model.get(HistoryClustersItemProperties.CLICK_HANDLER).onClick(null);
ShadowLooper.idleMainLooper();

verify(mBridge).loadMoreClusters("query");
spinnerItem = mModelList.get(mModelList.size() - 1);
assertEquals(spinnerItem.type, ItemType.MORE_PROGRESS);
assertEquals(spinnerItem.model.get(HistoryClustersItemProperties.PROGRESS_BUTTON_STATE),
State.LOADING);

fulfillPromise(secondPromise, mHistoryClustersFollowupResultWithQuery);
// There should no longer be a spinner or "load more" button once all possible results for
// the current query have been loaded.
for (int i = 0; i < mModelList.size(); i++) {
ListItem item = mModelList.get(i);
assertNotEquals(item.type, ItemType.MORE_PROGRESS);
}
}

@Test
public void testEmptyQuery() {
Promise<HistoryClustersResult> promise = new Promise<>();
Expand Down Expand Up @@ -393,14 +457,17 @@ public void testSearchTextChanged() {
mMediator.setQueryState(QueryState.forQuery("pan", ""));
mMediator.onSearchTextChanged("pan");

assertEquals(mModelList.size(), 0);
assertEquals(mModelList.size(), 1);
ListItem spinnerItem = mModelList.get(0);
assertEquals(spinnerItem.type, ItemType.MORE_PROGRESS);
assertEquals(spinnerItem.model.get(HistoryClustersItemProperties.PROGRESS_BUTTON_STATE),
State.LOADING);
verify(mBridge).queryClusters("pan");

doReturn(new Promise<>()).when(mBridge).queryClusters("");
mModelList.add(new ListItem(42, new PropertyModel()));

mMediator.onEndSearch();

assertEquals(mModelList.size(), 0);
verify(mBridge).queryClusters("");
}

Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import org.chromium.ui.modelutil.SimpleRecyclerViewAdapter;
import org.chromium.ui.util.AccessibilityUtil;

/**
* Root component for the HistoryClusters UI component, which displays lists of related history
Expand Down Expand Up @@ -70,7 +71,8 @@ public boolean isSelectionEnabled() {
HistoryClustersCoordinator(@NonNull Profile profile, @NonNull Activity activity,
TemplateUrlService templateUrlService, HistoryClustersDelegate historyClustersDelegate,
HistoryClustersMetricsLogger metricsLogger,
SelectionDelegate<ClusterVisit> selectionDelegate) {
SelectionDelegate<ClusterVisit> selectionDelegate,
AccessibilityUtil accessibilityUtil) {
mActivity = activity;
mDelegate = historyClustersDelegate;
mModelList = new ModelList();
Expand All @@ -84,7 +86,7 @@ public boolean isSelectionEnabled() {
mMediator = new HistoryClustersMediator(HistoryClustersBridge.getForProfile(profile),
new LargeIconBridge(profile), mActivity, mActivity.getResources(), mModelList,
mToolbarModel, mDelegate, System::currentTimeMillis, templateUrlService,
mSelectionDelegate, mMetricsLogger);
mSelectionDelegate, mMetricsLogger, accessibilityUtil);
}

/**
Expand All @@ -93,12 +95,14 @@ public boolean isSelectionEnabled() {
* @param activity Activity in which this UI resides.
* @param historyClustersDelegate Delegate that provides functionality that must be implemented
* externally, e.g. populating intents targeting activities we can't reference directly.
* @param accessibilityUtil Utility object that tells us about the current accessibility state.
*/
public HistoryClustersCoordinator(@NonNull Profile profile, @NonNull Activity activity,
TemplateUrlService templateUrlService,
HistoryClustersDelegate historyClustersDelegate) {
TemplateUrlService templateUrlService, HistoryClustersDelegate historyClustersDelegate,
AccessibilityUtil accessibilityUtil) {
this(profile, activity, templateUrlService, historyClustersDelegate,
new HistoryClustersMetricsLogger(templateUrlService), new SelectionDelegate<>());
new HistoryClustersMetricsLogger(templateUrlService), new SelectionDelegate<>(),
accessibilityUtil);
}

public void destroy() {
Expand Down Expand Up @@ -157,6 +161,8 @@ void inflateActivityView() {
HistoryClustersViewBinder::noopBindView);
mAdapter.registerType(ItemType.CLEAR_BROWSING_DATA, mDelegate::getClearBrowsingDataView,
HistoryClustersViewBinder::noopBindView);
mAdapter.registerType(ItemType.MORE_PROGRESS, this::buildMoreProgressView,
HistoryClustersViewBinder::bindMoreProgressView);

LayoutInflater layoutInflater = LayoutInflater.from(mActivity);
mActivityContentView = (ViewGroup) layoutInflater.inflate(
Expand Down Expand Up @@ -192,6 +198,11 @@ void inflateActivityView() {
mActivityViewInflated = true;
}

private View buildMoreProgressView(ViewGroup parent) {
return LayoutInflater.from(parent.getContext())
.inflate(R.layout.more_progress_button, parent, false);
}

private View buildClusterView(ViewGroup parent) {
SelectableItemView<HistoryCluster> clusterView =
(SelectableItemView<HistoryCluster>) LayoutInflater.from(parent.getContext())
Expand Down
Expand Up @@ -21,7 +21,7 @@
class HistoryClustersItemProperties {
@IntDef({HistoryClustersItemProperties.ItemType.VISIT, ItemType.CLUSTER,
ItemType.RELATED_SEARCHES, ItemType.TOGGLE, ItemType.PRIVACY_DISCLAIMER,
ItemType.CLEAR_BROWSING_DATA})
ItemType.CLEAR_BROWSING_DATA, ItemType.MORE_PROGRESS})
@Retention(RetentionPolicy.SOURCE)
@interface ItemType {
int VISIT = 1;
Expand All @@ -30,6 +30,7 @@ class HistoryClustersItemProperties {
int TOGGLE = 4;
int PRIVACY_DISCLAIMER = 5;
int CLEAR_BROWSING_DATA = 6;
int MORE_PROGRESS = 7;
}

static final WritableIntPropertyKey ACCESSIBILITY_STATE = new WritableIntPropertyKey();
Expand All @@ -48,6 +49,7 @@ class HistoryClustersItemProperties {
static final WritableObjectPropertyKey<Drawable> ICON_DRAWABLE =
new WritableObjectPropertyKey<>();
static final WritableObjectPropertyKey<String> LABEL = new WritableObjectPropertyKey<>();
static final WritableIntPropertyKey PROGRESS_BUTTON_STATE = new WritableIntPropertyKey();
static final WritableObjectPropertyKey<List<String>> RELATED_SEARCHES =
new WritableObjectPropertyKey<>();
static final WritableObjectPropertyKey<CharSequence> TITLE = new WritableObjectPropertyKey<>();
Expand All @@ -56,5 +58,5 @@ class HistoryClustersItemProperties {

static final PropertyKey[] ALL_KEYS = {ACCESSIBILITY_STATE, CHIP_CLICK_HANDLER, CLICK_HANDLER,
CLUSTER_VISIT, DIVIDER_VISIBLE, END_BUTTON_CLICK_HANDLER, END_BUTTON_DRAWABLE,
ICON_DRAWABLE, LABEL, RELATED_SEARCHES, TITLE, URL, VISIBILITY};
ICON_DRAWABLE, LABEL, PROGRESS_BUTTON_STATE, RELATED_SEARCHES, TITLE, URL, VISIBILITY};
}

0 comments on commit 86f3d60

Please sign in to comment.