contextual menus upgrade

[bradleyrichter]

Brave users will expect familiar contextual menus that include the commonly used items found in most browsers, plus convienent access to new Brave features.

Please implement as shown in the spec images below:

[TheDigitalOrchard]

You've done quite a bit of work preparing these mockups. However, I personally disagree with some of the proposed menu labels. "Open Link in New Tab", for example, You're not opening a "link", you're opening the page that the link points to... a "page". Therefore, "Open in New Tab" is better wording, in my opinion.

I do agree with many of the other changes, particularly small stuff like "Proper Title Case".

[bradleyrichter]

@TheDigitalOrchard thanks for the feedback. I'm all for improving things, and correcting lingo. But in this case, is it better to follow than lead? Safari, FX and Chrome all use "Open Link in..."

Maybe "Open Linked Page in..." would be more correct but takes more space.

[weems]

I think New Tab is general enough, people will know that pages are links. Short and sweet. Open Link in New Tab works fine.

[bsclifton]

Checking this one out... 😃 might have some questions

[diracdeltas]

@bsclifton how's it going on this? i can take it but don't want to duplicate work.

[bsclifton]

@diracdeltas things are going great 😄 I hope to wrap this up tonite. I'll be on Slack

[bsclifton]

@bradleyrichter in the first spec image (NEW MENU: on link), what does "Copy" do (versus copy link)?

[bradleyrichter]

I think it copies the selected rich text to clipboard as if it were not linked. The existing menu has it as well.

On Apr 28, 2016, at 9:58 PM, Brian Clifton notifications@github.com wrote:

@bradleyrichter in the first spec image (NEW MENU: on link), what does "Copy" do (versus copy link)?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[bsclifton]

Most changes done...
https://github.com/bsclifton/browser-laptop/commit/6b7188f76131e3e2ec9203f5420ec90b855237d8

Could definitely use help with the remaining items. Trying to figure out:

• how to tell if context menu was popped up for a selection, not a url/picture/etc
• how to show the "Search with " menu item (including wiring it up). Currently, this has the label "Open Search..." and is disabled.
• how to display the bravery menu on page context menu

[bsclifton]

Just noticed this... (not related to my changes). The current HEAD for master has a problem with the context menus (inside the hamburger menu):

[bbondy]

in the first spec image (NEW MENU: on link), what does "Copy" do (versus copy link)?

Copy should copy the text, on a link it will select and copy it.
(Works same in Chrome)

Re the menu problem:
I don't see that, if you could do a pull request that fixes it independent of this, that'd be awesome though. 0.9.3 is on Wednesday.

[bsclifton]

Copy should copy the text, on a link it will select and copy it.
(Works same in Chrome)

I think my confusion was that it doesn't show unless you have a selection- but I think the spec was to show where the copy went, if it did show

Re the menu problem:
I don't see that, if you could do a pull request that fixes it independent of this, that'd be awesome though. 0.9.3 is on Wednesday.

Perfect- I will look at that next

[bsclifton]

I believe there are only a few things required to close this out

Help needed with the commented out items in #1522 cc: @diracdeltas

• Tab context menu, move tab to new window
• Search selection w/ default search engine
• Search highlighted link w/ default search engine

Also need help:

• adding the bravery menu (see

  browser-laptop/js/contextMenus.js

  Line 697 in f472858

  // TODO: bravery menu goes here

  )
• Tab context menu, adding "Mute other tabs"

[bbondy]

Tab context menu, move tab to new window

This one is going to be a bit harder for now until @bridiver gets to moving webviews across windows. We also want it for shared pinned tabs by the way.

Search selection w/ default search engine

windowActions.newFrame w/ appState's searchDetail.searchURL replacing {searchTerms} with the selected text. I think that might be window.getSelection().toString()

Search highlighted link w/ default search engine

I think the same but use the link text

adding the bravery menu

This should be similar I think to what the hamburger menu does w/ Bravery.

Tab context menu, adding "Mute other tabs"

See muteAll which you probably wan to pull up higher and then add another parameter for the frame to skip over in the forEach loop.

[diracdeltas]

adding the bravery menu
This should be similar I think to what the hamburger menu does w/ Bravery.

I thought the bravery context menu was bravery settings for the current page only, not globally. If so it might be best to leave it out until per-site Bravery settings is implemented.

[bbondy]

that should be hooked up by 5/20.

[bradleyrichter]

it should wait. Also, it will need to invoke a reload on the current page after the setting change is selected. (unless anyone knows of an architectual reason not to reload with the changed settings?)

[bbondy]

Not against it fully but just a thought that someone might have a form they filled out that they can't submit, so try turning off adblock, and then they get a reload losing their form data. Perhaps prompting for that case. But that's out of scope of this ticket, so no need to discuss it here.

[bsclifton]

Added "Mute other Tabs" w/ #1535

Sounds like we'll have to wait on the bravery menu; I'll check out the search items next

[bsclifton]

Added search to context menus w/ #1672

[bradleyrichter]

@bbondy @bsclifton

Not sure how it happened but I unintentionally omitted 2 menu items in the new menu for Tab-Context. Here is a new image:

I also changed the grouping on the page background context menu as requested by Brian:

[bbondy]

Look up selection is being tracked here:
#1627

[bbondy]

The items mentioned here: #1253 (comment) are fixed.

[bsclifton]

I submitted #2329 which fixes the casing issues with context menus. They should all be in a consistent Title Case. This PR also matches Chrome for the order of context menu items for situations that were not mocked up.

Here's what's left:

1. Context menu when right clicking tabs => should show "Move to New Window" (should be possible once add clone tab support #2593 is merged)
2. Context menu for page should show the Bravery menu for the active window

[bsclifton]

Image copy to clipboard closed w/ #1174 (comment)

Image search will be closed w/ #2607

[bsclifton]

Closing this issue in favor of tasks tracking the remaining work:
#3129
#3130