diff --git a/js/lib/faviconUtil.js b/app/common/lib/faviconUtil.js similarity index 79% rename from js/lib/faviconUtil.js rename to app/common/lib/faviconUtil.js index 50006986997..5625860ab70 100644 --- a/js/lib/faviconUtil.js +++ b/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. diff --git a/js/components/bookmarksToolbar.js b/app/renderer/components/bookmarksToolbar.js similarity index 86% rename from js/components/bookmarksToolbar.js rename to app/renderer/components/bookmarksToolbar.js index fbac7f24109..e24bca73039 100644 --- a/js/components/bookmarksToolbar.js +++ b/app/renderer/components/bookmarksToolbar.js @@ -4,26 +4,27 @@ 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) @@ -31,9 +32,20 @@ class BookmarkToolbarButton extends ImmutableComponent { 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)) { @@ -41,9 +53,20 @@ class BookmarkToolbarButton extends ImmutableComponent { 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 diff --git a/docs/state.md b/docs/state.md index 360c930578a..4b60fff6537 100644 --- a/docs/state.md +++ b/docs/state.md @@ -358,6 +358,9 @@ WindowStore isVisible: boolean, // true if Menubar control is visible selectedIndex: Array, // 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: { diff --git a/docs/windowActions.md b/docs/windowActions.md index a7aa08167df..5f89c612503 100644 --- a/docs/windowActions.md +++ b/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 + + + * * * diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 550f5dd60f5..7e496f87d83 100644 --- a/js/actions/windowActions.js +++ b/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 + }) } } diff --git a/js/components/main.js b/js/components/main.js index 544de271a40..f7d1c30065a 100644 --- a/js/components/main.js +++ b/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') @@ -1027,7 +1027,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 }
{ 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: } diff --git a/test/components/bookmarksToolbarTest.js b/test/components/bookmarksToolbarTest.js new file mode 100644 index 00000000000..3ce1c71d4ba --- /dev/null +++ b/test/components/bookmarksToolbarTest.js @@ -0,0 +1,155 @@ +/* global describe, it, beforeEach */ + +const Brave = require('../lib/brave') +const { urlInput, bookmarksToolbar, navigator, navigatorNotBookmarked, saveButton } = require('../lib/selectors') +const settings = require('../../js/constants/settings') +const siteTags = require('../../js/constants/siteTags') +const assert = require('assert') + +function * setup (client) { + yield client + .waitUntilWindowLoaded() + .waitForUrl(Brave.newTabUrl) + .waitForBrowserWindow() + .waitForVisible('#window') + .waitForEnabled(urlInput) +} + +const findBookmarkFolder = (folderName, val) => { + const bookmarksMenu = val.value.menu.template.find((item) => { + return item.label === 'Bookmarks' + }) + if (bookmarksMenu && bookmarksMenu.submenu) { + const bookmarkFolder = bookmarksMenu.submenu.find((item) => { + return item.label === folderName + }) + if (bookmarkFolder) return true + } + return false +} + +describe('bookmarksToolbar', function () { + describe('configuration settings', function () { + Brave.beforeAll(this) + + it('shows the bookmarks toolbar if the setting is enabled', function * () { + yield this.app.client + .changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) + .waitForVisible(bookmarksToolbar, 1000) + }) + + it('hides the bookmarks toolbar if the setting is disabled', function * () { + yield this.app.client + .changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, false) + .waitForVisible(bookmarksToolbar, 1000, true) + }) + }) + + describe('when clicking a bookmark folder', function () { + Brave.beforeEach(this) + beforeEach(function * () { + yield setup(this.app.client) + }) + + it('shows a context menu', function * () { + yield this.app.client + .changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) + .waitForVisible(bookmarksToolbar, 1000) + .addSite({ + customTitle: 'demo1', + folderId: Math.random(), + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, siteTags.BOOKMARK_FOLDER) + .waitUntil(function () { + return this.getAppState().then((val) => { + return findBookmarkFolder('demo1', val) + }) + }) + .click('.bookmarkToolbarButton[title=demo1]') + .waitForVisible('.contextMenuItemText[data-l10n-id=emptyFolderItem]', 1000) + }) + + it('automatically opens context menu if you move mouse over a different folder', function * () { + this.page1Url = Brave.server.url('page1.html') + + const folderId1 = Math.random() + const folderId2 = Math.random() + + yield this.app.client + .changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) + .waitForVisible(bookmarksToolbar, 1000) + .addSite({ + customTitle: 'demo1', + folderId: folderId1, + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, siteTags.BOOKMARK_FOLDER) + .waitUntil(function () { + return this.getAppState().then((val) => { + return findBookmarkFolder('demo1', val) + }) + }) + .addSite({ + customTitle: 'demo2', + folderId: folderId2, + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, siteTags.BOOKMARK_FOLDER) + .waitUntil(function () { + return this.getAppState().then((val) => { + return findBookmarkFolder('demo2', val) + }) + }) + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowParentByUrl(this.page1Url) + .waitForVisible(navigator) + .moveToObject(navigator) + .waitForVisible(navigatorNotBookmarked) + .click(navigatorNotBookmarked) + .waitForVisible(saveButton) + .selectByValue('#bookmarkParentFolder select', folderId2) + .click(saveButton) + .click('.bookmarkToolbarButton[title=demo1]') + .moveToObject('.bookmarkToolbarButton[title=demo2]') + .getText('.contextMenuItemText').then((val) => { + assert(val === 'Page 1') + }) + }) + + it('hides context menu when mousing over regular bookmark', function * () { + this.page1Url = Brave.server.url('page1.html') + + yield this.app.client + .changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, true) + .waitForVisible(bookmarksToolbar, 1000) + .addSite({ + customTitle: 'demo1', + folderId: Math.random(), + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, siteTags.BOOKMARK_FOLDER) + .waitUntil(function () { + return this.getAppState().then((val) => { + return findBookmarkFolder('demo1', val) + }) + }) + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowParentByUrl(this.page1Url) + .waitForVisible(navigator) + .moveToObject(navigator) + .waitForVisible(navigatorNotBookmarked) + .click(navigatorNotBookmarked) + .waitForVisible(saveButton) + .setValue('#bookmarkName input', 'test1') + .click(saveButton) + .waitForVisible('.bookmarkToolbarButton[title^=test1]') + .click('.bookmarkToolbarButton[title=demo1]') + .waitForVisible('.contextMenuItemText[data-l10n-id=emptyFolderItem]', 1000) + .moveToObject('.bookmarkToolbarButton[title^=test1]') + .waitForVisible('.contextMenuItemText', 1000, true) + }) + }) +})