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

only update the menu when bookmarks or closed tabs change #3813

Merged
merged 1 commit into from
Sep 8, 2016

Conversation

bridiver
Copy link
Collaborator

@bridiver bridiver commented Sep 8, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:
Add bookmark

  1. Go to brave.com
  2. Add bookmark
  3. Bookmark should appear in menu

Bookmark indicator

  1. Open a new tab
  2. Bookmark indicator should not be activated
  3. Bookmarks menu "Bookmark this page" should not be checked
  4. Go back to brave.com tab
  5. Bookmark indicator should be active
  6. Bookmarks menu "Bookmark this page" should be checked

Last closed Frame

  1. Close the brave.com tab
  2. History menu should show brave.com under recently closed

Language

  1. Change language
  2. Restart Brave
  3. Text should be translated by language setting
  4. Change back and restart Brave
  5. Text should be translated by language setting

State

  1. Open brave.com
  2. Close Brave
  3. Reopen Brave and cnn.com tab should be loaded

Fix #3782

@bridiver
Copy link
Collaborator Author

bridiver commented Sep 8, 2016

cc @bsclifton

@bbondy
Copy link
Member

bbondy commented Sep 8, 2016

++ thanks

@bbondy bbondy merged commit 12c204f into master Sep 8, 2016
@@ -363,13 +365,17 @@ const createHistorySubmenu = (CommonMenu) => {
return submenu
}

const createBookmarksSubmenu = (CommonMenu) => {
const isCurrentLocationBookmarked = () => {
Copy link
Member

@bsclifton bsclifton Sep 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice cleanup with this method 😄 The previous code was hacky (because it could be changed in appStore OR in the navigationBar control) and there were definitely ways to cause it to fail (ex: shows checked but its not bookmarked)

@bsclifton
Copy link
Member

Great job 😄 ++

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

Successfully merging this pull request may close these issues.

Cmd+T (New Tab) shortcut becomes disabled while navigation occurs
5 participants