Skip to content

GitLab

T
tangled
Commit a65f80f8 authored by Patrick Noland's avatar Patrick Noland Committed by Commit Bot
Browse files
Options

[ToolbarMVC] Remove direct MenuButton interactions from Tabswitcher 

This only requires giving TabSwitcherModeTTCoordinatorPhone a
MenuButtonCoordinator and setting the MenuButton view once inflation happens.
The logic removed (tinting, setting the onClick/accessibilityDelegate)
is now handled internally by MenuButtonCoordinator/MenuButton.

Bug: 1086676
Change-Id: Iafbe6539974406fcd3db0f8a904e581d355a872b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2411401
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810750}
parent 5fa65816
Hide whitespace changes
Inline Side-by-side
Showing with 16 additions and 58 deletions
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/TabSwitcherModeTTCoordinatorPhone.java
View file @ a65f80f8
......@@ -11,6 +11,7 @@ import android.view.ViewStub;
import androidx.annotation.Nullable;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.tab.Tab;
......@@ -21,7 +22,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.toolbar.TabCountProvider;
import org.chromium.chrome.browser.ui.appmenu.AppMenuButtonHelper;
import org.chromium.chrome.browser.toolbar.menu_button.MenuButtonCoordinator;
/**
* The coordinator for the tab switcher mode top toolbar shown on phones, responsible for
......@@ -34,12 +35,12 @@ class TabSwitcherModeTTCoordinatorPhone {
// TODO(twellington): Create a model to hold all of these properties. Consider using
// LazyConstructionPropertyMcp to collect all of the properties since it is designed to
// aggregate properties and bind them to a view the first time it's shown.
private AppMenuButtonHelper mAppMenuButtonHelper;
private View.OnClickListener mTabSwitcherListener;
private View.OnClickListener mNewTabListener;
private TabCountProvider mTabCountProvider;
private TabModelSelector mTabModelSelector;
private IncognitoStateProvider mIncognitoStateProvider;
private MenuButtonCoordinator mMenuButtonCoordinator;
private boolean mAccessibilityEnabled;
private TabSwitcherModeTTPhone mTabSwitcherModeToolbar;
......@@ -47,8 +48,10 @@ class TabSwitcherModeTTCoordinatorPhone {
@Nullable
private TabModelObserver mTabModelObserver;
TabSwitcherModeTTCoordinatorPhone(ViewStub tabSwitcherToolbarStub) {
TabSwitcherModeTTCoordinatorPhone(
ViewStub tabSwitcherToolbarStub, MenuButtonCoordinator menuButtonCoordinator) {
mTabSwitcherToolbarStub = tabSwitcherToolbarStub;
mMenuButtonCoordinator = menuButtonCoordinator;
}
/**
......@@ -62,6 +65,10 @@ class TabSwitcherModeTTCoordinatorPhone {
if (mTabModelSelector != null && mTabModelObserver != null) {
mTabModelSelector.getModel(true).removeObserver(mTabModelObserver);
}
if (mMenuButtonCoordinator != null) {
mMenuButtonCoordinator.destroy();
mMenuButtonCoordinator = null;
}
}
/**
......@@ -80,16 +87,6 @@ class TabSwitcherModeTTCoordinatorPhone {
}
}
/**
* @param appMenuButtonHelper The helper for managing menu button interactions.
*/
void setAppMenuButtonHelper(AppMenuButtonHelper appMenuButtonHelper) {
mAppMenuButtonHelper = appMenuButtonHelper;
if (mTabSwitcherModeToolbar != null) {
mTabSwitcherModeToolbar.setAppMenuButtonHelper(appMenuButtonHelper);
}
}
/**
* Sets the OnClickListener that will be notified when the TabSwitcher button is pressed.
* @param listener The callback that will be notified when the TabSwitcher button is pressed.
......@@ -180,11 +177,10 @@ class TabSwitcherModeTTCoordinatorPhone {
});
}
/**
* @param isVisible Whether the bottom toolbar is visible.
*/
private void initializeTabSwitcherToolbar() {
mTabSwitcherModeToolbar = (TabSwitcherModeTTPhone) mTabSwitcherToolbarStub.inflate();
mMenuButtonCoordinator.setMenuButton(
mTabSwitcherModeToolbar.findViewById(R.id.menu_button_wrapper));
// It's expected that these properties are set by the time the tab switcher is entered.
assert mTabSwitcherListener != null;
......@@ -193,9 +189,6 @@ class TabSwitcherModeTTCoordinatorPhone {
assert mNewTabListener != null;
mTabSwitcherModeToolbar.setOnNewTabClickHandler(mNewTabListener);
assert mAppMenuButtonHelper != null;
mTabSwitcherModeToolbar.setAppMenuButtonHelper(mAppMenuButtonHelper);
assert mTabCountProvider != null;
mTabSwitcherModeToolbar.setTabCountProvider(mTabCountProvider);
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/TabSwitcherModeTTPhone.java
View file @ a65f80f8
......@@ -12,11 +12,8 @@ import android.graphics.Color;
import android.util.AttributeSet;
import android.view.View;
import android.view.ViewStub;
import androidx.annotation.Nullable;
import androidx.appcompat.content.res.AppCompatResources;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
......@@ -27,8 +24,6 @@ import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.toolbar.IncognitoToggleTabLayout;
import org.chromium.chrome.browser.toolbar.NewTabButton;
import org.chromium.chrome.browser.toolbar.TabCountProvider;
import org.chromium.chrome.browser.toolbar.menu_button.MenuButton;
import org.chromium.chrome.browser.ui.appmenu.AppMenuButtonHelper;
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.components.browser_ui.widget.animation.CancelAwareAnimatorListener;
import org.chromium.components.browser_ui.widget.animation.Interpolators;
......@@ -51,7 +46,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
// The following three buttons are not used when Duet is enabled.
private @Nullable NewTabButton mNewTabImageButton;
private @Nullable MenuButton mMenuButton;
private @Nullable ToggleTabStackButton mToggleTabStackButton;
private int mPrimaryColor;
......@@ -74,7 +68,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
mNewTabImageButton = findViewById(R.id.new_tab_button);
mNewTabViewButton = findViewById(R.id.new_tab_view);
mMenuButton = findViewById(R.id.menu_button_wrapper);
mToggleTabStackButton = findViewById(R.id.tab_switcher_mode_tab_switcher_button);
// TODO(twellington): Try to make NewTabButton responsible for handling its own clicks.
......@@ -117,9 +110,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
mIncognitoToggleTabLayout.destroy();
mIncognitoToggleTabLayout = null;
}
if (mMenuButton != null) {
mMenuButton = null;
}
}
/**
......@@ -178,17 +168,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
if (DeviceClassManager.enableAccessibilityLayout()) mVisiblityAnimator.end();
}
/**
* @param appMenuButtonHelper The helper for managing menu button interactions.
*/
void setAppMenuButtonHelper(AppMenuButtonHelper appMenuButtonHelper) {
if (mMenuButton == null) return;
mMenuButton.getImageButton().setOnTouchListener(appMenuButtonHelper);
mMenuButton.getImageButton().setAccessibilityDelegate(
appMenuButtonHelper.getAccessibilityDelegate());
}
/**
* Sets the OnClickListener that will be notified when the TabSwitcher button is pressed.
* @param listener The callback that will be notified when the TabSwitcher button is pressed.
......@@ -285,12 +264,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
}
}
private void setMenuButtonVisibility(boolean isButtonVisible) {
if (mMenuButton != null) {
mMenuButton.setVisibility(isButtonVisible ? VISIBLE : GONE);
}
}
private void updatePrimaryColorAndTint() {
int primaryColor = getToolbarColorForCurrentState();
if (mPrimaryColor != primaryColor) {
......@@ -322,11 +295,6 @@ public class TabSwitcherModeTTPhone extends OptimizedFrameLayout
getContext(), R.color.default_icon_color_tint_list);
}
ColorStateList tintList = useLightIcons ? mLightIconTint : mDarkIconTint;
if (mMenuButton != null) {
ApiCompatibilityUtils.setImageTintList(mMenuButton.getImageButton(), tintList);
}
if (mToggleTabStackButton != null) {
mToggleTabStackButton.setUseLightDrawables(useLightIcons);
}
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/TopToolbarCoordinator.java
View file @ a65f80f8
......@@ -106,7 +106,7 @@ public class TopToolbarCoordinator implements Toolbar {
ThemeColorProvider normalThemeColorProvider,
ThemeColorProvider overviewThemeColorProvider,
MenuButtonCoordinator browsingModeMenuButtonCoordinator,
MenuButtonCoordinator startSurfaceMenuButtonCoordinator,
MenuButtonCoordinator overviewModeMenuButtonCoordinator,
ObservableSupplier<AppMenuButtonHelper> appMenuButtonHelperSupplier,
ObservableSupplier<TabModelSelector> tabModelSelectorSupplier,
ObservableSupplier<Boolean> homeButtonVisibilitySupplier,
......@@ -129,11 +129,11 @@ public class TopToolbarCoordinator implements Toolbar {
controlContainer.getRootView().findViewById(R.id.tab_switcher_toolbar_stub),
userEducationHelper, overviewModeBehaviorSupplier,
identityDiscStateSupplier, overviewThemeColorProvider,
startSurfaceMenuButtonCoordinator, identityDiscButtonSupplier);
overviewModeMenuButtonCoordinator, identityDiscButtonSupplier);
} else {
mTabSwitcherModeCoordinatorPhone = new TabSwitcherModeTTCoordinatorPhone(
controlContainer.getRootView().findViewById(
R.id.tab_switcher_toolbar_stub));
controlContainer.getRootView().findViewById(R.id.tab_switcher_toolbar_stub),
overviewModeMenuButtonCoordinator);
}
}
controlContainer.setToolbar(this);
......@@ -151,9 +151,6 @@ public class TopToolbarCoordinator implements Toolbar {
*/
public void setAppMenuButtonHelper(AppMenuButtonHelper appMenuButtonHelper) {
mToolbarLayout.setAppMenuButtonHelper(appMenuButtonHelper);
if (mTabSwitcherModeCoordinatorPhone != null) {
mTabSwitcherModeCoordinatorPhone.setAppMenuButtonHelper(appMenuButtonHelper);
}
}
/**
......
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