From 903ffd3bd11122829c0634d5065e78593666fc1b Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Mon, 26 Jun 2017 13:37:54 +0200 Subject: [PATCH] Removes all non-primitve types from BookmarksToolbar and BookmarkToolbarButton Resolves #9712 Auditors: @bsclifton Test Plan: --- app/common/lib/bookmarkUtil.js | 19 +- .../bookmarks/bookmarkToolbarButton.js | 71 +-- .../components/bookmarks/bookmarksToolbar.js | 31 +- app/renderer/reducers/contextMenuReducer.js | 438 +++++++++++++++++- js/actions/bookmarkActions.js | 12 +- js/actions/windowActions.js | 24 + js/constants/windowConstants.js | 5 +- js/contextMenus.js | 124 +---- js/dnd.js | 4 +- js/state/siteUtil.js | 4 +- test/unit/app/common/lib/bookmarkUtilTest.js | 16 +- .../reducers/contextMenuReducerTest.js | 2 +- 12 files changed, 537 insertions(+), 213 deletions(-) diff --git a/app/common/lib/bookmarkUtil.js b/app/common/lib/bookmarkUtil.js index f1beea204f0..4189e3f2a32 100644 --- a/app/common/lib/bookmarkUtil.js +++ b/app/common/lib/bookmarkUtil.js @@ -41,8 +41,8 @@ const isBookmarkNameValid = (detail, isFolder) => { const title = detail.get('title') || detail.get('customTitle') const location = detail.get('location') return isFolder - ? (title != null && title.trim().length > 0) - : (location != null && location.trim().length > 0) + ? (title != null && title !== 0) && title.trim().length > 0 + : location != null && location.trim().length > 0 } const showOnlyFavicon = () => { @@ -56,24 +56,19 @@ const showFavicon = () => { btbMode === bookmarksToolbarMode.FAVICONS_ONLY } -const getDNDBookmarkData = (state, bookmark) => { +const getDNDBookmarkData = (state, bookmarkKey) => { const data = (state.getIn(['dragData', 'dragOverData', 'draggingOverType']) === dragTypes.BOOKMARK && state.getIn(['dragData', 'dragOverData'], Immutable.Map())) || Immutable.Map() - if (data.get('draggingOverKey') == null) { - return Immutable.Map() - } - - // TODO (nejc) this is slow, replace with simple ID check - we need to add id into bookmark object - return (Immutable.is(data.get('draggingOverKey'), bookmark)) ? data : Immutable.Map() + return data.get('draggingOverKey') === bookmarkKey ? data : Immutable.Map() } const getToolbarBookmarks = (state) => { const sites = state.get('sites', Immutable.List()) const noParentItems = siteUtil.getBookmarks(sites) - .sort(siteUtil.siteSort) .filter((bookmark) => !bookmark.get('parentFolderId')) + .sort(siteUtil.siteSort) let widthAccountedFor = 0 const overflowButtonWidth = 25 const onlyFavicon = showOnlyFavicon() @@ -119,9 +114,9 @@ const getToolbarBookmarks = (state) => { } return { - visibleBookmarks: noParentItems.take(i), + visibleBookmarks: noParentItems.take(i).map((item, index) => index).toList(), // Show at most 100 items in the overflow menu - hiddenBookmarks: noParentItems.skip(i).take(100) + hiddenBookmarks: noParentItems.skip(i).take(100).map((item, index) => index).toList() } } diff --git a/app/renderer/components/bookmarks/bookmarkToolbarButton.js b/app/renderer/components/bookmarks/bookmarkToolbarButton.js index 994148c7091..ec525bf2081 100644 --- a/app/renderer/components/bookmarks/bookmarkToolbarButton.js +++ b/app/renderer/components/bookmarks/bookmarkToolbarButton.js @@ -15,23 +15,16 @@ const windowActions = require('../../../../js/actions/windowActions') const appActions = require('../../../../js/actions/appActions') const bookmarkActions = require('../../../../js/actions/bookmarkActions') -// Store -const windowStore = require('../../../../js/stores/windowStore') - // Constants const dragTypes = require('../../../../js/constants/dragTypes') const {iconSize} = require('../../../../js/constants/config') -const {bookmarksToolbarMode} = require('../../../common/constants/settingsEnums') -const settings = require('../../../../js/constants/settings') // Utils const siteUtil = require('../../../../js/state/siteUtil') const {getCurrentWindowId} = require('../../currentWindow') const dnd = require('../../../../js/dnd') const cx = require('../../../../js/lib/classSet') -const {getSetting} = require('../../../../js/settings') const frameStateUtil = require('../../../../js/state/frameStateUtil') -const contextMenus = require('../../../../js/contextMenus') const bookmarkUtil = require('../../../common/lib/bookmarkUtil') // Styles @@ -58,10 +51,6 @@ class BookmarkToolbarButton extends React.Component { this.bookmarkNode.addEventListener('auxclick', this.onAuxClick) } - get activeFrame () { - return windowStore.getFrame(this.props.activeFrameKey) - } - onAuxClick (e) { if (e.button === 1) { this.onClick(e) @@ -96,11 +85,15 @@ class BookmarkToolbarButton extends React.Component { } onDragStart (e) { - dnd.onDragStart(dragTypes.BOOKMARK, this.props.bookmark, e) + dnd.onDragStart(dragTypes.BOOKMARK, Immutable.fromJS({ + location: this.props.location, + title: this.props.title, + bookmarkKey: this.props.bookmarkKey + }), e) } - onDragEnd (e) { - dnd.onDragEnd(dragTypes.BOOKMARK, this.props.bookmark, e) + onDragEnd () { + dnd.onDragEnd() } onDragEnter (e) { @@ -110,7 +103,7 @@ class BookmarkToolbarButton extends React.Component { if (dnd.isMiddle(e.target, e.clientX)) { this.showBookmarkFolderMenu(e) appActions.draggedOver({ - draggingOverKey: this.props.bookmark, + draggingOverKey: this.props.bookmarkKey, draggingOverType: dragTypes.BOOKMARK, draggingOverWindowId: getCurrentWindowId(), expanded: true @@ -123,7 +116,7 @@ class BookmarkToolbarButton extends React.Component { // Bookmark specific DND code to expand hover when on a folder item if (this.props.isFolder) { appActions.draggedOver({ - draggingOverKey: this.props.bookmark, + draggingOverKey: this.props.bookmarkKey, draggingOverType: dragTypes.BOOKMARK, draggingOverWindowId: getCurrentWindowId(), expanded: false @@ -135,8 +128,11 @@ class BookmarkToolbarButton extends React.Component { dnd.onDragOver( dragTypes.BOOKMARK, this.bookmarkNode.getBoundingClientRect(), - this.props.bookmark, - this.props.draggingOverData, + this.props.bookmarkKey, + Immutable.fromJS({ + draggingOverLeftHalf: this.props.isDraggingOverLeft, + draggingOverRightHalf: this.props.isDraggingOverRight + }), e ) } @@ -146,30 +142,42 @@ class BookmarkToolbarButton extends React.Component { } openContextMenu (e) { - contextMenus.onSiteDetailContextMenu(this.props.bookmark, this.activeFrame, e) + if (e) { + e.stopPropagation() + } + windowActions.onSiteDetailMenu(this.props.bookmarkKey) } clickBookmarkItem (e) { - return bookmarkActions.clickBookmarkItem(this.props.bookmarks, this.props.bookmark, this.activeFrame, e) + return bookmarkActions.clickBookmarkItem(this.props.bookmarkKey, this.props.tabId, e) } showBookmarkFolderMenu (e) { + const rectLeft = e.target.getBoundingClientRect() + const rectBottom = e.target.parentNode.getBoundingClientRect() + const left = (rectLeft.left | 0) - 2 + const top = (rectBottom.bottom | 0) - 1 + + if (e && e.stopPropagation) { + e.stopPropagation() + } + + // TODO merge this two actions into one + windowActions.onShowBookmarkFolderMenu(this.props.bookmarkKey, left, top) windowActions.setBookmarksToolbarSelectedFolderId(this.props.folderId) - contextMenus.onShowBookmarkFolderMenu(this.props.bookmarks, this.props.bookmark, this.activeFrame, e) } mergeProps (state, ownProps) { const currentWindow = state.get('currentWindow') const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() - const btbMode = getSetting(settings.BOOKMARKS_TOOLBAR_MODE) - const bookmark = ownProps.bookmark - const draggingOverData = bookmarkUtil.getDNDBookmarkData(state, bookmark) + const bookmarkKey = ownProps.bookmarkKey + const bookmark = state.getIn(['sites', bookmarkKey], Immutable.Map()) + const draggingOverData = bookmarkUtil.getDNDBookmarkData(state, bookmarkKey) const props = {} // used in renderer - props.showFavicon = btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || - btbMode === bookmarksToolbarMode.FAVICONS_ONLY - props.showOnlyFavicon = btbMode === bookmarksToolbarMode.FAVICONS_ONLY + props.showFavicon = bookmarkUtil.showFavicon() + props.showOnlyFavicon = bookmarkUtil.showOnlyFavicon() props.favIcon = bookmark.get('favicon') props.title = bookmark.get('customTitle', bookmark.get('title')) props.location = bookmark.get('location') @@ -177,14 +185,13 @@ class BookmarkToolbarButton extends React.Component { props.isDraggingOverLeft = draggingOverData.get('draggingOverLeftHalf', false) props.isDraggingOverRight = draggingOverData.get('draggingOverRightHalf', false) props.isExpanded = draggingOverData.get('expanded', false) - props.isDragging = Immutable.is(dnd.getInterBraveDragData(), bookmark) + props.isDragging = state.getIn(['dragData', 'data', 'bookmarkKey']) === bookmarkKey // used in other function - props.bookmark = bookmark // TODO (nejc) only primitives - props.bookmarks = siteUtil.getBookmarks(state.get('sites')) // TODO (nejc) only primitives - props.contextMenuDetail = currentWindow.get('contextMenuDetail') // TODO (nejc) only primitives - props.draggingOverData = draggingOverData // TODO (nejc) only primitives + props.bookmarkKey = bookmarkKey props.activeFrameKey = activeFrame.get('key') + props.tabId = activeFrame.get('tabId') + props.contextMenuDetail = !!currentWindow.get('contextMenuDetail') props.selectedFolderId = currentWindow.getIn(['ui', 'bookmarksToolbar', 'selectedFolderId']) props.folderId = bookmark.get('folderId') diff --git a/app/renderer/components/bookmarks/bookmarksToolbar.js b/app/renderer/components/bookmarks/bookmarksToolbar.js index be126e24bf3..d6df813994f 100644 --- a/app/renderer/components/bookmarks/bookmarksToolbar.js +++ b/app/renderer/components/bookmarks/bookmarksToolbar.js @@ -14,20 +14,17 @@ const BookmarkToolbarButton = require('./bookmarkToolbarButton') // Actions const appActions = require('../../../../js/actions/appActions') +const windowActions = require('../../../../js/actions/windowActions') // State const windowState = require('../../../common/state/windowState') -// Store -const windowStore = require('../../../../js/stores/windowStore') - // Constants const siteTags = require('../../../../js/constants/siteTags') const dragTypes = require('../../../../js/constants/dragTypes') // Utils const {isFocused} = require('../../currentWindow') -const siteUtil = require('../../../../js/state/siteUtil') const contextMenus = require('../../../../js/contextMenus') const cx = require('../../../../js/lib/classSet') const dnd = require('../../../../js/dnd') @@ -49,10 +46,6 @@ class BookmarksToolbar extends React.Component { this.onMoreBookmarksMenu = this.onMoreBookmarksMenu.bind(this) } - get activeFrame () { - return windowStore.getFrame(this.props.activeFrameKey) - } - onDrop (e) { e.preventDefault() const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer) @@ -62,15 +55,13 @@ class BookmarksToolbar extends React.Component { if (!bookmarkRef) { return false } - return !siteUtil.isEquivalent(bookmarkRef.props.bookmark, bookmark) + return bookmarkRef.props.bookmarkKey !== bookmark.get('bookmarkKey') }), e.clientX) if (droppedOn.selectedRef) { const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOn.selectedRef), e.clientX) - const droppedOnSiteDetail = droppedOn.selectedRef.props.bookmark || droppedOn.selectedRef.props.bookmarkFolder - const isDestinationParent = droppedOnSiteDetail.get('tags').includes(siteTags.BOOKMARK_FOLDER) && droppedOn && droppedOn.isDroppedOn - const bookmarkSiteKey = siteUtil.getSiteKey(bookmark) - const droppedOnSiteKey = siteUtil.getSiteKey(droppedOnSiteDetail) - appActions.moveSite(bookmarkSiteKey, droppedOnSiteKey, isLeftSide, isDestinationParent) + const droppedOnKey = droppedOn.selectedRef.props.bookmarkKey + const isDestinationParent = droppedOn.selectedRef.props.isFolder && droppedOn && droppedOn.isDroppedOn + appActions.moveSite(bookmark.get('bookmarkKey'), droppedOnKey, isLeftSide, isDestinationParent) } return } @@ -128,7 +119,8 @@ class BookmarksToolbar extends React.Component { } onMoreBookmarksMenu (e) { - contextMenus.onMoreBookmarksMenu(this.activeFrame, this.props.bookmarks, this.props.hiddenBookmarks, e) + const rect = e.target.getBoundingClientRect() + windowActions.onMoreBookmarksMenu(this.props.hiddenBookmarks, rect.bottom) } onContextMenu (e) { @@ -153,14 +145,13 @@ class BookmarksToolbar extends React.Component { props.showFavicon = bookmarkUtil.showFavicon() props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused()) && !isWindows() - props.visibleBookmarks = bookmarks.visibleBookmarks // TODO (nejc) only primitives - props.hiddenBookmarks = bookmarks.hiddenBookmarks // TODO (nejc) only primitives + props.visibleBookmarks = bookmarks.visibleBookmarks + props.hiddenBookmarks = bookmarks.hiddenBookmarks // used in other functions props.activeFrameKey = activeFrame.get('key') props.title = activeFrame.get('title') props.location = activeFrame.get('location') - props.bookmarks = siteUtil.getBookmarks(state.get('sites', Immutable.List())) // TODO (nejc) only primitives return props } @@ -182,11 +173,11 @@ class BookmarksToolbar extends React.Component { onDragOver={this.onDragOver} onContextMenu={this.onContextMenu}> { - this.props.visibleBookmarks.map((bookmark, i) => + this.props.visibleBookmarks.map((bookmarkKey, i) => this.bookmarkRefs.push(node)} key={`toolbar-button-${i}`} - bookmark={bookmark} />) + bookmarkKey={bookmarkKey} />) } { this.props.hiddenBookmarks.size !== 0 diff --git a/app/renderer/reducers/contextMenuReducer.js b/app/renderer/reducers/contextMenuReducer.js index de39df0a7a2..ffd7be53af6 100644 --- a/app/renderer/reducers/contextMenuReducer.js +++ b/app/renderer/reducers/contextMenuReducer.js @@ -9,9 +9,14 @@ const remote = electron.remote const Menu = remote.Menu // Constants -const config = require('../../../js/constants/config.js') +const config = require('../../../js/constants/config') const windowConstants = require('../../../js/constants/windowConstants') const settings = require('../../../js/constants/settings') +const dragTypes = require('../../../js/constants/dragTypes') +const siteTags = require('../../../js/constants/siteTags') + +// Store +const appStoreRenderer = require('../../../js/stores/appStoreRenderer') // State const contextMenuState = require('../../common/state/contextMenuState') @@ -19,15 +24,22 @@ const contextMenuState = require('../../common/state/contextMenuState') // Actions const appActions = require('../../../js/actions/appActions') const windowActions = require('../../../js/actions/windowActions') +const bookmarkActions = require('../../../js/actions/bookmarkActions') // Utils const eventUtil = require('../../../js/lib/eventUtil') const CommonMenu = require('../../common/commonMenu') -const locale = require('../../../js/l10n.js') -const {makeImmutable, isMap} = require('../../common/state/immutableUtil') -const {getSetting} = require('../../../js/settings') +const locale = require('../../../js/l10n') +const bookmarkUtil = require('../../common/lib/bookmarkUtil') +const siteUtil = require('../../../js/state/siteUtil') +const dnd = require('../../../js/dnd') +const menuUtil = require('../../common/lib/menuUtil') +const urlUtil = require('../../../js/lib/urlutil') const frameStateUtil = require('../../../js/state/frameStateUtil') +const dndData = require('../../../js/dndData') +const {makeImmutable, isMap} = require('../../common/state/immutableUtil') const {getCurrentWindow} = require('../../renderer/currentWindow') +const {getSetting} = require('../../../js/settings') const validateAction = function (action) { action = makeImmutable(action) @@ -89,6 +101,414 @@ const onTabPageMenu = function (state, action) { tabPageMenu.popup(getCurrentWindow()) } +const openInNewTabMenuItem = (url, isPrivate, partitionNumber, openerTabId) => { + const active = getSetting(settings.SWITCH_TO_NEW_TABS) === true + if (Array.isArray(url) && Array.isArray(partitionNumber)) { + return { + label: locale.translation('openInNewTabs'), + click: () => { + for (let i = 0; i < url.length; ++i) { + appActions.createTabRequested({ + url: url[i], + isPrivate, + partitionNumber: partitionNumber[i], + openerTabId, + active + }) + } + } + } + } else { + return { + label: locale.translation('openInNewTab'), + click: () => { + appActions.createTabRequested({ + url, + isPrivate, + partitionNumber, + openerTabId, + active + }) + } + } + } +} + +const openInNewPrivateTabMenuItem = (url, openerTabId) => { + const active = getSetting(settings.SWITCH_TO_NEW_TABS) === true + if (Array.isArray(url)) { + return { + label: locale.translation('openInNewPrivateTabs'), + click: () => { + for (let i = 0; i < url.length; ++i) { + appActions.createTabRequested({ + url: url[i], + isPrivate: true, + openerTabId, + active + }) + } + } + } + } else { + return { + label: locale.translation('openInNewPrivateTab'), + click: () => { + appActions.createTabRequested({ + url, + isPrivate: true, + openerTabId, + active + }) + } + } + } +} + +const openInNewSessionTabMenuItem = (url, openerTabId) => { + const active = getSetting(settings.SWITCH_TO_NEW_TABS) === true + if (Array.isArray(url)) { + return { + label: locale.translation('openInNewSessionTabs'), + click: () => { + for (let i = 0; i < url.length; ++i) { + appActions.createTabRequested({ + url: url[i], + isPartitioned: true, + openerTabId, + active + }) + } + } + } + } else { + return { + label: locale.translation('openInNewSessionTab'), + click: () => { + appActions.createTabRequested({ + url, + isPartitioned: true, + openerTabId, + active + }) + } + } + } +} + +const openInNewWindowMenuItem = (location, isPrivate, partitionNumber) => { + return { + label: locale.translation('openInNewWindow'), + click: () => { + appActions.newWindow({ location, isPrivate, partitionNumber }) + } + } +} + +/** + * Obtains an add bookmark menu item + */ +const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isParent) => { + return { + label: locale.translation(label), + accelerator: 'CmdOrCtrl+D', + click: () => { + if (isParent) { + siteDetail = siteDetail.set('parentFolderId', closestDestinationDetail && closestDestinationDetail.get('folderId', closestDestinationDetail.get('parentFolderId'))) + } + if (siteDetail.constructor !== Immutable.Map) { + siteDetail = Immutable.fromJS(siteDetail) + } + siteDetail = siteDetail.set('location', urlUtil.getLocationIfPDF(siteDetail.get('location'))) + windowActions.setBookmarkDetail(siteDetail, siteDetail, closestDestinationDetail, true) + } + } +} + +const addFolderMenuItem = (closestDestinationDetail, isParent) => { + return { + label: locale.translation('addFolder'), + click: () => { + let emptyFolder = Immutable.fromJS({ tags: [siteTags.BOOKMARK_FOLDER] }) + if (isParent) { + emptyFolder = emptyFolder.set('parentFolderId', closestDestinationDetail && closestDestinationDetail.get('folderId', closestDestinationDetail.get('parentFolderId'))) + } + windowActions.setBookmarkDetail(emptyFolder, undefined, closestDestinationDetail, false) + } + } +} + +const copyAddressMenuItem = (label, location) => { + return { + label: locale.translation(label), + click: () => { + if (location) { + appActions.clipboardTextCopied(location) + } + } + } +} + +const openAllInNewTabsMenuItem = (folderDetail) => { + return { + label: locale.translation('openAllInTabs'), + click: () => { + bookmarkActions.openBookmarksInFolder(folderDetail) + } + } +} + +const siteDetailTemplateInit = (state, siteKey) => { + let isHistoryEntry = false + let multipleHistoryEntries = false + let multipleBookmarks = false + let isFolder = false + let isSystemFolder = false + let deleteLabel + let deleteTag + let siteDetail = appStoreRenderer.state.getIn(['sites', siteKey], Immutable.Map()) + const activeFrame = frameStateUtil.getActiveFrame(state) || Immutable.Map() + + // TODO(bsclifton): pull this out to a method + if (siteUtil.isBookmark(siteDetail) && activeFrame) { + deleteLabel = 'deleteBookmark' + deleteTag = siteTags.BOOKMARK + } else if (siteUtil.isFolder(siteDetail)) { + isFolder = true + isSystemFolder = siteDetail.get('folderId') === 0 || + siteDetail.get('folderId') === -1 + deleteLabel = 'deleteFolder' + deleteTag = siteTags.BOOKMARK_FOLDER + } else if (siteUtil.isHistoryEntry(siteDetail)) { + isHistoryEntry = true + deleteLabel = 'deleteHistoryEntry' + } else if (Immutable.List.isList(siteDetail)) { + // Multiple bookmarks OR history entries selected + multipleHistoryEntries = true + multipleBookmarks = true + siteDetail.forEach((site) => { + if (!siteUtil.isBookmark(site)) multipleBookmarks = false + if (!siteUtil.isHistoryEntry(site)) multipleHistoryEntries = false + }) + if (multipleBookmarks) { + deleteLabel = 'deleteBookmarks' + deleteTag = siteTags.BOOKMARK + } else if (multipleHistoryEntries) { + deleteLabel = 'deleteHistoryEntries' + } + } else { + deleteLabel = '' + } + + const template = [] + + if (!isFolder) { + if (!Immutable.List.isList(siteDetail)) { + const location = siteDetail.get('location') + + template.push( + openInNewTabMenuItem(location, undefined, siteDetail.get('partitionNumber')), + openInNewPrivateTabMenuItem(location), + openInNewWindowMenuItem(location, undefined, siteDetail.get('partitionNumber')), + openInNewSessionTabMenuItem(location), + copyAddressMenuItem('copyLinkAddress', location), + CommonMenu.separatorMenuItem + ) + } else { + let locations = [] + let partitionNumbers = [] + siteDetail.forEach((site) => { + locations.push(site.get('location')) + partitionNumbers.push(site.get('partitionNumber')) + }) + + template.push( + openInNewTabMenuItem(locations, undefined, partitionNumbers), + openInNewPrivateTabMenuItem(locations), + openInNewSessionTabMenuItem(locations), + CommonMenu.separatorMenuItem + ) + } + } else { + template.push(openAllInNewTabsMenuItem(siteDetail), CommonMenu.separatorMenuItem) + } + + if (!isSystemFolder) { + // Picking this menu item pops up the AddEditBookmark modal + // - History can be deleted but not edited + // - Multiple bookmarks cannot be edited at once + // - "Bookmarks Toolbar" and "Other Bookmarks" folders cannot be deleted + if (!isHistoryEntry && !multipleHistoryEntries && !multipleBookmarks) { + template.push( + { + label: locale.translation(isFolder ? 'editFolder' : 'editBookmark'), + click: () => windowActions.setBookmarkDetail(siteDetail, siteDetail, null, true) + }, + CommonMenu.separatorMenuItem) + } + + template.push( + { + label: locale.translation(deleteLabel), + click: () => { + if (Immutable.List.isList(siteDetail)) { + siteDetail.forEach((site) => appActions.removeSite(site, deleteTag)) + } else { + appActions.removeSite(siteDetail, deleteTag) + } + } + }) + } + + if (!isHistoryEntry && !multipleHistoryEntries) { + if (template[template.length - 1] !== CommonMenu.separatorMenuItem) { + template.push(CommonMenu.separatorMenuItem) + } + template.push( + addBookmarkMenuItem('addBookmark', siteUtil.getDetailFromFrame(activeFrame, siteTags.BOOKMARK), siteDetail, true), + addFolderMenuItem(siteDetail, true)) + } + + return menuUtil.sanitizeTemplateItems(template) +} + +const onSiteDetailMenu = (state, siteKey) => { + state = validateState(state) + + const template = siteDetailTemplateInit(state, siteKey) + const menu = Menu.buildFromTemplate(template) + menu.popup(getCurrentWindow()) + + return state +} + +const showBookmarkFolderInit = (state, parentBookmarkFolderKey) => { + const appState = appStoreRenderer.state + const allBookmarkItems = siteUtil.getBookmarks(appState.get('sites', Immutable.List())) + const parentBookmarkFolder = appState.getIn(['sites', parentBookmarkFolderKey]) + const items = siteUtil.filterSitesRelativeTo(allBookmarkItems, parentBookmarkFolder) + if (items.size === 0) { + return [{ + l10nLabelId: 'emptyFolderItem', + enabled: false, + dragOver: function (e) { + e.preventDefault() + e.dataTransfer.dropEffect = 'move' + }, + drop (e) { + e.preventDefault() + const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer) + if (bookmark) { + const bookmarkSiteKey = siteUtil.getSiteKey(bookmark) + appActions.moveSite(bookmarkSiteKey, parentBookmarkFolderKey, false, true) + } + } + }] + } + return bookmarkItemsInit(state, items) +} + +const bookmarkItemsInit = (state, items) => { + const activeFrame = frameStateUtil.getActiveFrame(state) || Immutable.Map() + const showFavicon = bookmarkUtil.showFavicon() + const template = [] + for (let item of items) { + const site = item[1] + const siteKey = item[0] + const isFolder = siteUtil.isFolder(site) + let faIcon + if (showFavicon && !site.get('favicon')) { + faIcon = isFolder ? 'fa-folder-o' : 'fa-file-o' + } + const templateItem = { + bookmark: site, + draggable: true, + label: site.get('customTitle', site.get('title', site.get('location'))), + icon: showFavicon ? site.get('favicon') : undefined, + faIcon, + contextMenu: function () { + windowActions.onSiteDetailMenu(siteKey) + }, + dragEnd: function () { + dnd.onDragEnd() + }, + dragStart: function (e) { + dnd.onDragStart(dragTypes.BOOKMARK, Immutable.fromJS({ + location: site.get('location'), + title: site.get('customTitle', site.get('title')), + bookmarkKey: siteKey + }), e) + }, + dragOver: function (e) { + e.preventDefault() + e.dataTransfer.dropEffect = 'move' + }, + drop: function (e) { + e.preventDefault() + const bookmarkItem = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer) + if (bookmarkItem) { + appActions.moveSite(bookmarkItem.get('bookmarkKey'), siteKey, dndData.shouldPrependVerticalItem(e.target, e.clientY)) + } + }, + click: function (e) { + bookmarkActions.clickBookmarkItem(siteKey, activeFrame.get('tabId'), e) + } + } + if (isFolder) { + templateItem.submenu = showBookmarkFolderInit(state, siteKey) + } + template.push(templateItem) + } + + return menuUtil.sanitizeTemplateItems(template) +} + +const onMoreBookmarksMenu = (state, action) => { + action = validateAction(action) + state = validateState(state) + + const sites = appStoreRenderer.state.get('sites') + let newSites = {} + + for (let siteKey of action.get('bookmarks')) { + newSites[siteKey] = sites.get(siteKey) + } + + const menuTemplate = bookmarkItemsInit(state, Immutable.fromJS(newSites)) + + menuTemplate.push({ + l10nLabelId: 'moreBookmarks', + click: function () { + appActions.createTabRequested({ + url: 'about:bookmarks' + }) + windowActions.setContextMenuDetail() + } + }) + + state = contextMenuState.setContextMenu(state, makeImmutable({ + right: 0, + top: action.get('top'), + template: menuTemplate + })) + + return state +} + +const onShowBookmarkFolderMenu = (state, action) => { + action = validateAction(action) + state = validateState(state) + + const menuTemplate = showBookmarkFolderInit(state, action.get('bookmarkKey')) + state = contextMenuState.setContextMenu(state, makeImmutable({ + left: action.get('left'), + top: action.get('top'), + template: menuTemplate + })) + + return state +} + const onLongBackHistory = (state, action) => { action = validateAction(action) state = validateState(state) @@ -206,8 +626,16 @@ const contextMenuReducer = (windowState, action) => { case windowConstants.WINDOW_ON_TAB_PAGE_CONTEXT_MENU: onTabPageMenu(windowState, action) break + case windowConstants.WINDOW_ON_MORE_BOOKMARKS_MENU: + windowState = onMoreBookmarksMenu(windowState, action) + break + case windowConstants.WINDOW_ON_SHOW_BOOKMARK_FOLDER_MENU: + windowState = onShowBookmarkFolderMenu(windowState, action) + break + case windowConstants.WINDOW_ON_SITE_DETAIL_MENU: + windowState = onSiteDetailMenu(windowState, action.bookmarkKey) + break } - return windowState } diff --git a/js/actions/bookmarkActions.js b/js/actions/bookmarkActions.js index 30bb7507c90..51152a2f38b 100644 --- a/js/actions/bookmarkActions.js +++ b/js/actions/bookmarkActions.js @@ -4,15 +4,18 @@ 'use strict' +const Immutable = require('immutable') const siteUtil = require('../state/siteUtil') const appActions = require('./appActions') const windowActions = require('./windowActions') const eventUtil = require('../lib/eventUtil') +const appStoreRenderer = require('../stores/appStoreRenderer') const {SWITCH_TO_NEW_TABS} = require('../constants/settings') const getSetting = require('../settings').getSetting const bookmarkActions = { - openBookmarksInFolder: function (allBookmarkItems, folderDetail) { + openBookmarksInFolder: function (folderDetail) { + const allBookmarkItems = siteUtil.getBookmarks(appStoreRenderer.state.get('sites')) // We have a middle clicked folder const bookmarks = allBookmarkItems .filter((bookmark) => (bookmark.get('parentFolderId') || 0) === (folderDetail.get('folderId') || 0) && siteUtil.isBookmark(bookmark)) @@ -39,7 +42,8 @@ const bookmarkActions = { * Performs an action based on the passed in event to the bookmark item * @return true if an action was performed */ - clickBookmarkItem: function (allBookmarkItems, bookmarkItem, activeFrame, e) { + clickBookmarkItem: function (bookmarkKey, tabId, e) { + const bookmarkItem = appStoreRenderer.state.getIn(['sites', bookmarkKey], Immutable.Map()) const isFolder = siteUtil.isFolder(bookmarkItem) if (!isFolder) { if (eventUtil.isForSecondaryAction(e)) { @@ -49,12 +53,12 @@ const bookmarkActions = { active: !!e.shiftKey || getSetting(SWITCH_TO_NEW_TABS) }) } else { - appActions.loadURLRequested(activeFrame.get('tabId'), bookmarkItem.get('location')) + appActions.loadURLRequested(tabId, bookmarkItem.get('location')) } windowActions.setContextMenuDetail() return true } else if (eventUtil.isForSecondaryAction(e)) { - this.openBookmarksInFolder(allBookmarkItems, bookmarkItem) + this.openBookmarksInFolder(bookmarkItem) windowActions.setContextMenuDetail() return true } diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 7802c6fbbd8..e74da9669d5 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1164,6 +1164,30 @@ const windowActions = { isFocused, shouldRender }) + }, + + onMoreBookmarksMenu: function (bookmarks, top) { + dispatch({ + actionType: windowConstants.WINDOW_ON_MORE_BOOKMARKS_MENU, + bookmarks, + top + }) + }, + + onShowBookmarkFolderMenu: function (bookmarkKey, left, top) { + dispatch({ + actionType: windowConstants.WINDOW_ON_SHOW_BOOKMARK_FOLDER_MENU, + bookmarkKey, + left, + top + }) + }, + + onSiteDetailMenu: function (bookmarkKey) { + dispatch({ + actionType: windowConstants.WINDOW_ON_SITE_DETAIL_MENU, + bookmarkKey + }) } } diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 85c77e7c427..58a191943a5 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -104,7 +104,10 @@ const windowConstants = { WINDOW_ON_CERT_ERROR: _, WINDOW_ON_TAB_PAGE_CONTEXT_MENU: _, WINDOW_ON_FRAME_BOOKMARK: _, - WINDOW_ON_STOP: _ + WINDOW_ON_STOP: _, + WINDOW_ON_MORE_BOOKMARKS_MENU: _, + WINDOW_ON_SHOW_BOOKMARK_FOLDER_MENU: _, + WINDOW_ON_SITE_DETAIL_MENU: _ } module.exports = mapValuesByKeys(windowConstants) diff --git a/js/contextMenus.js b/js/contextMenus.js index 6c282607932..19566685536 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -15,14 +15,11 @@ const bookmarkActions = require('./actions/bookmarkActions') const appActions = require('./actions/appActions') const siteTags = require('./constants/siteTags') const electronDownloadItemActions = require('../app/common/constants/electronDownloadItemActions') -const dragTypes = require('./constants/dragTypes') const siteUtil = require('./state/siteUtil') const downloadUtil = require('./state/downloadUtil') const menuUtil = require('../app/common/lib/menuUtil') const urlUtil = require('./lib/urlutil') const CommonMenu = require('../app/common/commonMenu') -const dnd = require('./dnd') -const dndData = require('./dndData') const appStoreRenderer = require('./stores/appStoreRenderer') const ipc = require('electron').ipcRenderer const locale = require('../js/l10n') @@ -34,10 +31,8 @@ const {isIntermediateAboutPage, isUrl, aboutUrls} = require('./lib/appUrlUtil') const {getBase64FromImageUrl} = require('./lib/imageUtil') const urlParse = require('../app/common/urlParse') const {getCurrentWindow} = require('../app/renderer/currentWindow') -const {bookmarksToolbarMode} = require('../app/common/constants/settingsEnums') const extensionState = require('../app/common/state/extensionState') const extensionActions = require('../app/common/actions/extensionActions') -const appStore = require('./stores/appStoreRenderer') const {makeImmutable} = require('../app/common/state/immutableUtil') const isDarwin = process.platform === 'darwin' @@ -325,95 +320,6 @@ function siteDetailTemplateInit (siteDetail, activeFrame) { return menuUtil.sanitizeTemplateItems(template) } -function showBookmarkFolderInit (allBookmarkItems, parentBookmarkFolder, activeFrame) { - const items = siteUtil.filterSitesRelativeTo(allBookmarkItems, parentBookmarkFolder) - if (items.size === 0) { - return [{ - l10nLabelId: 'emptyFolderItem', - enabled: false, - dragOver: function (e) { - e.preventDefault() - e.dataTransfer.dropEffect = 'move' - }, - drop (e) { - e.preventDefault() - const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer) - if (bookmark) { - const bookmarkSiteKey = siteUtil.getSiteKey(bookmark) - const parentBookmarkFolderKey = siteUtil.getSiteKey(parentBookmarkFolder) - appActions.moveSite(bookmarkSiteKey, parentBookmarkFolderKey, false, true) - } - } - }] - } - return bookmarkItemsInit(allBookmarkItems, items, activeFrame) -} - -function bookmarkItemsInit (allBookmarkItems, items, activeFrame) { - const btbMode = getSetting(settings.BOOKMARKS_TOOLBAR_MODE) - const showFavicon = (btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || btbMode === bookmarksToolbarMode.FAVICONS_ONLY) - const itemsList = items.toList() - const template = itemsList.map((site) => { - const isFolder = siteUtil.isFolder(site) - let faIcon - if (showFavicon && !site.get('favicon')) { - faIcon = isFolder ? 'fa-folder-o' : 'fa-file-o' - } - const templateItem = { - bookmark: site, - draggable: true, - label: site.get('customTitle') || site.get('title') || site.get('location'), - icon: showFavicon ? site.get('favicon') : undefined, - faIcon, - contextMenu: function (e) { - onSiteDetailContextMenu(site, activeFrame, e) - }, - dragEnd: function (e) { - dnd.onDragEnd(dragTypes.BOOKMARK, site, e) - }, - dragStart: function (e) { - dnd.onDragStart(dragTypes.BOOKMARK, site, e) - }, - dragOver: function (e) { - e.preventDefault() - e.dataTransfer.dropEffect = 'move' - }, - drop: function (e) { - e.preventDefault() - const bookmarkItem = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer) - if (bookmarkItem) { - const bookmarkItemSiteKey = siteUtil.getSiteKey(bookmarkItem) - const siteKey = siteUtil.getSiteKey(site) - - appActions.moveSite(bookmarkItemSiteKey, siteKey, dndData.shouldPrependVerticalItem(e.target, e.clientY)) - } - }, - click: function (e) { - bookmarkActions.clickBookmarkItem(allBookmarkItems, site, activeFrame, e) - } - } - if (isFolder) { - templateItem.submenu = showBookmarkFolderInit(allBookmarkItems, site, activeFrame) - } - return templateItem - }).toJS() - return menuUtil.sanitizeTemplateItems(template) -} - -function moreBookmarksTemplateInit (allBookmarkItems, bookmarks, activeFrame) { - const template = bookmarkItemsInit(allBookmarkItems, bookmarks, activeFrame) - template.push({ - l10nLabelId: 'moreBookmarks', - click: function () { - appActions.createTabRequested({ - url: 'about:bookmarks' - }) - windowActions.setContextMenuDetail() - } - }) - return menuUtil.sanitizeTemplateItems(template) -} - function autofillTemplateInit (suggestions, frame) { const template = [] for (let i = 0; i < suggestions.length; ++i) { @@ -1188,7 +1094,7 @@ function mainTemplateInit (nodeProps, frame, tab) { const extensionContextMenus = isPrivate ? undefined - : extensionState.getContextMenusProperties(appStore.state) + : extensionState.getContextMenusProperties(appStoreRenderer.state) if (extensionContextMenus !== undefined && extensionContextMenus.length) { template.push(CommonMenu.separatorMenuItem) @@ -1387,20 +1293,6 @@ function onLedgerContextMenu (location, hostPattern) { menu.popup(getCurrentWindow()) } -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 - })) -} - function onShowAutofillMenu (suggestions, targetRect, frame, boundingClientRect) { const menuTemplate = autofillTemplateInit(suggestions, frame) // toolbar UI scale ratio @@ -1416,17 +1308,7 @@ function onShowAutofillMenu (suggestions, targetRect, frame, boundingClientRect) })) } -function onMoreBookmarksMenu (activeFrame, allBookmarkItems, overflowItems, e) { - const menuTemplate = moreBookmarksTemplateInit(allBookmarkItems, overflowItems, activeFrame) - const rect = e.target.getBoundingClientRect() - windowActions.setContextMenuDetail(Immutable.fromJS({ - right: 0, - top: rect.bottom, - template: menuTemplate - })) -} - -function onReloadContextMenu (target) { +function onReloadContextMenu () { const menuTemplate = [ CommonMenu.reloadPageMenuItem(), CommonMenu.cleanReloadMenuItem() @@ -1446,8 +1328,6 @@ module.exports = { onUrlBarContextMenu, onFindBarContextMenu, onSiteDetailContextMenu, - onShowBookmarkFolderMenu, onShowAutofillMenu, - onMoreBookmarksMenu, onReloadContextMenu } diff --git a/js/dnd.js b/js/dnd.js index 3db55c1cab5..49355f4790c 100644 --- a/js/dnd.js +++ b/js/dnd.js @@ -23,7 +23,7 @@ module.exports.onDragStart = (dragType, data, e) => { e.dataTransfer.effectAllowed = 'all' dndData.setupDataTransferBraveData(e.dataTransfer, dragType, data) if (dragType === dragTypes.BOOKMARK) { - dndData.setupDataTransferURL(e.dataTransfer, data.get('location'), data.get('customTitle') || data.get('title')) + dndData.setupDataTransferURL(e.dataTransfer, data.get('location'), data.get('title')) } appActions.dragStarted(getCurrentWindowId(), dragType, data) } @@ -34,7 +34,7 @@ document.addEventListener('keyup', (e) => { } }, true) -module.exports.onDragEnd = (dragType, key) => { +module.exports.onDragEnd = () => { windowActions.setContextMenuDetail() // TODO: This timeout is a hack to give time for the keyup event to fire. // The keydown event is not fired currently for dragend events that diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index b8fe29b9fb0..6c711a0b49d 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -474,8 +474,8 @@ module.exports.isMoveAllowed = (sites, sourceDetail, destinationDetail) => { module.exports.moveSite = function (state, sourceKey, destinationKey, prepend, destinationIsParent, disallowReparent) { let sites = state.get('sites') - let sourceSite = sites.get(sourceKey) || Immutable.Map() - const destinationSite = sites.get(destinationKey) || Immutable.Map() + let sourceSite = sites.get(sourceKey, Immutable.Map()) + const destinationSite = sites.get(destinationKey, Immutable.Map()) if (sourceSite.isEmpty() || !module.exports.isMoveAllowed(sites, sourceSite, destinationSite)) { return state diff --git a/test/unit/app/common/lib/bookmarkUtilTest.js b/test/unit/app/common/lib/bookmarkUtilTest.js index 034f5629468..9813306f6f7 100644 --- a/test/unit/app/common/lib/bookmarkUtilTest.js +++ b/test/unit/app/common/lib/bookmarkUtilTest.js @@ -265,15 +265,11 @@ describe('bookmarkUtil test', function () { dragData: { dragOverData: { draggingOverType: dragTypes.BOOKMARK, - draggingOverKey: { - location: 'https://brave.com' - } + draggingOverKey: 'https://brave.com|0|0' } } }) - const bookmark = makeImmutable({ - location: 'https://brave.com' - }) + const bookmark = 'https://brave.com|0|0' const result = bookmarkUtil.getDNDBookmarkData(state, bookmark) assert.deepEqual(result, state.getIn(['dragData', 'dragOverData'])) }) @@ -283,15 +279,11 @@ describe('bookmarkUtil test', function () { dragData: { dragOverData: { draggingOverType: dragTypes.BOOKMARK, - draggingOverKey: { - location: 'https://brave.com' - } + draggingOverKey: 'https://brave.com|0|0' } } }) - const bookmark = makeImmutable({ - location: 'https://clifton.io' - }) + const bookmark = 'https://clifton.io|0|0' const result = bookmarkUtil.getDNDBookmarkData(state, bookmark) assert.deepEqual(result, Immutable.Map()) }) diff --git a/test/unit/app/renderer/reducers/contextMenuReducerTest.js b/test/unit/app/renderer/reducers/contextMenuReducerTest.js index 1afee81beb7..1fe160f18b0 100644 --- a/test/unit/app/renderer/reducers/contextMenuReducerTest.js +++ b/test/unit/app/renderer/reducers/contextMenuReducerTest.js @@ -24,7 +24,7 @@ describe('contextMenuReducer', function () { useCleanCache: true }) mockery.registerMock('electron', fakeElectron) - mockery.registerMock('../../../js/l10n.js', fakeLocale) + mockery.registerMock('../../../js/l10n', fakeLocale) fakeElectronMenu = require('../../../lib/fakeElectronMenu') contextMenuReducer = require('../../../../../app/renderer/reducers/contextMenuReducer') })