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

Context menu upgrades #1504

Merged
merged 1 commit into from
Apr 30, 2016
Merged

Context menu upgrades #1504

merged 1 commit into from
Apr 30, 2016

Conversation

bsclifton
Copy link
Member

Per #1253

  • menu on URL nearly matches spec (just missing "search with X")
  • menu for selection nearly matches spec (missing "look up X")
  • menu on page bg nearly matches spec (missing bravery menu)
  • menu for tab nearly matches spec (move to new window needs functionality)

I need help working through the remaining items (search, bravery menu, move tab to new window)

- menu on URL nearly matches spec (just missing "search with X")
- menu for selection nearly matches spec (missing "look up X")
- menu on page bg nearly matches spec (missing bravery menu)
- menu for tab nearly matches spec (move to new window needs functionality)
@bsclifton
Copy link
Member Author

bsclifton commented Apr 29, 2016

Some screen caps:

On link

2016-04-29 4

On selection

2016-04-29 5

On page BG

2016-04-29 6

On tab

2016-04-29 3

@aekeus
Copy link
Member

aekeus commented Apr 29, 2016

This looks good to me! Tested manually as well as code inspected.@bbondy or @diracdeltas would you like to confirm these changes as well?

@aekeus
Copy link
Member

aekeus commented Apr 29, 2016

Thanks @bsclifton for the commit!

@bsclifton
Copy link
Member Author

You're welcome @aekeus! 😄 If you'd like, I can back out the incomplete items (since they were just implemented by putting a disabled menu item in place). The accompanying code has "//TODO:" blocks in the parts I didn't know about implementing

Let me know and definitely curious what @bbondy and @diracdeltas think 😃

@bbondy
Copy link
Member

bbondy commented Apr 30, 2016

Thanks @bsclifton I'll merge this but ya please comment out the disabled items for now. Leaving it as is just commented for now is fine. Do that in a followup, thanks!

@bbondy bbondy merged commit 8fe48af into brave:master Apr 30, 2016
@luixxiul luixxiul added this to the 0.9.3dev milestone Oct 3, 2016
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.

4 participants