jenkins-bot has submitted this change and it was merged. Change subject: Replace contextual action bar menu on text select ......................................................................
Replace contextual action bar menu on text select The contextual action bar menu (CAB) appears differently across devices. We think it's probably simplest to replace it with our menu every time, no matter the device, for a consistent experience. This patch does not address API 23 concerns[0] around the new floaty CAB nor the text selection bug[1] on API 15. [0] https://www.mediawiki.org/wiki/Notes_from_Google_I/O_15#Soon [1] https://phabricator.wikimedia.org/T115516 Bug: T107298 Bug: T109346 Change-Id: Ib0a80cfe80a1baa57a3dfb370f49b442ddefae81 --- M app/src/main/java/org/wikipedia/page/PageActivity.java M app/src/main/java/org/wikipedia/page/snippet/ShareHandler.java A app/src/main/res/menu/menu_text_select.xml M app/src/main/res/values-qq/strings.xml M app/src/main/res/values/strings.xml 5 files changed, 104 insertions(+), 101 deletions(-) Approvals: Mholloway: Looks good to me, but someone else must approve Dbrant: Looks good to me, approved jenkins-bot: Verified diff --git a/app/src/main/java/org/wikipedia/page/PageActivity.java b/app/src/main/java/org/wikipedia/page/PageActivity.java index a96db0a..02e1aec 100644 --- a/app/src/main/java/org/wikipedia/page/PageActivity.java +++ b/app/src/main/java/org/wikipedia/page/PageActivity.java @@ -882,6 +882,7 @@ public void onSupportActionModeStarted(ActionMode mode) { if (!isCabOpen() && !isAppInitiatedActionMode(mode) && getCurPageFragment() != null) { // Initiated by the system, likely in response to highlighting text in the WebView. + replaceTextSelectMenu(mode); getCurPageFragment().onActionModeShown(mode); } @@ -980,4 +981,10 @@ widgetIntent.putExtra(AppWidgetManager.EXTRA_APPWIDGET_IDS, ids); sendBroadcast(widgetIntent); } + + private void replaceTextSelectMenu(ActionMode mode) { + Menu menu = mode.getMenu(); + menu.clear(); + mode.getMenuInflater().inflate(R.menu.menu_text_select, menu); + } } diff --git a/app/src/main/java/org/wikipedia/page/snippet/ShareHandler.java b/app/src/main/java/org/wikipedia/page/snippet/ShareHandler.java index c683371..11b9ccf 100755 --- a/app/src/main/java/org/wikipedia/page/snippet/ShareHandler.java +++ b/app/src/main/java/org/wikipedia/page/snippet/ShareHandler.java @@ -4,12 +4,10 @@ import android.content.DialogInterface; import android.content.res.Resources; import android.graphics.Bitmap; -import android.graphics.Color; import android.support.annotation.ColorInt; import android.support.annotation.ColorRes; import android.support.annotation.IntegerRes; import android.support.annotation.NonNull; -import android.support.v4.view.MenuItemCompat; import android.support.v7.view.ActionMode; import android.util.Log; import android.view.Menu; @@ -34,11 +32,13 @@ import org.wikipedia.page.PageFragment; import org.wikipedia.tooltip.ToolTipUtil; import org.wikipedia.activity.ActivityUtil; -import org.wikipedia.util.ApiUtil; +import org.wikipedia.util.ClipboardUtil; +import org.wikipedia.util.FeedbackUtil; import org.wikipedia.util.ShareUtils; import org.json.JSONException; import org.json.JSONObject; +import org.wikipedia.util.log.L; import java.util.Map; @@ -49,9 +49,12 @@ */ public class ShareHandler { public static final String TAG = "ShareHandler"; + private static final String PAYLOAD_PURPOSE_KEY = "purpose"; + private static final String PAYLOAD_PURPOSE_SHARE = "share"; + private static final String PAYLOAD_PURPOSE_COPY = "copy"; + private static final String PAYLOAD_TEXT_KEY = "text"; @ColorRes private static final int SHARE_TOOL_TIP_COLOR = R.color.blue_liberal; - @ColorInt private static final int DEFAULT_ICON_COLOR = Color.WHITE; private final PageActivity activity; private final CommunicationBridge bridge; @@ -74,29 +77,41 @@ bridge.addListener("onGetTextSelection", new CommunicationBridge.JSEventListener() { @Override public void onMessage(String messageType, JSONObject messagePayload) { - String purpose = messagePayload.optString("purpose", ""); - String text = messagePayload.optString("text", ""); - if (purpose.equals("share")) { - if (funnel == null) { - createFunnel(); - } - shareSnippet(text); - funnel.logShareTap(text); + String purpose = messagePayload.optString(PAYLOAD_PURPOSE_KEY, ""); + String text = messagePayload.optString(PAYLOAD_TEXT_KEY, ""); + switch (purpose) { + case PAYLOAD_PURPOSE_SHARE: + onSharePayload(text); + break; + case PAYLOAD_PURPOSE_COPY: + onCopyPayload(text); + break; + default: + L.d("Unknown purpose=" + purpose); } } }); } - private void requestTextSelection() { - // send an event to the WebView that will make it return the - // selected text (or first paragraph) back to us... - try { - JSONObject payload = new JSONObject(); - payload.put("purpose", "share"); - bridge.sendMessage("getTextSelection", payload); - } catch (JSONException e) { - //nope + private void onSharePayload(String text) { + if (funnel == null) { + createFunnel(); } + shareSnippet(text); + funnel.logShareTap(text); + } + + private void onCopyPayload(String text) { + copyText(text); + showCopySnackbar(); + } + + private void copyText(String text) { + ClipboardUtil.setPlainText(activity, text, text); + } + + private void showCopySnackbar() { + FeedbackUtil.showMessage(activity, R.string.text_copied); } public void onDestroy() { @@ -165,20 +180,7 @@ webViewActionMode = mode; Menu menu = mode.getMenu(); - // Hide "select all" and "web search" menu items (but leave them enabled) - hideSystemMenuItems(menu, "select_all", "web_search"); - - // Find the "share" context menu item from the WebView's action mode. - MenuItem shareItem = getSystemMenuItemByName(menu, "share"); - - // if we were unable to find the Share button, then inject our own! - if (shareItem == null) { - shareItem = menu.add(Menu.NONE, Menu.NONE, Menu.NONE, - activity.getString(R.string.menu_page_share)); - shareItem.setIcon(R.drawable.ic_share_dark); - MenuItemCompat.setShowAsAction(shareItem, MenuItemCompat.SHOW_AS_ACTION_ALWAYS - | MenuItemCompat.SHOW_AS_ACTION_WITH_TEXT); - } + MenuItem shareItem = menu.findItem(R.id.menu_text_select_share); if (WikipediaApp.getInstance().isFeatureSelectTextAndShareTutorialEnabled() && WikipediaApp.getInstance().getOnboardingStateMachine().isShareTutorialEnabled()) { @@ -186,73 +188,13 @@ WikipediaApp.getInstance().getOnboardingStateMachine().setShareTutorial(); } - // provide our own listener for the Share button... - shareItem.setOnMenuItemClickListener(new MenuItem.OnMenuItemClickListener() { - @Override - public boolean onMenuItemClick(MenuItem item) { - requestTextSelection(); - - // leave context mode... - if (webViewActionMode != null) { - webViewActionMode.finish(); - webViewActionMode = null; - } - return true; - } - }); + // Provide our own listeners for the copy and share buttons. + shareItem.setOnMenuItemClickListener(new RequestTextSelectOnMenuItemClickListener(PAYLOAD_PURPOSE_SHARE)); + MenuItem copyItem = menu.findItem(R.id.menu_text_select_copy); + copyItem.setOnMenuItemClickListener(new RequestTextSelectOnMenuItemClickListener(PAYLOAD_PURPOSE_COPY)); createFunnel(); funnel.logHighlight(); - } - - /** - * Hide desired items from a system-controlled context menu. - * @param menu Menu on which to hide buttons. - * @param itemNames List of menu item resource names. - */ - private void hideSystemMenuItems(Menu menu, String... itemNames) { - for (String itemName : itemNames) { - MenuItem item = getSystemMenuItemByName(menu, itemName); - if (item != null) { - item.setVisible(false); - } - } - } - - /** - * Retrieve a specific menu item from a context menu that is controlled by the system - * by searching for the item by its actual resource name. - * @param menu Menu to search. - * @param nameSubstring Portion of the resource name to match. - * @return The requested menu item, or null if it wasn't found. - */ - private MenuItem getSystemMenuItemByName(Menu menu, String nameSubstring) { - MenuItem foundItem = null; - for (int i = 0; i < menu.size(); i++) { - MenuItem item = menu.getItem(i); - String resourceName = null; - try { - resourceName = activity.getResources().getResourceName(item.getItemId()); - } catch (Resources.NotFoundException e) { - // Looks like some devices don't provide access to these menu items through - // the context of the app, in which case, there's nothing we can do... - } - if (resourceName != null && resourceName.contains(nameSubstring)) { - foundItem = item; - } - // In APIs lower than 21, some of the action mode icons may not respect the - // current theme, so we need to manually tint those icons. - if (!ApiUtil.hasLollipop()) { - fixMenuItemTheme(item); - } - } - return foundItem; - } - - private void fixMenuItemTheme(MenuItem item) { - if (item != null && item.getIcon() != null) { - WikipediaApp.getInstance().setDrawableTint(item.getIcon(), DEFAULT_ICON_COLOR); - } } private void showShareOnboarding(MenuItem shareItem) { @@ -293,6 +235,37 @@ private Resources getResources() { return activity.getResources(); } + + private class RequestTextSelectOnMenuItemClickListener implements MenuItem.OnMenuItemClickListener { + @NonNull private final String purpose; + public RequestTextSelectOnMenuItemClickListener(@NonNull String purpose) { + this.purpose = purpose; + } + + @Override + public boolean onMenuItemClick(MenuItem item) { + requestTextSelection(purpose); + + // leave context mode... + if (webViewActionMode != null) { + webViewActionMode.finish(); + webViewActionMode = null; + } + return true; + } + + private void requestTextSelection(String purpose) { + // send an event to the WebView that will make it return the + // selected text (or first paragraph) back to us... + try { + JSONObject payload = new JSONObject(); + payload.put(PAYLOAD_PURPOSE_KEY, purpose); + bridge.sendMessage("getTextSelection", payload); + } catch (JSONException e) { + throw new RuntimeException(e); + } + } + } } /** diff --git a/app/src/main/res/menu/menu_text_select.xml b/app/src/main/res/menu/menu_text_select.xml new file mode 100644 index 0000000..2814456 --- /dev/null +++ b/app/src/main/res/menu/menu_text_select.xml @@ -0,0 +1,15 @@ +<?xml version="1.0" encoding="utf-8"?> +<menu xmlns:android="http://schemas.android.com/apk/res/android"; + xmlns:app="http://schemas.android.com/apk/res-auto";> + + <item android:id="@+id/menu_text_select_copy" + android:title="@string/menu_text_select_copy" + android:icon="?actionModeCopyDrawable" + app:showAsAction="ifRoom" /> + + <item android:id="@+id/menu_text_select_share" + android:title="@string/menu_text_select_share" + android:icon="?actionModeShareDrawable" + app:showAsAction="ifRoom" /> + +</menu> \ No newline at end of file diff --git a/app/src/main/res/values-qq/strings.xml b/app/src/main/res/values-qq/strings.xml index a6a2a43..289ebbb 100644 --- a/app/src/main/res/values-qq/strings.xml +++ b/app/src/main/res/values-qq/strings.xml @@ -66,7 +66,9 @@ <string name="menu_long_press_open_page">Menu item for opening a link in the current tab. {{Identical|Open}}</string> <string name="menu_long_press_open_in_new_tab">Menu item for opening a link in a new tab.</string> - <string name="menu_long_press_copy_page">Menu item for copying a link to the system clipbord for pasting later.</string> + <string name="menu_long_press_copy_page">Menu item for copying a link to the system clipboard for pasting later.</string> + <string name="menu_text_select_copy">Menu item caption for copying text to the system clipboard for pasting later.</string> + <string name="menu_text_select_share">Menu item caption for sharing text and images with other apps such as Email and Twitter.</string> <string name="nav_item_nearby">Entry in nav drawer for Nearby feature {{Identical|Nearby}}</string> <string name="nearby_empty_title">Title line for message when no nearby pages were found</string> @@ -356,7 +358,8 @@ <string name="error_unknown">Generic error message that tells the user that an unknown error occurred.</string> <string name="format_error_server_message">Message displayed to the user that shows the actual error response received from the server. The \"%s\" symbol is replaced with the API response from the server.</string> <string name="format_error_server_code">Message displayed to the user that shows the actual error code received from the server. The \"%s\" symbol is replaced with the API error code.</string> - <string name="address_copied">Message shown after copying a link to the clipboard</string> + <string name="address_copied">Message shown after copying a link to the system clipboard.</string> + <string name="text_copied">Message shown after copying text to the system clipboard.</string> <string name="button_continue_to_article">Button to continue to the full article from the current link preview.</string> <string name="preference_title_show_link_previews">Title of the preference for enabling or disabling link previews.</string> <string name="preference_summary_show_link_previews">Description of the preference for enabling or disabling link previews.</string> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 08df63f..8f07e74 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -50,6 +50,10 @@ <string name="menu_long_press_open_in_new_tab">Open in new tab</string> <string name="menu_long_press_copy_page">Copy link address</string> + <!-- Toolbar menu items --> + <string name="menu_text_select_copy">Copy</string> + <string name="menu_text_select_share">Share</string> + <!-- Nearby --> <string name="nav_item_nearby">Nearby</string> <string name="nearby_empty_title">No nearby pages here!</string> @@ -281,6 +285,7 @@ <string name="format_error_server_message">Message: \"%s\"</string> <string name="format_error_server_code">Code: \"%s\"</string> <string name="address_copied">Address copied to clipboard.</string> + <string name="text_copied">Text copied to clipboard.</string> <string name="button_continue_to_article">Continue to article</string> <string name="preference_title_show_link_previews">Show link previews</string> <string name="preference_summary_show_link_previews">Show a quick preview of articles when tapping on links.</string> -- To view, visit https://gerrit.wikimedia.org/r/246414 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib0a80cfe80a1baa57a3dfb370f49b442ddefae81 Gerrit-PatchSet: 2 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Niedzielski <[email protected]> Gerrit-Reviewer: BearND <[email protected]> Gerrit-Reviewer: Brion VIBBER <[email protected]> Gerrit-Reviewer: Dbrant <[email protected]> Gerrit-Reviewer: Mholloway <[email protected]> Gerrit-Reviewer: Niedzielski <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
