Commit 312f7ed0 authored by Gang Wu's avatar Gang Wu Committed by Commit Bot

[AppMenu] Custom views highlight their sub view when needed

Lets custom view binders check if their sub view needs to be
highlighted.
This CL needs to be merged to M87 because the CL caused this bug is in
M87.(put R.id.downloads_menu_id into a submenu.)
https://chromium-review.googlesource.com/c/chromium/src/+/2420256

There is also an issue with ChipView, but since ChipView in AppMenu
will be enabled in M88, so we will fix it later.

Bug: 1133105
Change-Id: I7cce43ff0a2c5ce32222141d54ae8ece09bfc3ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2458447Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815173}
parent 5f26c941
...@@ -11,12 +11,14 @@ import android.view.MenuItem; ...@@ -11,12 +11,14 @@ import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.annotation.Nullable;
import androidx.appcompat.content.res.AppCompatResources; import androidx.appcompat.content.res.AppCompatResources;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.app.appmenu.AppMenuPropertiesDelegateImpl.ThreeButtonActionBarType; import org.chromium.chrome.browser.app.appmenu.AppMenuPropertiesDelegateImpl.ThreeButtonActionBarType;
import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler; import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler;
import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder; import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder;
import org.chromium.components.browser_ui.widget.highlight.ViewHighlighter;
import org.chromium.components.browser_ui.widget.text.TextViewWithCompoundDrawables; import org.chromium.components.browser_ui.widget.text.TextViewWithCompoundDrawables;
import org.chromium.ui.widget.ChipView; import org.chromium.ui.widget.ChipView;
...@@ -45,8 +47,9 @@ class ChipViewMenuItemViewBinder implements CustomViewBinder { ...@@ -45,8 +47,9 @@ class ChipViewMenuItemViewBinder implements CustomViewBinder {
} }
@Override @Override
public View getView(MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
assert item.getItemId() == R.id.downloads_row_menu_id assert item.getItemId() == R.id.downloads_row_menu_id
|| item.getItemId() == R.id.all_bookmarks_row_menu_id; || item.getItemId() == R.id.all_bookmarks_row_menu_id;
...@@ -99,6 +102,20 @@ class ChipViewMenuItemViewBinder implements CustomViewBinder { ...@@ -99,6 +102,20 @@ class ChipViewMenuItemViewBinder implements CustomViewBinder {
final MenuItem finalChipViewMenuItem = chipViewMenuItem; final MenuItem finalChipViewMenuItem = chipViewMenuItem;
holder.chipView.setOnClickListener( holder.chipView.setOnClickListener(
v -> appMenuClickHandler.onItemClick(finalChipViewMenuItem)); v -> appMenuClickHandler.onItemClick(finalChipViewMenuItem));
if (highlightedItemId != null && chipViewMenuItem.getItemId() == highlightedItemId) {
// TODO(crbug.com/1136169): Should not remove the background once the bug fixed.
holder.chipView.setBackgroundResource(0);
ViewHighlighter.turnOnHighlight(holder.chipView, true);
} else {
ViewHighlighter.turnOffHighlight(holder.chipView);
}
}
if (highlightedItemId != null && mainMenuItem.getItemId() == highlightedItemId) {
ViewHighlighter.turnOnHighlight(holder.title, false);
} else {
ViewHighlighter.turnOffHighlight(holder.title);
} }
convertView.setEnabled(false); convertView.setEnabled(false);
......
...@@ -10,6 +10,8 @@ import android.view.MenuItem; ...@@ -10,6 +10,8 @@ import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.annotation.Nullable;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler; import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler;
import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder; import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder;
...@@ -32,8 +34,9 @@ class DividerLineMenuItemViewBinder implements CustomViewBinder { ...@@ -32,8 +34,9 @@ class DividerLineMenuItemViewBinder implements CustomViewBinder {
} }
@Override @Override
public View getView(MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
assert item.getItemId() == R.id.divider_line_id; assert item.getItemId() == R.id.divider_line_id;
if (convertView == null) { if (convertView == null) {
......
...@@ -11,6 +11,8 @@ import android.view.MenuItem; ...@@ -11,6 +11,8 @@ import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.annotation.Nullable;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.incognito.IncognitoUtils; import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler; import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler;
...@@ -36,8 +38,9 @@ class IncognitoMenuItemViewBinder implements CustomViewBinder { ...@@ -36,8 +38,9 @@ class IncognitoMenuItemViewBinder implements CustomViewBinder {
} }
@Override @Override
public View getView(MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
assert item.getItemId() == R.id.new_incognito_tab_menu_id; assert item.getItemId() == R.id.new_incognito_tab_menu_id;
IncognitoMenuItemViewHolder holder; IncognitoMenuItemViewHolder holder;
......
...@@ -10,6 +10,8 @@ import android.view.MenuItem; ...@@ -10,6 +10,8 @@ import android.view.MenuItem;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.annotation.Nullable;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler; import org.chromium.chrome.browser.ui.appmenu.AppMenuClickHandler;
import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder; import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder;
...@@ -32,8 +34,9 @@ class ManagedByMenuItemViewBinder implements CustomViewBinder { ...@@ -32,8 +34,9 @@ class ManagedByMenuItemViewBinder implements CustomViewBinder {
} }
@Override @Override
public View getView(MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
assert item.getItemId() == R.id.managed_by_menu_id; assert item.getItemId() == R.id.managed_by_menu_id;
if (convertView == null) { if (convertView == null) {
......
...@@ -15,6 +15,7 @@ import android.view.ViewGroup; ...@@ -15,6 +15,7 @@ import android.view.ViewGroup;
import android.widget.ImageView; import android.widget.ImageView;
import android.widget.TextView; import android.widget.TextView;
import androidx.annotation.Nullable;
import androidx.appcompat.content.res.AppCompatResources; import androidx.appcompat.content.res.AppCompatResources;
import androidx.core.graphics.drawable.DrawableCompat; import androidx.core.graphics.drawable.DrawableCompat;
...@@ -41,8 +42,9 @@ class UpdateMenuItemViewBinder implements CustomViewBinder { ...@@ -41,8 +42,9 @@ class UpdateMenuItemViewBinder implements CustomViewBinder {
} }
@Override @Override
public View getView(MenuItem item, View convertView, ViewGroup parent, LayoutInflater inflater, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
assert item.getItemId() == R.id.update_menu_id; assert item.getItemId() == R.id.update_menu_id;
UpdateMenuItemViewHolder holder; UpdateMenuItemViewHolder holder;
if (convertView == null || !(convertView.getTag() instanceof UpdateMenuItemViewHolder)) { if (convertView == null || !(convertView.getTag() instanceof UpdateMenuItemViewHolder)) {
......
...@@ -17,6 +17,8 @@ import org.chromium.chrome.browser.feature_engagement.ScreenshotMonitor; ...@@ -17,6 +17,8 @@ import org.chromium.chrome.browser.feature_engagement.ScreenshotMonitor;
import org.chromium.chrome.browser.feature_engagement.ScreenshotMonitorDelegate; import org.chromium.chrome.browser.feature_engagement.ScreenshotMonitorDelegate;
import org.chromium.chrome.browser.feature_engagement.ScreenshotTabObserver; import org.chromium.chrome.browser.feature_engagement.ScreenshotTabObserver;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher; import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver; import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver;
import org.chromium.chrome.browser.net.spdyproxy.DataReductionProxySettings; import org.chromium.chrome.browser.net.spdyproxy.DataReductionProxySettings;
...@@ -270,12 +272,17 @@ public class ToolbarButtonInProductHelpController ...@@ -270,12 +272,17 @@ public class ToolbarButtonInProductHelpController
return; return;
} }
final Integer offlinePageId =
CachedFeatureFlags.isEnabled(
ChromeFeatureList.TABBED_APP_OVERFLOW_MENU_THREE_BUTTON_ACTIONBAR)
? R.id.offline_page_chip_id
: R.id.offline_page_id;
mUserEducationHelper.requestShowIPH( mUserEducationHelper.requestShowIPH(
new IPHCommandBuilder(mActivity.getResources(), featureName, new IPHCommandBuilder(mActivity.getResources(), featureName,
R.string.iph_download_page_for_offline_usage_text, R.string.iph_download_page_for_offline_usage_text,
R.string.iph_download_page_for_offline_usage_accessibility_text) R.string.iph_download_page_for_offline_usage_accessibility_text)
.setOnShowCallback( .setOnShowCallback(() -> turnOnHighlightForMenuItem(offlinePageId, true))
() -> turnOnHighlightForMenuItem(R.id.offline_page_id, true))
.setOnDismissCallback(this::turnOffHighlightForMenuItem) .setOnDismissCallback(this::turnOffHighlightForMenuItem)
.setAnchorView(mActivity.getToolbarManager().getMenuButtonView()) .setAnchorView(mActivity.getToolbarManager().getMenuButtonView())
.build()); .build());
......
...@@ -288,8 +288,8 @@ class AppMenuAdapter extends BaseAdapter { ...@@ -288,8 +288,8 @@ class AppMenuAdapter extends BaseAdapter {
convertView = null; convertView = null;
} }
convertView = binder.getView( convertView = binder.getView(item, convertView, parent, mInflater,
item, convertView, parent, mInflater, mAppMenuClickHandler); mAppMenuClickHandler, mHighlightedItemId);
if (binder.supportsEnterAnimation(item.getItemId())) { if (binder.supportsEnterAnimation(item.getItemId())) {
convertView.setTag(R.id.menu_item_enter_anim_id, convertView.setTag(R.id.menu_item_enter_anim_id,
......
...@@ -538,7 +538,8 @@ public class AppMenuAdapterTest extends DummyUiActivityTestCase { ...@@ -538,7 +538,8 @@ public class AppMenuAdapterTest extends DummyUiActivityTestCase {
@Override @Override
public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
int itemId = item.getItemId(); int itemId = item.getItemId();
Assert.assertTrue("getView called for incorrect item", Assert.assertTrue("getView called for incorrect item",
itemId == supportedId1 || itemId == supportedId2 || itemId == supportedId3); itemId == supportedId1 || itemId == supportedId2 || itemId == supportedId3);
...@@ -586,7 +587,8 @@ public class AppMenuAdapterTest extends DummyUiActivityTestCase { ...@@ -586,7 +587,8 @@ public class AppMenuAdapterTest extends DummyUiActivityTestCase {
@Override @Override
public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent, public View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler) { LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId) {
int itemId = item.getItemId(); int itemId = item.getItemId();
Assert.assertTrue("getView called for incorrect item", itemId == supportedId1); Assert.assertTrue("getView called for incorrect item", itemId == supportedId1);
......
...@@ -49,10 +49,15 @@ public interface CustomViewBinder { ...@@ -49,10 +49,15 @@ public interface CustomViewBinder {
* @param parent The parent that this view will eventually be attached to. * @param parent The parent that this view will eventually be attached to.
* @param inflater A {@link LayoutInflater} to use when inflating new views. * @param inflater A {@link LayoutInflater} to use when inflating new views.
* @param appMenuClickHandler A {@link AppMenuClickHandler} to handle click events in the view. * @param appMenuClickHandler A {@link AppMenuClickHandler} to handle click events in the view.
* @param highlightedItemId The resource id of the menu item that should be highlighted.
* Can be {@code null} if no item should be highlighted. Note that
* the custom view binder is responsible for highlighting the
* appropriate sub-view based on this id.
* @return A View corresponding to the provided menu item. * @return A View corresponding to the provided menu item.
*/ */
View getView(MenuItem item, @Nullable View convertView, ViewGroup parent, View getView(MenuItem item, @Nullable View convertView, ViewGroup parent,
LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler); LayoutInflater inflater, AppMenuClickHandler appMenuClickHandler,
@Nullable Integer highlightedItemId);
/** /**
* Determines whether the enter animation should be applied to the menu item matching the * Determines whether the enter animation should be applied to the menu item matching the
......
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