Commit 6ee31dce authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Make popup tab strip adapt to bottom sheet

Currently, when triggering contents in bottom sheet, the popup tab
strip shows above bottom sheet, which is unexpected. This CL makes
popup tab strip hides when bottom sheet shows and restores its original
visibility when bottom sheet hides.

Bug: 1045974
Change-Id: Icd5fe051d9eaa2cdff6da22ae19e4515cdde8738
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036936Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739010}
parent 92a12e82
...@@ -51,7 +51,7 @@ public class TabGroupPopupUiCoordinator ...@@ -51,7 +51,7 @@ public class TabGroupPopupUiCoordinator
model, mTabGroupPopupUiParent, TabGroupPopupUiViewBinder::bind); model, mTabGroupPopupUiParent, TabGroupPopupUiViewBinder::bind);
mMediator = new TabGroupPopupUiMediator(model, activity.getTabModelSelector(), mMediator = new TabGroupPopupUiMediator(model, activity.getTabModelSelector(),
activity.getOverviewModeBehavior(), activity.getFullscreenManager(), this, activity.getOverviewModeBehavior(), activity.getFullscreenManager(), this,
mTabGroupUiCoordinator); mTabGroupUiCoordinator, activity.getBottomSheetController());
mMediator.onAnchorViewChanged(mAnchorView, mAnchorView.getId()); mMediator.onAnchorViewChanged(mAnchorView, mAnchorView.getId());
} }
......
...@@ -17,6 +17,9 @@ import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver; ...@@ -17,6 +17,9 @@ import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelObserver; import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter; import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
import org.chromium.ui.KeyboardVisibilityDelegate; import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
...@@ -48,18 +51,22 @@ public class TabGroupPopupUiMediator { ...@@ -48,18 +51,22 @@ public class TabGroupPopupUiMediator {
private final KeyboardVisibilityDelegate.KeyboardVisibilityListener mKeyboardVisibilityListener; private final KeyboardVisibilityDelegate.KeyboardVisibilityListener mKeyboardVisibilityListener;
private final TabGroupPopUiUpdater mUiUpdater; private final TabGroupPopUiUpdater mUiUpdater;
private final TabGroupUiMediator.TabGroupUiController mTabGroupUiController; private final TabGroupUiMediator.TabGroupUiController mTabGroupUiController;
private final BottomSheetController mBottomSheetController;
private final BottomSheetObserver mBottomSheetObserver;
private boolean mIsOverviewModeVisible; private boolean mIsOverviewModeVisible;
TabGroupPopupUiMediator(PropertyModel model, TabModelSelector tabModelSelector, TabGroupPopupUiMediator(PropertyModel model, TabModelSelector tabModelSelector,
OverviewModeBehavior overviewModeBehavior, ChromeFullscreenManager fullscreenManager, OverviewModeBehavior overviewModeBehavior, ChromeFullscreenManager fullscreenManager,
TabGroupPopUiUpdater updater, TabGroupUiMediator.TabGroupUiController controller) { TabGroupPopUiUpdater updater, TabGroupUiMediator.TabGroupUiController controller,
BottomSheetController bottomSheetController) {
mModel = model; mModel = model;
mTabModelSelector = tabModelSelector; mTabModelSelector = tabModelSelector;
mOverviewModeBehavior = overviewModeBehavior; mOverviewModeBehavior = overviewModeBehavior;
mFullscreenManager = fullscreenManager; mFullscreenManager = fullscreenManager;
mUiUpdater = updater; mUiUpdater = updater;
mTabGroupUiController = controller; mTabGroupUiController = controller;
mBottomSheetController = bottomSheetController;
mFullscreenListener = new ChromeFullscreenManager.FullscreenListener() { mFullscreenListener = new ChromeFullscreenManager.FullscreenListener() {
@Override @Override
...@@ -160,6 +167,25 @@ public class TabGroupPopupUiMediator { ...@@ -160,6 +167,25 @@ public class TabGroupPopupUiMediator {
KeyboardVisibilityDelegate.getInstance().addKeyboardVisibilityListener( KeyboardVisibilityDelegate.getInstance().addKeyboardVisibilityListener(
mKeyboardVisibilityListener); mKeyboardVisibilityListener);
mBottomSheetObserver = new EmptyBottomSheetObserver() {
private Boolean mWasShowingStrip;
@Override
public void onSheetStateChanged(int newState) {
if (newState == BottomSheetController.SheetState.HIDDEN) {
if (mWasShowingStrip != null && mWasShowingStrip) {
maybeShowTabStrip();
}
mWasShowingStrip = null;
} else {
if (mWasShowingStrip == null) {
mWasShowingStrip = isTabStripShowing();
}
hideTabStrip();
}
}
};
mBottomSheetController.addObserver(mBottomSheetObserver);
// TODO(yuezhanggg): Reset the strip with empty tab list as well. // TODO(yuezhanggg): Reset the strip with empty tab list as well.
mTabGroupUiController.setupLeftButtonOnClickListener(view -> hideTabStrip()); mTabGroupUiController.setupLeftButtonOnClickListener(view -> hideTabStrip());
} }
...@@ -215,6 +241,7 @@ public class TabGroupPopupUiMediator { ...@@ -215,6 +241,7 @@ public class TabGroupPopupUiMediator {
mTabModelSelector.getTabModelFilterProvider().removeTabModelFilterObserver( mTabModelSelector.getTabModelFilterProvider().removeTabModelFilterObserver(
mTabModelObserver); mTabModelObserver);
mFullscreenManager.removeListener(mFullscreenListener); mFullscreenManager.removeListener(mFullscreenListener);
mBottomSheetController.removeObserver(mBottomSheetObserver);
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -41,6 +41,8 @@ import org.chromium.chrome.browser.tabmodel.TabModelObserver; ...@@ -41,6 +41,8 @@ import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorImpl; import org.chromium.chrome.browser.tabmodel.TabModelSelectorImpl;
import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter; import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
import org.chromium.chrome.browser.toolbar.top.ToolbarPhone; import org.chromium.chrome.browser.toolbar.top.ToolbarPhone;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.testing.local.LocalRobolectricTestRunner; import org.chromium.testing.local.LocalRobolectricTestRunner;
...@@ -89,6 +91,8 @@ public class TabGroupPopupUiMediatorUnitTest { ...@@ -89,6 +91,8 @@ public class TabGroupPopupUiMediatorUnitTest {
ToolbarPhone mTopAnchorView; ToolbarPhone mTopAnchorView;
@Mock @Mock
FrameLayout mBottomAnchorView; FrameLayout mBottomAnchorView;
@Mock
BottomSheetController mBottomSheetController;
@Captor @Captor
ArgumentCaptor<TabModelObserver> mTabModelObserverCaptor; ArgumentCaptor<TabModelObserver> mTabModelObserverCaptor;
@Captor @Captor
...@@ -98,6 +102,8 @@ public class TabGroupPopupUiMediatorUnitTest { ...@@ -98,6 +102,8 @@ public class TabGroupPopupUiMediatorUnitTest {
@Captor @Captor
ArgumentCaptor<KeyboardVisibilityDelegate.KeyboardVisibilityListener> ArgumentCaptor<KeyboardVisibilityDelegate.KeyboardVisibilityListener>
mKeyboardVisibilityListenerCaptor; mKeyboardVisibilityListenerCaptor;
@Captor
ArgumentCaptor<BottomSheetObserver> mBottomSheetObserver;
private TabImpl mTab1; private TabImpl mTab1;
private TabImpl mTab2; private TabImpl mTab2;
...@@ -129,11 +135,12 @@ public class TabGroupPopupUiMediatorUnitTest { ...@@ -129,11 +135,12 @@ public class TabGroupPopupUiMediatorUnitTest {
doNothing() doNothing()
.when(mKeyboardVisibilityDelegate) .when(mKeyboardVisibilityDelegate)
.addKeyboardVisibilityListener(mKeyboardVisibilityListenerCaptor.capture()); .addKeyboardVisibilityListener(mKeyboardVisibilityListenerCaptor.capture());
doNothing().when(mBottomSheetController).addObserver(mBottomSheetObserver.capture());
KeyboardVisibilityDelegate.setInstance(mKeyboardVisibilityDelegate); KeyboardVisibilityDelegate.setInstance(mKeyboardVisibilityDelegate);
mModel = new PropertyModel(TabGroupPopupUiProperties.ALL_KEYS); mModel = new PropertyModel(TabGroupPopupUiProperties.ALL_KEYS);
mMediator = new TabGroupPopupUiMediator(mModel, mTabModelSelector, mOverviewModeBehavior, mMediator = new TabGroupPopupUiMediator(mModel, mTabModelSelector, mOverviewModeBehavior,
mChromeFullscreenManager, mUpdater, mTabGroupUiController); mChromeFullscreenManager, mUpdater, mTabGroupUiController, mBottomSheetController);
} }
@After @After
...@@ -470,6 +477,53 @@ public class TabGroupPopupUiMediatorUnitTest { ...@@ -470,6 +477,53 @@ public class TabGroupPopupUiMediatorUnitTest {
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false)); assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false));
} }
@Test
public void testShowBottomSheet_HideStrip() {
// Mock that the strip is showing.
mModel.set(TabGroupPopupUiProperties.IS_VISIBLE, true);
// Show bottom sheet.
mBottomSheetObserver.getValue().onSheetStateChanged(BottomSheetController.SheetState.PEEK);
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false));
}
@Test
public void testHideBottomSheet_ShowStrip() {
// Mock that the strip is showing before showing the bottom sheet. tab1 and tab2 are in the
// same group, and tab1 is the current tab.
mModel.set(TabGroupPopupUiProperties.IS_VISIBLE, true);
List<Tab> tabGroup = new ArrayList<>(Arrays.asList(mTab1, mTab2));
createTabGroup(tabGroup, TAB1_ID);
doReturn(mTab1).when(mTabModelSelector).getCurrentTab();
// Hide the bottom sheet after showing it.
mBottomSheetObserver.getValue().onSheetStateChanged(BottomSheetController.SheetState.PEEK);
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false));
mBottomSheetObserver.getValue().onSheetStateChanged(
BottomSheetController.SheetState.HIDDEN);
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(true));
}
@Test
public void testHideBottomSheet_NotReshowStrip() {
// Mock that the strip is hidden before showing the bottom sheet. tab1 and tab2 are in the
// same group, and tab1 is the current tab.
mModel.set(TabGroupPopupUiProperties.IS_VISIBLE, false);
List<Tab> tabGroup = new ArrayList<>(Arrays.asList(mTab1, mTab2));
createTabGroup(tabGroup, TAB1_ID);
doReturn(mTab1).when(mTabModelSelector).getCurrentTab();
// Hide the bottom sheet after showing it.
mBottomSheetObserver.getValue().onSheetStateChanged(BottomSheetController.SheetState.PEEK);
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false));
mBottomSheetObserver.getValue().onSheetStateChanged(
BottomSheetController.SheetState.HIDDEN);
assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(false));
}
@Test @Test
public void testAnchorViewChange_TopToolbar() { public void testAnchorViewChange_TopToolbar() {
mMediator.onAnchorViewChanged(mTopAnchorView, R.id.toolbar); mMediator.onAnchorViewChanged(mTopAnchorView, R.id.toolbar);
......
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