Merge pull request #4361 from bsclifton/bookmarks-toolbar-click

Fixing bookmarks toolbar "hover after click" behavior on folders

js/lib/faviconUtil.js → app/common/lib/faviconUtil.js

@@ -2,16 +2,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const {isSourceAboutUrl} = require('./appUrlUtil')
-const UrlUtil = require('./urlutil')
+const { isSourceAboutUrl } = require('../../../js/lib/appUrlUtil')
+const UrlUtil = require('../../../js/lib/urlutil')
 
-module.exports = function getFavicon (frameProps, iconHref) {
+module.exports.iconSize = 16
+
+module.exports.getFavicon = (frameProps, iconHref) => {
   return new Promise((resolve, reject) => {
     if (!frameProps.get('location')) {
       resolve(null)
     }
 
-    const size = window.devicePixelRatio * 16
+    const size = window.devicePixelRatio * module.exports.iconSize
     const resolution = '#-moz-resolution=' + size + ',' + size
 
     // Default to favicon.ico if we can't find an icon.

js/components/bookmarksToolbar.js → app/renderer/components/bookmarksToolbar.js

@@ -4,46 +4,69 @@
 
 const React = require('react')
 const ReactDOM = require('react-dom')
-const ImmutableComponent = require('./immutableComponent')
-const contextMenus = require('../contextMenus')
-const windowActions = require('../actions/windowActions')
-const bookmarkActions = require('../actions/bookmarkActions')
-const appActions = require('../actions/appActions')
-const siteTags = require('../constants/siteTags')
-const siteUtil = require('../state/siteUtil')
-const dragTypes = require('../constants/dragTypes')
-const Button = require('../components/button')
-const cx = require('../lib/classSet')
-const dnd = require('../dnd')
-const dndData = require('../dndData')
-const calculateTextWidth = require('../lib/textCalculator').calculateTextWidth
-const windowStore = require('../stores/windowStore')
-const iconSize = 16
+const ImmutableComponent = require('../../../js/components/immutableComponent')
+const contextMenus = require('../../../js/contextMenus')
+const windowActions = require('../../../js/actions/windowActions')
+const bookmarkActions = require('../../../js/actions/bookmarkActions')
+const appActions = require('../../../js/actions/appActions')
+const siteTags = require('../../../js/constants/siteTags')
+const siteUtil = require('../../../js/state/siteUtil')
+const dragTypes = require('../../../js/constants/dragTypes')
+const Button = require('../../../js/components/button')
+const cx = require('../../../js/lib/classSet')
+const dnd = require('../../../js/dnd')
+const dndData = require('../../../js/dndData')
+const calculateTextWidth = require('../../../js/lib/textCalculator').calculateTextWidth
+const windowStore = require('../../../js/stores/windowStore')
+const iconSize = require('../../common/lib/faviconUtil').iconSize
 
 class BookmarkToolbarButton extends ImmutableComponent {
   constructor () {
     super()
     this.onClick = this.onClick.bind(this)
+    this.onMouseOver = this.onMouseOver.bind(this)
     this.onDragStart = this.onDragStart.bind(this)
     this.onDragEnd = this.onDragEnd.bind(this)
     this.onDragEnter = this.onDragEnter.bind(this)
     this.onDragLeave = this.onDragLeave.bind(this)
     this.onDragOver = this.onDragOver.bind(this)
     this.onContextMenu = this.onContextMenu.bind(this)
   }
-  get activeFrame () {
-    return windowStore.getFrame(this.props.activeFrameKey)
+
+  showBookmarkFolderMenu (e) {
+    e.target = ReactDOM.findDOMNode(this)
+
+    if (e && e.stopPropagation) {
+      e.stopPropagation()
+    }
+
+    const xPos = (e.target.getBoundingClientRect().left | 0) - 2
+    const yPos = (e.target.parentNode.getBoundingClientRect().bottom | 0) - 1
+
+    windowActions.onMouseOverBookmarkFolder(this.props.bookmark.get('folderId'), this.props.sites, this.props.bookmark, xPos, yPos)
   }
+
   onClick (e) {
     if (!this.props.clickBookmarkItem(this.props.bookmark, e) &&
         this.props.bookmark.get('tags').includes(siteTags.BOOKMARK_FOLDER)) {
       if (this.props.contextMenuDetail) {
         windowActions.setContextMenuDetail()
         return
       }
-      e.target = ReactDOM.findDOMNode(this)
-      this.props.showBookmarkFolderMenu(this.props.bookmark, e)
-      return
+      this.showBookmarkFolderMenu(e)
+    }
+  }
+
+  onMouseOver (e) {
+    // Behavior when a bookmarks toolbar folder has its list expanded
+    if (this.props.selectedFolderId) {
+      if (this.isFolder && this.props.selectedFolderId !== this.props.bookmark.get('folderId')) {
+        // Auto-expand the menu if user mouses over another folder
+        this.showBookmarkFolderMenu(e)
+      } else if (!this.isFolder && this.props.selectedFolderId !== -1) {
+        // Hide the currently expanded menu if user mouses over a non-folder
+        windowActions.onMouseOverBookmarkFolder(-1)
+      }
     }
   }
 
@@ -61,7 +84,7 @@ class BookmarkToolbarButton extends ImmutableComponent {
       e.target = ReactDOM.findDOMNode(this)
       if (dnd.isMiddle(e.target, e.clientX)) {
         e.target.getBoundingClientRect
-        this.props.showBookmarkFolderMenu(this.props.bookmark, e)
+        this.showBookmarkFolderMenu(e)
         windowActions.setIsBeingDraggedOverDetail(dragTypes.BOOKMARK, this.props.bookmark, {
           expanded: true
         })
@@ -168,6 +191,7 @@ class BookmarkToolbarButton extends ImmutableComponent {
       ref={(node) => { this.bookmarkNode = node }}
       title={hoverTitle}
       onClick={this.onClick}
+      onMouseOver={this.onMouseOver}
       onDragStart={this.onDragStart}
       onDragEnd={this.onDragEnd}
       onDragEnter={this.onDragEnter}
@@ -214,7 +238,6 @@ class BookmarksToolbar extends ImmutableComponent {
     this.onMoreBookmarksMenu = this.onMoreBookmarksMenu.bind(this)
     this.openContextMenu = this.openContextMenu.bind(this)
     this.clickBookmarkItem = this.clickBookmarkItem.bind(this)
-    this.showBookmarkFolderMenu = this.showBookmarkFolderMenu.bind(this)
   }
   get activeFrame () {
     return windowStore.getFrame(this.props.activeFrameKey)
@@ -270,9 +293,6 @@ class BookmarksToolbar extends ImmutableComponent {
   clickBookmarkItem (bookmark, e) {
     return bookmarkActions.clickBookmarkItem(this.bookmarks, bookmark, this.activeFrame, e)
   }
-  showBookmarkFolderMenu (bookmark, e) {
-    contextMenus.onShowBookmarkFolderMenu(this.bookmarks, bookmark, this.activeFrame, e)
-  }
   updateBookmarkData (props) {
     this.bookmarks = siteUtil.getBookmarks(props.sites)
 
@@ -387,14 +407,14 @@ class BookmarksToolbar extends ImmutableComponent {
               ref={(node) => this.bookmarkRefs.push(node)}
               key={i}
               contextMenuDetail={this.props.contextMenuDetail}
-              activeFrameKey={this.props.activeFrameKey}
               draggingOverData={this.props.draggingOverData}
               openContextMenu={this.openContextMenu}
               clickBookmarkItem={this.clickBookmarkItem}
-              showBookmarkFolderMenu={this.showBookmarkFolderMenu}
               bookmark={bookmark}
+              sites={this.bookmarks}
               showFavicon={this.props.showFavicon}
-              showOnlyFavicon={this.props.showOnlyFavicon} />)
+              showOnlyFavicon={this.props.showOnlyFavicon}
+              selectedFolderId={this.props.selectedFolderId} />)
       }
       {
         this.overflowBookmarkItems.size !== 0

docs/state.md

@@ -359,6 +359,9 @@ WindowStore
       isVisible: boolean, // true if Menubar control is visible
       selectedIndex: Array<number>, // indices of the selected menu item(s) (or null for none selected)
       lastFocusedSelector: string // selector for the last selected element (browser ui, not frame content)
+    },
+    bookmarksToolbar: {
+      selectedFolderId: number // folderId from the siteDetail of the currently expanded folder
     }
   },
   searchDetail: {

docs/windowActions.md

@@ -891,6 +891,25 @@ See `eventStore.js` for an example use-case
 
 
 
+### onMouseOverBookmarkFolder(folderId, bookmarks, bookmark, xPos, yPos) 
+
+Fired when the mouse clicks or hovers over a bookmark folder in the bookmarks toolbar
+
+**Parameters**
+
+**folderId**: `number`, from the siteDetail for the bookmark folder
+  If set to null, no menu is open. If set to -1, mouse is over a bookmark, not a folder
+
+**bookmarks**: `Object`, all of the users bookmarks
+
+**bookmark**: `Object`, the object representing the selected folder
+
+**xPos**: `number`, x position of the left corner of the context mennu
+
+**yPos**: `number`, y position of the top corner of the context menu
+
+
+
 
 * * *
 

js/actions/windowActions.js

@@ -1121,6 +1121,26 @@ const windowActions = {
       tabId,
       details
     })
+  },
+
+  /**
+   * Fired when the mouse clicks or hovers over a bookmark folder in the bookmarks toolbar
+   * @param {number} folderId - from the siteDetail for the bookmark folder
+   *   If set to null, no menu is open. If set to -1, mouse is over a bookmark, not a folder
+   * @param {Object} bookmarks - all of the users bookmarks
+   * @param {Object} bookmark - the object representing the selected folder
+   * @param {number} xPos - x position of the left corner of the context mennu
+   * @param {number} yPos - y position of the top corner of the context menu
+   */
+  onMouseOverBookmarkFolder: function (folderId, bookmarks, bookmark, xPos, yPos) {
+    dispatch({
+      actionType: WindowConstants.WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER,
+      folderId,
+      bookmarks,
+      bookmark,
+      xPos,
+      yPos
+    })
   }
 }
 

js/components/main.js

@@ -35,7 +35,7 @@ const AutofillCreditCardPanel = require('./autofillCreditCardPanel')
 const AddEditBookmark = require('./addEditBookmark')
 const LoginRequired = require('./loginRequired')
 const ReleaseNotes = require('./releaseNotes')
-const BookmarksToolbar = require('./bookmarksToolbar')
+const BookmarksToolbar = require('../../app/renderer/components/bookmarksToolbar')
 const ContextMenu = require('./contextMenu')
 const PopupWindow = require('./popupWindow')
 const NoScriptInfo = require('./noScriptInfo')
@@ -1033,7 +1033,8 @@ class Main extends ImmutableComponent {
             activeFrameKey={activeFrame && activeFrame.get('key') || undefined}
             windowWidth={window.innerWidth}
             contextMenuDetail={this.props.windowState.get('contextMenuDetail')}
-            sites={this.props.appState.get('sites')} />
+            sites={this.props.appState.get('sites')}
+            selectedFolderId={this.props.windowState.getIn(['ui', 'bookmarksToolbar', 'selectedFolderId'])} />
           : null
         }
         <div className={cx({

js/constants/windowConstants.js

@@ -77,7 +77,8 @@ const windowConstants = {
   WINDOW_RESET_MENU_STATE: _,
   WINDOW_SET_SUBMENU_SELECTED_INDEX: _,
   WINDOW_SET_LAST_FOCUSED_SELECTOR: _,
-  WINDOW_GOT_RESPONSE_DETAILS: _
+  WINDOW_GOT_RESPONSE_DETAILS: _,
+  WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER: _
 }
 
 module.exports = mapValuesByKeys(windowConstants)

js/contextMenus.js

@@ -1103,20 +1103,6 @@ function onLedgerContextMenu (location, hostPattern) {
   menu.destroy()
 }
 
-function onShowBookmarkFolderMenu (bookmarks, bookmark, activeFrame, e) {
-  if (e && e.stopPropagation) {
-    e.stopPropagation()
-  }
-  const menuTemplate = showBookmarkFolderInit(bookmarks, bookmark, activeFrame)
-  const rectLeft = e.target.getBoundingClientRect()
-  const rectBottom = e.target.parentNode.getBoundingClientRect()
-  windowActions.setContextMenuDetail(Immutable.fromJS({
-    left: (rectLeft.left | 0) - 2,
-    top: (rectBottom.bottom | 0) - 1,
-    template: menuTemplate
-  }))
-}
-
 /**
  * @param {Object} usernames - map of username to plaintext password
  * @param {string} origin - origin of the form
@@ -1256,10 +1242,10 @@ module.exports = {
   onTabPageContextMenu,
   onUrlBarContextMenu,
   onSiteDetailContextMenu,
-  onShowBookmarkFolderMenu,
   onShowUsernameMenu,
   onShowAutofillMenu,
   onMoreBookmarksMenu,
   onBackButtonHistoryMenu,
-  onForwardButtonHistoryMenu
+  onForwardButtonHistoryMenu,
+  showBookmarkFolderInit
 }

js/stores/windowStore.js

@@ -17,10 +17,10 @@ const UrlUtil = require('../lib/urlutil')
 const urlParse = require('url').parse
 const currentWindow = require('../../app/renderer/currentWindow')
 const {tabFromFrame} = require('../state/frameStateUtil')
-
 const {l10nErrorText} = require('../../app/common/lib/httpUtil')
 const {aboutUrls, getSourceAboutUrl, isIntermediateAboutPage, navigatableTypes} = require('../lib/appUrlUtil')
 const Serializer = require('../dispatcher/serializer')
+const { showBookmarkFolderInit } = require('../contextMenus')
 
 let windowState = Immutable.fromJS({
   activeFrameKey: null,
@@ -238,6 +238,35 @@ const newFrame = (frameOpts, openInForeground, insertionIndex) => {
 const windowStore = new WindowStore()
 const emitChanges = debounce(windowStore.emitChanges.bind(windowStore), 5)
 
+const showContextMenu = (action) => {
+  windowState = windowState.set('contextMenuDetail', action.detail)
+  // Drag and drop bookmarks code expects this to be set sync
+  windowStore.emitChanges()
+}
+
+const hideContextMenu = (action) => {
+  windowState = windowState.delete('contextMenuDetail')
+  // Drag and drop bookmarks code expects this to be set sync
+  windowStore.emitChanges()
+}
+
+const onMouseOverBookmarkFolder = (action) => {
+  windowState = windowState.setIn(['ui', 'bookmarksToolbar', 'selectedFolderId'], action.folderId)
+
+  if (action.folderId && action.folderId !== -1) {
+    const activeFrame = FrameStateUtil.getActiveFrame(windowState)
+    showContextMenu({
+      detail: Immutable.fromJS({
+        left: action.xPos,
+        top: action.yPos,
+        template: showBookmarkFolderInit(action.bookmarks, action.bookmark, activeFrame)
+      })
+    })
+  } else if (action.folderId === -1) {
+    hideContextMenu(action)
+  }
+}
+
 // Register callback to handle all updates
 const doAction = (action) => {
   // console.log(action.actionType, action, windowState.toJS())
@@ -605,12 +634,10 @@ const doAction = (action) => {
       return
     case WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL:
       if (!action.detail) {
-        windowState = windowState.delete('contextMenuDetail')
+        hideContextMenu(action)
       } else {
-        windowState = windowState.set('contextMenuDetail', action.detail)
+        showContextMenu(action)
       }
-      // Drag and drop bookmarks code expects this to be set sync
-      windowStore.emitChanges()
       return
     case WindowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL:
       if (!action.detail) {
@@ -797,8 +824,7 @@ const doAction = (action) => {
       break
     case WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE:
       if (getSetting(settings.AUTO_HIDE_MENU)) {
-        // Close existing context menus
-        doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL})
+        hideContextMenu(action)
         // Use value if provided; if not, toggle to opposite.
         const newVisibleStatus = typeof action.isVisible === 'boolean'
           ? action.isVisible
@@ -815,9 +841,11 @@ const doAction = (action) => {
       if (getSetting(settings.AUTO_HIDE_MENU)) {
         doAction({actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, isVisible: false})
       } else {
-        doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL})
+        hideContextMenu(action)
       }
       doAction({actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX})
+      doAction({actionType: WindowConstants.WINDOW_SET_BOOKMARKS_TOOLBAR_SELECTED_FOLDER_ID})
+      windowState = windowState.setIn(['ui', 'bookmarksToolbar', 'selectedFolderId'], null)
       break
     case WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX:
       windowState = windowState.setIn(['ui', 'menubar', 'selectedIndex'],
@@ -828,6 +856,9 @@ const doAction = (action) => {
     case WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR:
       windowState = windowState.setIn(['ui', 'menubar', 'lastFocusedSelector'], action.selector)
       break
+    case WindowConstants.WINDOW_ON_MOUSE_OVER_BOOKMARK_FOLDER:
+      onMouseOverBookmarkFolder(action)
+      break
 
     default:
   }