Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

See https://github.com/brave/browser-laptop/issues/1253 #1672

Merged
merged 2 commits into from
May 12, 2016
Merged

See https://github.com/brave/browser-laptop/issues/1253 #1672

merged 2 commits into from
May 12, 2016

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented May 11, 2016

Added search to context menu (for urls and for selection). Text could use updating, per the spec ("Search engine for 'term'").

@bsclifton
Copy link
Member Author

bsclifton commented May 11, 2016

Here's a screen cap
cc: @bradleyrichter

2016-05-11

@bbondy
Copy link
Member

bbondy commented May 11, 2016

@bradleyrichter I think Copy is used a lot more than search, I know the spec says differently but could we put those commands first, and then the search related ones in its own section after?

For reference:
#1253

Also I think we need to update the text as it looks to me currently like it's something related to OpenSearch which is an xml format. And the ... is reserved for when there's a secondary action coming up.

@bradleyrichter
Copy link
Contributor

@bbondy I'm guessing that the order used by chrome and safari places the unique actions on top for access priority and "mode recognition" over "Copy" because copy the most common keyboard shortcut?

Firefox has the order as you are suggesting but I prefer the chrome/safari approach in this case.

Regarding Open Search... - this is a place holder until we can connect the variables to the menu?

@bsclifton
Copy link
Member Author

bsclifton commented May 11, 2016

@bradleyrichter yes, that would be the next step 😄

I'd have to dig in more to read the search engine preference. That change would need to include a change to the menu.properties file with a token replaceable entry like "Search {{engine}} with '{{term}}'" so that other languages would display properly (since they could have a different ordering for the words)

@bradleyrichter
Copy link
Contributor

@bsclifton Can you start with "Search the web for [selected variable]"?

(if it's not too long)

@bsclifton
Copy link
Member Author

Will do 👍

Added search to context menu (for urls and for selection). Text could use updating, per the spec (Search <engine> for <term>).
@bsclifton
Copy link
Member Author

bsclifton commented May 12, 2016

@bradleyrichter fixed, per your comment. I went with Search for "{{selected variable}}"

Here's a pic
2016-05-12

@bbondy bbondy added this to the 0.10.0dev milestone May 12, 2016
@bbondy
Copy link
Member

bbondy commented May 12, 2016

Great addition, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants