Commit 32600f59 authored by Yusuf Ozuysal's avatar Yusuf Ozuysal Committed by Commit Bot

Functionality touch up on grid tab switcher

1) Add back button support :)
- Adds a OverviewModeController that also gives the owner the APIs to
show and hide the Overview.
- Adds an internal implementation in ChromeTabbedActivity that will hold
on to an overridable Controller and use that for all signaling. Every
component gets the Container class and never changes it.
- Extend the API for RecyclerView to support not animating hide and show
- Have GridMediator implement controller and the Coordinator hand that
off with a public API when needed.
- Have the TabbedActivity explicitly own the Grid component and override
the Controller.
- Replace all calls related with Controller using LayoutManager with
related Controller calls.

2) Makes sure the grid always shows the current tab when shown.

3) Disables swiping from toolbar to show the StackLayout and
ToolbarSwipeLayout. (I will try to see if there is a path to enabling
ToolbarSwipeLayout without adding a ton, but for now this is the most
"elegant solution")

BUG=934564

Change-Id: Ic778d2e2c158ba5e089f22d829f321f42e0e7684
Reviewed-on: https://chromium-review.googlesource.com/c/1484734
Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635755}
parent 4fe92e6c
......@@ -41,7 +41,7 @@ import java.util.List;
* A {@link Layout} controller for the more complicated Chrome browser. This is currently a
* superset of {@link LayoutManager}.
*/
public class LayoutManagerChrome extends LayoutManager implements OverviewModeBehavior {
public class LayoutManagerChrome extends LayoutManager implements OverviewModeController {
// Layouts
/** An {@link Layout} that should be used as the accessibility tab switcher. */
protected OverviewListLayout mOverviewListLayout;
......@@ -318,6 +318,7 @@ public class LayoutManagerChrome extends LayoutManager implements OverviewModeBe
* all of the {@link Tab}s opened by the user.
* @param animate Whether or not to animate the transition to overview mode.
*/
@Override
public void showOverview(boolean animate) {
boolean useAccessibility = DeviceClassManager.enableAccessibilityLayout();
......@@ -340,6 +341,7 @@ public class LayoutManagerChrome extends LayoutManager implements OverviewModeBe
* Hides the current {@link Layout}, returning to the default {@link Layout}.
* @param animate Whether or not to animate the transition to the default {@link Layout}.
*/
@Override
public void hideOverview(boolean animate) {
Layout activeLayout = getActiveLayout();
if (activeLayout != null && !activeLayout.isHiding()) {
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.compositor.layouts;
/**
* Interface to control the OverviewMode.
*/
public interface OverviewModeController extends OverviewModeBehavior {
/**
* Hide the overview.
* @param animate Whether we should animate while hiding.
*/
void hideOverview(boolean animate);
/**
* Show the overview.
* @param animate Whether we should animate while showing.
*/
void showOverview(boolean animate);
}
\ No newline at end of file
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.dependency_injection;
import org.chromium.chrome.browser.contextual_suggestions.ContextualSuggestionsCoordinator;
import org.chromium.chrome.browser.contextual_suggestions.ContextualSuggestionsModule;
import org.chromium.chrome.browser.tasks.tab_list_ui.GridTabSwitcherCoordinator;
import dagger.Subcomponent;
......@@ -20,5 +19,4 @@ public interface ChromeActivityComponent {
// Temporary getters for DI migration process.
ContextualSuggestionsCoordinator resolveContextualSuggestionsCoordinator();
GridTabSwitcherCoordinator resolveGridTabSwitcherCoordinator();
}
......@@ -27,9 +27,9 @@ import android.widget.TextView;
import org.chromium.base.TraceEvent;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.compositor.layouts.EmptyOverviewModeObserver;
import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
import org.chromium.chrome.browser.compositor.layouts.LayoutManagerChrome;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.compositor.layouts.content.InvalidationAwareThumbnailProvider;
import org.chromium.chrome.browser.explore_sites.ExperimentalExploreSitesSection;
import org.chromium.chrome.browser.explore_sites.ExploreSitesBridge;
......@@ -269,20 +269,19 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
VrModuleProvider.registerVrModeObserver(this);
if (VrModuleProvider.getDelegate().isInVr()) onEnterVr();
LayoutManager layoutManager =
tab.getActivity().getCompositorViewHolder().getLayoutManager();
if (layoutManager instanceof LayoutManagerChrome) {
final LayoutManagerChrome chromeLayoutManager = (LayoutManagerChrome) layoutManager;
if (chromeLayoutManager.overviewVisible()) {
if (tab.getActivity() instanceof ChromeTabbedActivity) {
OverviewModeBehavior overviewModeBehavior =
((ChromeTabbedActivity) tab.getActivity()).getOverviewModeBehavior();
if (overviewModeBehavior.overviewVisible()) {
mOverviewObserver = new EmptyOverviewModeObserver() {
@Override
public void onOverviewModeFinishedHiding() {
maybeShowIPHOnHomepageTile();
chromeLayoutManager.removeOverviewModeObserver(mOverviewObserver);
overviewModeBehavior.removeOverviewModeObserver(mOverviewObserver);
mOverviewObserver = null;
}
};
chromeLayoutManager.addOverviewModeObserver(mOverviewObserver);
overviewModeBehavior.addOverviewModeObserver(mOverviewObserver);
}
}
......@@ -909,9 +908,9 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
VrModuleProvider.unregisterVrModeObserver(this);
// Need to null-check compositor view holder and layout manager since they might've
// been cleared by now.
if (mOverviewObserver != null && mTab.getActivity().getCompositorViewHolder() != null
&& mTab.getActivity().getCompositorViewHolder().getLayoutManager() != null) {
((LayoutManagerChrome) mTab.getActivity().getCompositorViewHolder().getLayoutManager())
if (mOverviewObserver != null && mTab.getActivity() instanceof ChromeTabbedActivity) {
((ChromeTabbedActivity) mTab.getActivity())
.getOverviewModeBehavior()
.removeOverviewModeObserver(mOverviewObserver);
mOverviewObserver = null;
}
......
......@@ -4,13 +4,11 @@
package org.chromium.chrome.browser.tasks.tab_list_ui;
import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.ACTIVITY_CONTEXT;
import android.content.Context;
import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeController;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
import org.chromium.chrome.browser.dependency_injection.ActivityScope;
import org.chromium.chrome.browser.init.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.tabmodel.TabModel;
......@@ -19,21 +17,16 @@ import org.chromium.chrome.browser.toolbar.ToolbarManager;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import javax.inject.Inject;
import javax.inject.Named;
/**
* Parent coordinator that is responsible for showing a grid of tabs for the main TabSwitcher UI.
*/
@ActivityScope
public class GridTabSwitcherCoordinator implements Destroyable {
private final PropertyModelChangeProcessor mContainerViewChangeProcessor;
private final ActivityLifecycleDispatcher mLifecycleDispatcher;
private final TabListCoordinator mTabGridCoordinator;
private final GridTabSwitcherMediator mMediator;
@Inject
public GridTabSwitcherCoordinator(@Named(ACTIVITY_CONTEXT) Context context,
public GridTabSwitcherCoordinator(Context context,
ActivityLifecycleDispatcher lifecycleDispatcher, ToolbarManager toolbarManager,
TabModelSelector tabModelSelector, TabContentManager tabContentManager,
CompositorViewHolder compositorViewHolder) {
......@@ -46,13 +39,19 @@ public class GridTabSwitcherCoordinator implements Destroyable {
mTabGridCoordinator.getContainerView(), TabGridContainerViewBinder::bind);
mMediator = new GridTabSwitcherMediator(this, containerViewModel, tabModelSelector);
toolbarManager.overrideTabSwitcherBehavior(
mMediator.getTabSwitcherButtonClickListener(), mMediator);
mLifecycleDispatcher = lifecycleDispatcher;
mLifecycleDispatcher.register(this);
}
/**
* @return OverviewModeController implementation that will can be used for controlling
* OverviewMode changes.
*/
public OverviewModeController getOverviewModeController() {
return mMediator;
}
/**
* Reset the tab grid with the given {@link TabModel}. Can be null.
* @param tabModel The current {@link TabModel} to show the tabs for in the grid.
......
......@@ -4,14 +4,13 @@
package org.chromium.chrome.browser.tasks.tab_list_ui;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.ANIMATE_VISIBILITY_CHANGES;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.INITIAL_SCROLL_INDEX;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.IS_INCOGNITO;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.IS_VISIBLE;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.VISIBILITY_LISTENER;
import android.view.View;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeController;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelSelectorObserver;
import org.chromium.chrome.browser.tabmodel.TabModel;
......@@ -28,7 +27,9 @@ import java.util.List;
* changes.
*/
class GridTabSwitcherMediator
implements OverviewModeBehavior, TabListRecyclerView.VisibilityListener {
implements OverviewModeController, TabListRecyclerView.VisibilityListener {
private static final int INITIAL_SCROLL_INDEX_OFFSET = 3;
private final GridTabSwitcherCoordinator mCoordinator;
private final PropertyModel mContainerViewModel;
private final TabModelSelector mTabModelSelector;
......@@ -93,22 +94,17 @@ class GridTabSwitcherMediator
mContainerViewModel.set(VISIBILITY_LISTENER, this);
mContainerViewModel.set(IS_INCOGNITO, mTabModelSelector.getCurrentModel().isIncognito());
}
/**
* @return The {@link android.view.View.OnClickListener} to override for the tab switcher
* button.
*/
View.OnClickListener getTabSwitcherButtonClickListener() {
return view -> {
boolean setVisible = !mContainerViewModel.get(IS_VISIBLE);
setVisibility(setVisible);
if (setVisible) RecordUserAction.record("MobileToolbarShowStackView");
};
mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, true);
}
private void setVisibility(boolean isVisible) {
if (isVisible) mCoordinator.resetWithTabModel(mTabModelSelector.getCurrentModel());
if (isVisible) {
mCoordinator.resetWithTabModel(mTabModelSelector.getCurrentModel());
int initialPosition = Math.max(
mTabModelSelector.getCurrentModel().index() - INITIAL_SCROLL_INDEX_OFFSET, 0);
mContainerViewModel.set(INITIAL_SCROLL_INDEX, initialPosition);
}
mContainerViewModel.set(IS_VISIBLE, isVisible);
}
......@@ -127,6 +123,20 @@ class GridTabSwitcherMediator
mObservers.remove(listener);
}
@Override
public void hideOverview(boolean animate) {
if (!animate) mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, false);
setVisibility(false);
mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, true);
}
@Override
public void showOverview(boolean animate) {
if (!animate) mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, false);
setVisibility(true);
mContainerViewModel.set(ANIMATE_VISIBILITY_CHANGES, true);
}
@Override
public void startedShowing() {
for (OverviewModeObserver observer : mObservers) {
......
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.tasks.tab_list_ui;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.ANIMATE_VISIBILITY_CHANGES;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.INITIAL_SCROLL_INDEX;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.IS_INCOGNITO;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.IS_VISIBLE;
import static org.chromium.chrome.browser.tasks.tab_list_ui.TabListContainerProperties.VISIBILITY_LISTENER;
......@@ -23,15 +25,17 @@ class TabGridContainerViewBinder {
PropertyModel model, TabListRecyclerView view, PropertyKey propertyKey) {
if (IS_VISIBLE == propertyKey) {
if (model.get(IS_VISIBLE)) {
view.startShowing();
view.startShowing(model.get(ANIMATE_VISIBILITY_CHANGES));
} else {
view.startHiding();
view.startHiding(model.get(ANIMATE_VISIBILITY_CHANGES));
}
} else if (IS_INCOGNITO == propertyKey) {
view.setBackgroundColor(
ColorUtils.getDefaultThemeColor(view.getResources(), model.get(IS_INCOGNITO)));
} else if (VISIBILITY_LISTENER == propertyKey) {
view.setVisibilityListener(model.get(VISIBILITY_LISTENER));
} else if (INITIAL_SCROLL_INDEX == propertyKey) {
view.scrollToPosition(model.get(INITIAL_SCROLL_INDEX));
}
}
}
......@@ -18,6 +18,12 @@ class TabListContainerProperties {
.WritableObjectPropertyKey<TabListRecyclerView.VisibilityListener> VISIBILITY_LISTENER =
new PropertyModel.WritableObjectPropertyKey<>();
public static final PropertyKey[] ALL_KEYS =
new PropertyKey[] {IS_VISIBLE, IS_INCOGNITO, VISIBILITY_LISTENER};
public static final PropertyModel.WritableIntPropertyKey INITIAL_SCROLL_INDEX =
new PropertyModel.WritableIntPropertyKey();
public static final PropertyModel.WritableBooleanPropertyKey ANIMATE_VISIBILITY_CHANGES =
new PropertyModel.WritableBooleanPropertyKey();
public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {IS_VISIBLE, IS_INCOGNITO,
VISIBILITY_LISTENER, INITIAL_SCROLL_INDEX, ANIMATE_VISIBILITY_CHANGES};
}
......@@ -67,8 +67,9 @@ class TabListRecyclerView extends RecyclerView {
/**
* Start showing the tab list.
* @param animate Whether the visibility change should be animated.
*/
void startShowing() {
void startShowing(boolean animate) {
mListener.startedShowing();
cancelAllAnimations();
setAlpha(0);
......@@ -83,12 +84,14 @@ class TabListRecyclerView extends RecyclerView {
mListener.finishedShowing();
}
});
if (!animate) mFadeInAnimator.end();
}
/**
* Start hiding the tab list.
* @param animate Whether the visibility change should be animated.
*/
void startHiding() {
void startHiding(boolean animate) {
mListener.startedHiding();
cancelAllAnimations();
mFadeOutAnimator = ObjectAnimator.ofFloat(this, View.ALPHA, 0);
......@@ -102,6 +105,7 @@ class TabListRecyclerView extends RecyclerView {
}
});
mFadeOutAnimator.start();
if (!animate) mFadeOutAnimator.end();
}
private void cancelAllAnimations() {
......
......@@ -1041,25 +1041,6 @@ public class ToolbarManager
mToolbar.disableExperimentalButton();
}
/**
* Overrides tab switcher launching behavior.
* @param newClickListener The new {@link OnClickListener} for tab switcher button clicks.
* @param overviewModeBehavior The OverviewModeBehavior to be used for tab switcher states.
*/
public void overrideTabSwitcherBehavior(
OnClickListener newClickListener, OverviewModeBehavior overviewModeBehavior) {
assert mInitializedWithNative;
mToolbar.setTabSwitcherClickListener(newClickListener);
mOverviewModeBehavior.removeOverviewModeObserver(mOverviewModeObserver);
mOverviewModeBehavior = overviewModeBehavior;
mOverviewModeBehavior.addOverviewModeObserver(mOverviewModeObserver);
if (mBottomToolbarCoordinator != null) {
mBottomToolbarCoordinator.overrideTabSwitcherBehavior(
newClickListener, overviewModeBehavior);
}
}
/**
* @return The experimental toolbar button if it exists.
*/
......
......@@ -149,17 +149,6 @@ public class BottomToolbarCoordinator {
return mBrowsingModeCoordinator.getMenuButton();
}
/**
* Overrides tab switcher launching behavior.
* @param newClickListener The new {@link OnClickListener} for tab switcher button clicks.
* @param overviewModeBehavior The OverviewModeBehavior to be used for tab switcher states.
*/
public void overrideTabSwitcherBehavior(
OnClickListener newClickListener, OverviewModeBehavior overviewModeBehavior) {
mBrowsingModeCoordinator.overrideTabSwitcherBehavior(
newClickListener, overviewModeBehavior);
}
/**
* Clean up any state when the bottom toolbar is destroyed.
*/
......
......@@ -190,17 +190,6 @@ public class BrowsingModeBottomToolbarCoordinator {
return mMenuButton;
}
/**
* Overrides tab switcher launching behavior.
* @param newClickListener The new {@link OnClickListener} for tab switcher button clicks.
* @param overviewModeBehavior The OverviewModeBehavior to be used for tab switcher states.
*/
public void overrideTabSwitcherBehavior(
OnClickListener newClickListener, OverviewModeBehavior overviewModeBehavior) {
mTabSwitcherButtonCoordinator.setTabSwitcherListener(newClickListener);
mMediator.setOverviewModeBehavior(overviewModeBehavior);
}
/**
* @return Whether the browsing mode toolbar is visible.
*/
......
......@@ -304,6 +304,7 @@ public class ToolbarControlContainer extends OptimizedFrameLayout implements Con
@Override
public boolean shouldRecognizeSwipe(MotionEvent e1, MotionEvent e2) {
if (FeatureUtilities.isGridTabSwitcherEnabled(getContext())) return false;
if (isOnTabStrip(e1)) return false;
if (mToolbar != null && mToolbar.shouldIgnoreSwipeGesture()) return false;
if (KeyboardVisibilityDelegate.getInstance().isKeyboardShowing(
......
......@@ -303,6 +303,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/compositor/layouts/LayoutRenderHost.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/LayoutUpdateHost.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/OverviewModeBehavior.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/OverviewModeController.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/SceneChangeObserver.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/StaticLayout.java",
"java/src/org/chromium/chrome/browser/compositor/layouts/ToolbarSwipeLayout.java",
......
......@@ -395,7 +395,8 @@ public class BrowserActionActivityTest {
// The Intent of the Browser Actions notification should not toggle overview mode and should
mActivityTestRule.startActivityCompletely(notificationIntent);
Assert.assertFalse(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
Assert.assertFalse(
mActivityTestRule.getActivity().getOverviewModeBehavior().overviewVisible());
Assert.assertNotEquals(prevTabId, mActivityTestRule.getActivity().getActivityTab().getId());
Assert.assertEquals(newTabId, mActivityTestRule.getActivity().getActivityTab().getId());
}
......@@ -459,9 +460,10 @@ public class BrowserActionActivityTest {
mActivityTestRule.startActivityCompletely(notificationIntent);
if (mActivityTestRule.getActivity().isTablet()) {
Assert.assertFalse(
mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
mActivityTestRule.getActivity().getOverviewModeBehavior().overviewVisible());
} else {
Assert.assertTrue(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
Assert.assertTrue(
mActivityTestRule.getActivity().getOverviewModeBehavior().overviewVisible());
}
}
......@@ -571,9 +573,10 @@ public class BrowserActionActivityTest {
// Tab switcher should be shown on phones.
if (mActivityTestRule.getActivity().isTablet()) {
Assert.assertFalse(
mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
mActivityTestRule.getActivity().getOverviewModeBehavior().overviewVisible());
} else {
Assert.assertTrue(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
Assert.assertTrue(
mActivityTestRule.getActivity().getOverviewModeBehavior().overviewVisible());
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment