1305574 - Implement Dismiss option for context menu in Activity Stream panel with AS block list

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect, P1)

Comment 1

[Bryan Munar (:bkmunar)]

Attachment: PR

Comment 3

[Stephan Leroux [:sleroux]]

Comment on attachment 8798607 [details] [review]
PR

Works pretty well - just left some comments to check out to fix up some things.

Comment 4

[Stephan Leroux [:sleroux]]

Comment on attachment 8798607 [details] [review]
PR

LGTM! Just needs a rebase and I can land away.

Comment 5

[Stephan Leroux [:sleroux]]

master https://github.com/mozilla-mobile/firefox-ios/commit/67e5eb552003ea9b3211453868d94e245d574749

Comment 6

[Maurya Talisetti]

"Dismiss" made me think that the context menu would get dismissed. But, it turns out that the specific highlight is removed from the Highlights list! So, I'm wondering if it would confused other users as well?

Comment 7

[Bryan Munar (:bkmunar)]

I thought that too as well as I was making it - I'll ask Bryan Bell for more direction

Comment 8

[Maurya Talisetti]

On the same topic, I just recollected, even the "Remove From History" option was confusing. And this was my thought process while tapping that option:

We are also implementing the same context menu for top sites (Bug 1243412). So, the same options would be used for it as well.

Does it remove both the highlight/top-site and the history entries? Or, does it remove just the history entries?
After tapping it looked like it removed the history entry. But, if I close and relaunch firefox, would the highlight/top-site remain?

Would a user want to delete from history, but make it remain in the highlights/top-sites? May be, only one of them, the Dismiss option and the Remove From History option, is necessary? It would also make the context menu look smaller and cleaner. After adding the new tab functionality, it would have 7 options:
Open in New Tab
Open in New Private Tab
Bookmark
Delete from History
Share
Dismiss (for highlights) / Remove (for topsites)

Comment 9

[Stephan Leroux [:sleroux]]

Hey Bryan (bell), do you have any thoughts on the discussion above? Not sure if Bryan M reached out on or not.

Comment 10

[Maurya Talisetti]

Sorry, I have more to add :) Though I'm currently only testing this 6 option context menu in the simulator (as opposed to in a device), the number of options and thereby the size of the context menu is being a bit bothersome. When I'm done looking at the context menu, I try to dismiss it by clicking on the area outside the context menu, usually on the bottom side. But, I end up mistakenly clicking the last option (Dismiss) and much to my disappointment, the highlight/top-site gets removed from the AS panel. So, essentially I'm hinting at wanting lesser number of options in the context menu.