Commit 9146de15 authored by martiw's avatar martiw Committed by Commit bot

Allow to use measured width in translate overflow menu

When showing the overflow menu, we should use measured width (by
setting fixedWidth to false) to make sure all items fit inside the
menu.
When showing the language menu (with 100+ languages), we should use the
hardcoded width (fixedWidth = true) so that it won't take a long time
to measure all item widths.

BUG=709964

Review-Url: https://codereview.chromium.org/2824083003
Cr-Commit-Position: refs/heads/master@{#468518}
parent dda54d94
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2017 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. -->
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:chrome="http://schemas.android.com/apk/res-auto"
style="@style/AppMenuItem"
android:layout_width="match_parent"
android:layout_height="?android:attr/listPreferredItemHeightSmall" >
<org.chromium.chrome.browser.widget.TintedImageView
android:id="@+id/menu_item_icon"
android:src="@drawable/ic_check_googblue_24dp"
android:layout_width="24dp"
android:layout_height="match_parent"
android:layout_gravity="end"
android:gravity="center_vertical"
chrome:tint="@color/dark_mode_tint" />
<TextView
android:id="@+id/menu_item_text"
android:textAppearance="?android:attr/textAppearanceLargePopupMenu"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:layout_gravity="start"
android:gravity="center_vertical"
android:singleLine="true"
android:paddingStart="16dp" />
</LinearLayout>
...@@ -15,6 +15,7 @@ import org.chromium.chrome.R; ...@@ -15,6 +15,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.translate.TranslateMenu; import org.chromium.chrome.browser.infobar.translate.TranslateMenu;
import org.chromium.chrome.browser.infobar.translate.TranslateMenuHelper; import org.chromium.chrome.browser.infobar.translate.TranslateMenuHelper;
import org.chromium.chrome.browser.infobar.translate.TranslateTabLayout; import org.chromium.chrome.browser.infobar.translate.TranslateTabLayout;
import org.chromium.chrome.browser.widget.TintedImageButton;
import org.chromium.ui.widget.Toast; import org.chromium.ui.widget.Toast;
/** /**
...@@ -33,7 +34,12 @@ class TranslateCompactInfoBar extends InfoBar ...@@ -33,7 +34,12 @@ class TranslateCompactInfoBar extends InfoBar
private long mNativeTranslateInfoBarPtr; private long mNativeTranslateInfoBarPtr;
private TranslateTabLayout mTabLayout; private TranslateTabLayout mTabLayout;
private TranslateMenuHelper mMenuHelper; // Need 2 instances of TranslateMenuHelper to prevent a race condition bug which happens when
// showing language menu after dismissing overflow menu.
private TranslateMenuHelper mOverflowMenuHelper;
private TranslateMenuHelper mLanguageMenuHelper;
private TintedImageButton mMenuButton;
@CalledByNative @CalledByNative
private static InfoBar create(int initialStep, String sourceLanguageCode, private static InfoBar create(int initialStep, String sourceLanguageCode,
...@@ -74,21 +80,35 @@ class TranslateCompactInfoBar extends InfoBar ...@@ -74,21 +80,35 @@ class TranslateCompactInfoBar extends InfoBar
mTabLayout.addOnTabSelectedListener(this); mTabLayout.addOnTabSelectedListener(this);
content.findViewById(R.id.translate_infobar_menu_button) mMenuButton = (TintedImageButton) content.findViewById(R.id.translate_infobar_menu_button);
.setOnClickListener(new OnClickListener() { mMenuButton.setOnClickListener(new OnClickListener() {
@Override @Override
public void onClick(View v) { public void onClick(View v) {
initMenuHelper(v); initMenuHelper(TranslateMenu.MENU_OVERFLOW);
mMenuHelper.show(TranslateMenu.MENU_OVERFLOW); mOverflowMenuHelper.show(TranslateMenu.MENU_OVERFLOW);
} }
}); });
parent.addContent(content, 1.0f); parent.addContent(content, 1.0f);
} }
private void initMenuHelper(View anchorView) { private void initMenuHelper(int menuType) {
if (mMenuHelper == null) { switch (menuType) {
mMenuHelper = new TranslateMenuHelper(getContext(), anchorView, mOptions, this); case TranslateMenu.MENU_OVERFLOW:
if (mOverflowMenuHelper == null) {
mOverflowMenuHelper =
new TranslateMenuHelper(getContext(), mMenuButton, mOptions, this);
}
return;
case TranslateMenu.MENU_TARGET_LANGUAGE:
case TranslateMenu.MENU_SOURCE_LANGUAGE:
if (mLanguageMenuHelper == null) {
mLanguageMenuHelper =
new TranslateMenuHelper(getContext(), mMenuButton, mOptions, this);
}
return;
default:
assert false : "Unsupported Menu Item Id";
} }
} }
...@@ -153,7 +173,8 @@ class TranslateCompactInfoBar extends InfoBar ...@@ -153,7 +173,8 @@ class TranslateCompactInfoBar extends InfoBar
public void onOverflowMenuItemClicked(int itemId) { public void onOverflowMenuItemClicked(int itemId) {
switch (itemId) { switch (itemId) {
case TranslateMenu.ID_OVERFLOW_MORE_LANGUAGE: case TranslateMenu.ID_OVERFLOW_MORE_LANGUAGE:
mMenuHelper.show(TranslateMenu.MENU_TARGET_LANGUAGE); initMenuHelper(TranslateMenu.MENU_TARGET_LANGUAGE);
mLanguageMenuHelper.show(TranslateMenu.MENU_TARGET_LANGUAGE);
return; return;
case TranslateMenu.ID_OVERFLOW_ALWAYS_TRANSLATE: case TranslateMenu.ID_OVERFLOW_ALWAYS_TRANSLATE:
nativeApplyBoolTranslateOption( nativeApplyBoolTranslateOption(
...@@ -171,7 +192,8 @@ class TranslateCompactInfoBar extends InfoBar ...@@ -171,7 +192,8 @@ class TranslateCompactInfoBar extends InfoBar
showSnackbar(TranslateSnackbarType.NEVER_TRANSLATE_SITE); showSnackbar(TranslateSnackbarType.NEVER_TRANSLATE_SITE);
return; return;
case TranslateMenu.ID_OVERFLOW_NOT_THIS_LANGUAGE: case TranslateMenu.ID_OVERFLOW_NOT_THIS_LANGUAGE:
mMenuHelper.show(TranslateMenu.MENU_SOURCE_LANGUAGE); initMenuHelper(TranslateMenu.MENU_SOURCE_LANGUAGE);
mLanguageMenuHelper.show(TranslateMenu.MENU_SOURCE_LANGUAGE);
return; return;
default: default:
assert false : "Unexpected overflow menu code"; assert false : "Unexpected overflow menu code";
......
...@@ -40,9 +40,8 @@ public final class TranslateMenu { ...@@ -40,9 +40,8 @@ public final class TranslateMenu {
// Menu item type config. // Menu item type config.
public static final int ITEM_DIVIDER = 0; public static final int ITEM_DIVIDER = 0;
public static final int ITEM_LANGUAGE = 1; public static final int ITEM_LANGUAGE = 1;
public static final int ITEM_TEXT_OPTION = 2; public static final int ITEM_CHECKBOX_OPTION = 2;
public static final int ITEM_CHECKBOX_OPTION = 3; public static final int MENU_ITEM_TYPE_COUNT = 3;
public static final int MENU_ITEM_TYPE_COUNT = 4;
// Menu Item ID config for MENU_OVERFLOW . // Menu Item ID config for MENU_OVERFLOW .
public static final int ID_UNDEFINED = 0; public static final int ID_UNDEFINED = 0;
...@@ -61,12 +60,13 @@ public final class TranslateMenu { ...@@ -61,12 +60,13 @@ public final class TranslateMenu {
// Load overflow menu item if it's empty. // Load overflow menu item if it's empty.
synchronized (OVERFLOW_MENU) { synchronized (OVERFLOW_MENU) {
if (OVERFLOW_MENU.isEmpty()) { if (OVERFLOW_MENU.isEmpty()) {
OVERFLOW_MENU.add(new MenuItem(ITEM_TEXT_OPTION, ID_OVERFLOW_MORE_LANGUAGE)); OVERFLOW_MENU.add(new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_MORE_LANGUAGE));
OVERFLOW_MENU.add(new MenuItem(ITEM_DIVIDER, ID_UNDEFINED)); OVERFLOW_MENU.add(new MenuItem(ITEM_DIVIDER, ID_UNDEFINED));
OVERFLOW_MENU.add(new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_ALWAYS_TRANSLATE)); OVERFLOW_MENU.add(new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_ALWAYS_TRANSLATE));
OVERFLOW_MENU.add(new MenuItem(ITEM_TEXT_OPTION, ID_OVERFLOW_NEVER_LANGUAGE)); OVERFLOW_MENU.add(new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_NEVER_LANGUAGE));
OVERFLOW_MENU.add(new MenuItem(ITEM_TEXT_OPTION, ID_OVERFLOW_NEVER_SITE)); OVERFLOW_MENU.add(new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_NEVER_SITE));
OVERFLOW_MENU.add(new MenuItem(ITEM_TEXT_OPTION, ID_OVERFLOW_NOT_THIS_LANGUAGE)); OVERFLOW_MENU.add(
new MenuItem(ITEM_CHECKBOX_OPTION, ID_OVERFLOW_NOT_THIS_LANGUAGE));
} }
} }
return OVERFLOW_MENU; return OVERFLOW_MENU;
......
...@@ -5,10 +5,13 @@ ...@@ -5,10 +5,13 @@
package org.chromium.chrome.browser.infobar.translate; package org.chromium.chrome.browser.infobar.translate;
import android.content.Context; import android.content.Context;
import android.graphics.Rect;
import android.os.Build;
import android.support.v4.content.ContextCompat; import android.support.v4.content.ContextCompat;
import android.view.ContextThemeWrapper; import android.view.ContextThemeWrapper;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.View; import android.view.View;
import android.view.View.MeasureSpec;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.AdapterView; import android.widget.AdapterView;
import android.widget.ArrayAdapter; import android.widget.ArrayAdapter;
...@@ -16,8 +19,10 @@ import android.widget.ListPopupWindow; ...@@ -16,8 +19,10 @@ import android.widget.ListPopupWindow;
import android.widget.PopupWindow; import android.widget.PopupWindow;
import android.widget.TextView; import android.widget.TextView;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.TranslateOptions; import org.chromium.chrome.browser.infobar.TranslateOptions;
import org.chromium.chrome.browser.widget.TintedImageView;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
...@@ -32,7 +37,6 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -32,7 +37,6 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
private ContextThemeWrapper mContextWrapper; private ContextThemeWrapper mContextWrapper;
private TranslateMenuAdapter mAdapter; private TranslateMenuAdapter mAdapter;
private View mAnchorView; private View mAnchorView;
private ListPopupWindow mPopup; private ListPopupWindow mPopup;
/** /**
...@@ -53,7 +57,7 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -53,7 +57,7 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
} }
/** /**
* Build transalte menu by menu type. * Build translate menu by menu type.
*/ */
private List<TranslateMenu.MenuItem> getMenuList(int menuType) { private List<TranslateMenu.MenuItem> getMenuList(int menuType) {
List<TranslateMenu.MenuItem> menuList = new ArrayList<TranslateMenu.MenuItem>(); List<TranslateMenu.MenuItem> menuList = new ArrayList<TranslateMenu.MenuItem>();
...@@ -86,15 +90,23 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -86,15 +90,23 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
// Need to explicitly set the background here. Relying on it being set in the style // Need to explicitly set the background here. Relying on it being set in the style
// caused an incorrectly drawn background. // caused an incorrectly drawn background.
// TODO(martiw): We might need a new menu background here.
mPopup.setBackgroundDrawable( mPopup.setBackgroundDrawable(
ContextCompat.getDrawable(mContextWrapper, R.drawable.edge_menu_bg)); ContextCompat.getDrawable(mContextWrapper, R.drawable.edge_menu_bg));
mPopup.setOnItemClickListener(this); mPopup.setOnItemClickListener(this);
int popupWidth = mContextWrapper.getResources().getDimensionPixelSize( // The menu must be shifted down by the height of the anchor view in order to be
R.dimen.infobar_translate_menu_width); // displayed over and above it.
// TODO (martiw) make the width dynamic to handle longer items. int anchorHeight = mAnchorView.getHeight();
mPopup.setWidth(popupWidth); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
// Setting a positive offset here shifts the menu down.
mPopup.setVerticalOffset(anchorHeight);
} else {
// The framework's PopupWindow positioning changed between N and M. Setting
// a negative offset here shifts the menu down rather than up.
mPopup.setVerticalOffset(-anchorHeight);
}
mAdapter = new TranslateMenuAdapter(menuType); mAdapter = new TranslateMenuAdapter(menuType);
mPopup.setAdapter(mAdapter); mPopup.setAdapter(mAdapter);
...@@ -102,12 +114,53 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -102,12 +114,53 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
mAdapter.refreshMenu(menuType); mAdapter.refreshMenu(menuType);
} }
if (menuType == TranslateMenu.MENU_OVERFLOW) {
// Use measured width when it is a overflow menu.
Rect bgPadding = new Rect();
mPopup.getBackground().getPadding(bgPadding);
mPopup.setWidth(measureMenuWidth(mAdapter) + bgPadding.left + bgPadding.right);
} else {
// Use fixed width otherwise.
int popupWidth = mContextWrapper.getResources().getDimensionPixelSize(
R.dimen.infobar_translate_menu_width);
mPopup.setWidth(popupWidth);
}
// When layout is RTL, set the horizontal offset to align the menu with the left side of the
// screen.
if (ApiCompatibilityUtils.isLayoutRtl(mAnchorView)) {
int[] tempLocation = new int[2];
mAnchorView.getLocationOnScreen(tempLocation);
mPopup.setHorizontalOffset(-tempLocation[0]);
}
if (!mPopup.isShowing()) { if (!mPopup.isShowing()) {
mPopup.show(); mPopup.show();
mPopup.getListView().setItemsCanFocus(true); mPopup.getListView().setItemsCanFocus(true);
} }
} }
private int measureMenuWidth(TranslateMenuAdapter adapter) {
final int widthMeasureSpec = MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED);
final int heightMeasureSpec = MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED);
final int count = adapter.getCount();
int width = 0;
int itemType = 0;
View itemView = null;
for (int i = 0; i < count; i++) {
final int positionType = adapter.getItemViewType(i);
if (positionType != itemType) {
itemType = positionType;
itemView = null;
}
itemView = adapter.getView(i, itemView, null);
itemView.measure(widthMeasureSpec, heightMeasureSpec);
width = Math.max(width, itemView.getMeasuredWidth());
}
return width;
}
@Override @Override
public void onItemClick(AdapterView<?> parent, View view, int position, long id) { public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
dismiss(); dismiss();
...@@ -151,9 +204,6 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -151,9 +204,6 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
* The provides the views of the menu items and dividers. * The provides the views of the menu items and dividers.
*/ */
private final class TranslateMenuAdapter extends ArrayAdapter<TranslateMenu.MenuItem> { private final class TranslateMenuAdapter extends ArrayAdapter<TranslateMenu.MenuItem> {
// TODO(martiw) create OVERFLOW_MENU_ITEM_WITH_CHECKBOX_CHECKED and
// OVERFLOW_MENU_ITEM_WITH_CHECKBOX_UNCHECKED for "Always Translate Language"
private final LayoutInflater mInflater; private final LayoutInflater mInflater;
private int mMenuType; private int mMenuType;
...@@ -195,7 +245,7 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -195,7 +245,7 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
assert false : "Unexpected Overflow Item Id"; assert false : "Unexpected Overflow Item Id";
} }
} else { } else {
// Get source and tagert language menu items text by language code. // Get source and target language menu items text by language code.
return mOptions.getRepresentationFromCode(item.mCode); return mOptions.getRepresentationFromCode(item.mCode);
} }
return ""; return "";
...@@ -216,6 +266,16 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -216,6 +266,16 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
return getItem(position).mId != TranslateMenu.ID_UNDEFINED; return getItem(position).mId != TranslateMenu.ID_UNDEFINED;
} }
private View getItemView(
View menuItemView, int position, ViewGroup parent, int resourceId) {
if (menuItemView == null) {
menuItemView = mInflater.inflate(resourceId, parent, false);
}
((TextView) menuItemView.findViewById(R.id.menu_item_text))
.setText(getItemViewText(getItem(position)));
return menuItemView;
}
@Override @Override
public View getView(int position, View convertView, ViewGroup parent) { public View getView(int position, View convertView, ViewGroup parent) {
View menuItemView = convertView; View menuItemView = convertView;
...@@ -227,15 +287,20 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -227,15 +287,20 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
} }
break; break;
case TranslateMenu.ITEM_CHECKBOX_OPTION: case TranslateMenu.ITEM_CHECKBOX_OPTION:
case TranslateMenu.ITEM_TEXT_OPTION: menuItemView = getItemView(
// TODO(martiw) create the layout for ITEM_TEXT_OPTION and ITEM_CHECKBOX_OPTION menuItemView, position, parent, R.layout.translate_menu_item_checked);
case TranslateMenu.ITEM_LANGUAGE: TintedImageView checkboxIcon =
if (menuItemView == null) { (TintedImageView) menuItemView.findViewById(R.id.menu_item_icon);
menuItemView = if (getItem(position).mId == TranslateMenu.ID_OVERFLOW_ALWAYS_TRANSLATE
mInflater.inflate(R.layout.translate_menu_item, parent, false); && mOptions.alwaysTranslateLanguageState()) {
checkboxIcon.setVisibility(View.VISIBLE);
} else {
checkboxIcon.setVisibility(View.INVISIBLE);
} }
((TextView) menuItemView.findViewById(R.id.menu_item_text)) break;
.setText(getItemViewText(getItem(position))); case TranslateMenu.ITEM_LANGUAGE:
menuItemView = getItemView(
menuItemView, position, parent, R.layout.translate_menu_item);
break; break;
default: default:
assert false : "Unexpected MenuItem type"; assert false : "Unexpected MenuItem type";
......
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