Merge pull request #187 from brave/fix/toggle-menu-bookmarks

Add toggling bookmarks to application menu
brave · Jan 19, 2016 · 1edaeca · 1edaeca
2 parents 12b8214 + 66e2855
commit 1edaeca
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 10 deletions.
diff --git a/app/index.js b/app/index.js
@@ -128,6 +128,10 @@ app.on('ready', function () {
       app.quit()
     })
 
+    ipcMain.on(messages.UPDATE_APP_MENU, (e, args) => {
+      Menu.init(args)
+    })
+
     ipcMain.on(messages.CONTEXT_MENU_OPENED, (e, nodeName) => {
       BrowserWindow.getFocusedWindow().webContents.send(messages.CONTEXT_MENU_OPENED, nodeName)
     })

diff --git a/app/menu.js b/app/menu.js
@@ -28,7 +28,28 @@ const sendToFocusedWindow = (focusedWindow, message) => {
   }
 }
 
-const init = () => {
+/**
+ * Sets up the menu.
+ * @param {Object} args - Arguments to initialize the menu with if any
+ * @param {boolean} state.bookmarked - Whether the current active page is
+ *   bookmarked
+ */
+const init = (args) => {
+  args = args || {}
+  // Create references to menu items that need to be updated dynamically
+  var bookmarkPageMenu = {
+    label: 'Bookmark this page',
+    type: 'checkbox',
+    accelerator: 'CmdOrCtrl+D',
+    checked: args.bookmarked || false,
+    click: function (item, focusedWindow) {
+      var msg = bookmarkPageMenu.checked
+        ? messages.SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK
+        : messages.SHORTCUT_ACTIVE_FRAME_BOOKMARK
+      sendToFocusedWindow(focusedWindow, [msg])
+    }
+  }
+
   var template = [
     {
       label: 'File',
@@ -357,13 +378,8 @@ const init = () => {
     }, {
       label: 'Bookmarks',
       submenu: [
+        bookmarkPageMenu,
         {
-          label: 'Add Bookmark',
-          accelerator: 'CmdOrCtrl+D',
-          click: function (item, focusedWindow) {
-            sendToFocusedWindow(focusedWindow, [messages.SHORTCUT_ACTIVE_FRAME_BOOKMARK])
-          }
-        }, {
           label: 'Add to Favorites Bar',
           enabled: false,
           accelerator: 'Shift+CmdOrCtrl+D'
@@ -422,7 +438,7 @@ const init = () => {
           checked: Filtering.isResourceEnabled(AdBlock.resourceName),
           click: function (item, focusedWindow) {
             AppActions.setResourceEnabled(AdBlock.resourceName, !Filtering.isResourceEnabled(AdBlock.resourceName))
-            init()
+            init({bookmarked: bookmarkPageMenu.checked})
           }
         }, {
           type: 'checkbox',
@@ -435,7 +451,7 @@ const init = () => {
           checked: Filtering.isResourceEnabled(TrackingProtection.resourceName),
           click: function (item, focusedWindow) {
             AppActions.setResourceEnabled(TrackingProtection.resourceName, !Filtering.isResourceEnabled(TrackingProtection.resourceName))
-            init()
+            init({bookmarked: bookmarkPageMenu.checked})
           }
         }, {
           type: 'checkbox',
@@ -448,7 +464,7 @@ const init = () => {
           checked: Filtering.isResourceEnabled(HttpsEverywhere.resourceName),
           click: function (item, focusedWindow) {
             AppActions.setResourceEnabled(HttpsEverywhere.resourceName, !Filtering.isResourceEnabled(HttpsEverywhere.resourceName))
-            init()
+            init({bookmarked: bookmarkPageMenu.checked})
           }
         }, {
           type: 'separator'

diff --git a/js/components/navigationBar.js b/js/components/navigationBar.js
@@ -56,6 +56,16 @@ class NavigationBar extends ImmutableComponent {
 
   componentDidMount () {
     ipc.on(messages.SHORTCUT_ACTIVE_FRAME_BOOKMARK, this.onAddBookmark.bind(this))
+    ipc.on(messages.SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK, this.onRemoveBookmark.bind(this))
+  }
+
+  componentDidUpdate (prevProps) {
+    // Update the app menu to reflect whether the current page is bookmarked
+    var prevBookmarked = prevProps.activeFrame &&
+      isSiteInList(prevProps.sites, prevProps.activeFrame.get('location'), SiteTags.BOOKMARK)
+    if (this.bookmarked !== prevBookmarked) {
+      ipc.send(messages.UPDATE_APP_MENU, {bookmarked: this.bookmarked})
+    }
   }
 
   render () {

diff --git a/js/constants/messages.js b/js/constants/messages.js
@@ -25,6 +25,7 @@ const messages = {
   SHORTCUT_ACTIVE_FRAME_BACK: _,
   SHORTCUT_ACTIVE_FRAME_FORWARD: _,
   SHORTCUT_ACTIVE_FRAME_BOOKMARK: _,
+  SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK: _,
   // Frame management shortcuts
   SHORTCUT_NEW_FRAME: _, /** @arg {string} opt_url to load if any */
   SHORTCUT_CLOSE_FRAME: _, /** @arg {number} opt_key of frame, defaults to active frame */
@@ -33,7 +34,9 @@ const messages = {
   SHORTCUT_FRAME_RELOAD: _, /** @arg {number} key of frame */
   SHORTCUT_NEXT_TAB: _,
   SHORTCUT_PREV_TAB: _,
+  // Misc application events
   QUIT_APPLICATION: _,
+  UPDATE_APP_MENU: _, /** @arg {Object} args menu args to update */
   // Updates
   UPDATE_REQUESTED: _,
   UPDATE_AVAILABLE: _,