New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 663747 link

Starred by 1 user
Status: Fixed
Owner:
█ dgn@chromium.org
Not on Chrome anymore
Closed: Nov 2016
Cc:
mvanouwe...@chromium.org
█ bauerb@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature

Blocking:
issue 638580
issue 664085
issue 664156
issue 665005



Sign in to add a comment

Support configuring the items to show in the context menu

Project Member Reported by dgn@chromium.org, Nov 9 2016 
 Issue 651045  merged the 2 usages of context menu we had on the NTP: Most Visited tiles and Articles.

There are new UI elements now that should support the context menu and would need to show different items in it:

- Status card and action items: Remove (make dismissals accessible again).
- Downloads: no need for open in new/other tab, etc. We might want to keep incognito to open some assets in Chrome instead of intenting into other apps, but that's not supported for now anyway.
- Recent tabs: same, opening a new tab is irrelevant.

So the Context Menu needs to allow all that.

Comment 1 by peconn@chromium.org, Nov 9 2016

Cc: -peconn@chromium.org dgn@chromium.org
Owner: peconn@chromium.org
I'm happy to take this one.

Comment 2 by dgn@chromium.org, Nov 9 2016 

FYI: for Status cards and action items, they are DismissSiblings (see SuggestionSection#getDismissSiblingPosDelta), so some changes will be needed to make removal work for them. TBD in  issue 638580 . 

For the rest of the items, no weirdness to be expected I think.

Comment 3 by vitaliii@chromium.org, Nov 10 2016 

I created a separate bug (issue 664085) to decide what we want to show for download suggestions.

Comment 4 by dgn@chromium.org, Nov 10 2016

Blocking: 664085

Comment 5 by fi...@chromium.org, Nov 10 2016

Labels: -Pri-2 M-56 zine-triaged Pri-1

Comment 6 by vitaliii@chromium.org, Nov 11 2016 

There is some chance (see issue 664085), that we will to show different context menus for different suggestions in the same section (i.e. "Open in new tab" for asset and offline page downloads). Does you planned solution allow this?

Comment 7 by dgn@chromium.org, Nov 14 2016

Cc: -dgn@chromium.org
Labels: zine-16-11-14
Owner: dgn@chromium.org
Status: Started (was: Assigned)
Taking over, since my CL about the Status card changes quite a lot the ContextMenu helper, I'm addressing this issue at the same time. 

Re #c6: Should be possible, there is method to override to choose which items to show.

Comment 8 by vitaliii@chromium.org, Nov 15 2016

Blocking: 664156

Comment 9 by vitaliii@chromium.org, Nov 15 2016

Blocking: 665005
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b

commit be9cf09ebbbb0990e0a5c82b5a68c4523259b35b
Author: dgn <dgn@chromium.org>
Date: Wed Nov 16 20:36:18 2016

[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG= 638580 , 663747 

Review-Url: https://codereview.chromium.org/2459033002
Cr-Commit-Position: refs/heads/master@{#432613}

[delete] https://crrev.com/085701ef9a4ad12ba89368704c4f92f6d8dbccfe/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java
[add] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageScrollView.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/java_sources.gni
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageTest.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
[modify] https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java

Comment 11 by dgn@chromium.org, Nov 17 2016

Status: Fixed (was: Started)
►

Sign in to add a comment