Commit 7bf9f7b9 authored by Patrick Noland's avatar Patrick Noland Committed by Commit Bot

[ToolbarMVC] Use MenuButton in CustomTabToolbar

Current CCToolbar uses a bare ImageButton instead of MenuButton. In the
linked bugs, this causes us to fail to connect the button with the
AppMenuButtonHelper and leaves the menu un-anchored. Rather than writing
special logic to set the AppMenuButtonHelper for the bare image button,
it seems cleaner to just use MenuButton on the CCToolbar and have it be
safe/correct to use in that scenario (no update badge).

Bug:1125048,1125075

Change-Id: I041a95467252d64af9559e40342c424645983e4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399315
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805346}
parent 1e81bf1f
......@@ -96,14 +96,26 @@
android:layout_gravity="center_vertical|end"
android:gravity="center_vertical"
android:orientation="horizontal" />
<org.chromium.ui.widget.ChromeImageButton
android:id="@+id/menu_button"
style="@style/ToolbarButton"
<org.chromium.chrome.browser.toolbar.menu_button.MenuButton
android:layout_height="wrap_content"
android:layout_gravity="center_vertical|end"
android:layout_width="42dp"
android:paddingEnd="4dp"
android:src="@drawable/ic_more_vert_24dp"
android:clickable="true"
android:focusable="true"
android:contentDescription="@string/accessibility_toolbar_btn_menu"
android:background="@null"
app:tint="@color/default_icon_color_tint_list" />
android:id="@+id/menu_button_wrapper">
<org.chromium.ui.widget.ChromeImageButton
android:id="@+id/menu_button"
style="@style/ToolbarButton"
android:src="@drawable/ic_more_vert_24dp"
android:layout_gravity="center"
android:importantForAccessibility="no"
android:background="@null"
app:tint="@color/default_icon_color_tint_list" />
</org.chromium.chrome.browser.toolbar.menu_button.MenuButton>
</org.chromium.chrome.browser.toolbar.top.CustomTabToolbar>
......@@ -79,14 +79,6 @@ public class MenuButton extends FrameLayout implements TintObserver {
mMenuImageButton.setAccessibilityDelegate(mAppMenuButtonHelper.getAccessibilityDelegate());
}
public AppMenuButtonHelper getAppMenuButtonHelper() {
return mAppMenuButtonHelper;
}
public View getMenuBadge() {
return mUpdateBadgeView;
}
public ImageButton getImageButton() {
return mMenuImageButton;
}
......@@ -99,6 +91,7 @@ public class MenuButton extends FrameLayout implements TintObserver {
* TODO(crbug.com/865801): Clean this up when MenuButton and UpdateMenuItemHelper is MVCed.
*/
private void setUpdateBadgeVisibility(boolean visible) {
if (mUpdateBadgeView == null) return;
mUpdateBadgeView.setVisibility(visible ? View.VISIBLE : View.GONE);
if (visible) updateImageResources();
updateContentDescription(visible);
......@@ -130,7 +123,7 @@ public class MenuButton extends FrameLayout implements TintObserver {
// As an optimization, don't re-calculate drawable state for the update badge unless we
// intend to actually show it.
MenuButtonState buttonState = UpdateMenuItemHelper.getInstance().getUiState().buttonState;
if (buttonState == null) return;
if (buttonState == null || mUpdateBadgeView == null) return;
@DrawableRes
int drawable = mUseLightDrawables ? buttonState.lightBadgeIcon : buttonState.darkBadgeIcon;
mUpdateBadgeView.setImageDrawable(
......@@ -248,7 +241,7 @@ public class MenuButton extends FrameLayout implements TintObserver {
* @return Whether the update badge is showing.
*/
public boolean isShowingAppMenuUpdateBadge() {
return mUpdateBadgeView.getVisibility() == View.VISIBLE;
return mUpdateBadgeView != null && mUpdateBadgeView.getVisibility() == View.VISIBLE;
}
private static boolean isBadgeAvailable() {
......
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