From e21ade14872628054b404541ebbcb30c82f99ffc Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Mon, 22 May 2017 20:52:42 +0200 Subject: [PATCH] Converts bookmarksToolbar and bookmarkToolbarButton into redux Resolves #9421 Auditors: @bsclifton @bridiver Test Plan: --- app/common/state/bookmarksState.js | 19 ++ .../bookmarks/bookmarkToolbarButton.js | 166 ++++++++++-------- .../components/bookmarks/bookmarksToolbar.js | 143 +++++++++------ app/renderer/components/main/main.js | 18 +- 4 files changed, 201 insertions(+), 145 deletions(-) create mode 100644 app/common/state/bookmarksState.js diff --git a/app/common/state/bookmarksState.js b/app/common/state/bookmarksState.js new file mode 100644 index 00000000000..8ef72176b1b --- /dev/null +++ b/app/common/state/bookmarksState.js @@ -0,0 +1,19 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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 Immutable = require('immutable') + +// Constants +const dragTypes = require('../../../js/constants/dragTypes') + +const bookmarksState = { + getDNDBookmarkData: (state, bookmark) => { + const data = (state.getIn(['dragData', 'dragOverData', 'draggingOverType']) === dragTypes.BOOKMARK && + state.getIn(['dragData', 'dragOverData'], Immutable.Map())) || Immutable.Map() + + return (Immutable.is(data.get('draggingOverKey'), bookmark)) ? data : Immutable.Map() + } +} + +module.exports = bookmarksState diff --git a/app/renderer/components/bookmarks/bookmarkToolbarButton.js b/app/renderer/components/bookmarks/bookmarkToolbarButton.js index 3904d899dac..47fc475fdc9 100644 --- a/app/renderer/components/bookmarks/bookmarkToolbarButton.js +++ b/app/renderer/components/bookmarks/bookmarkToolbarButton.js @@ -8,33 +8,42 @@ const Immutable = require('immutable') const {StyleSheet, css} = require('aphrodite/no-important') // Components -const ImmutableComponent = require('../immutableComponent') +const ReduxComponent = require('../reduxComponent') // Actions 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') +// State +const bookmarkState = require('../../../common/state/bookmarksState') + // Constants -const siteTags = require('../../../../js/constants/siteTags') 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') // Styles const globalStyles = require('../styles/global') -class BookmarkToolbarButton extends ImmutableComponent { - constructor () { - super() +class BookmarkToolbarButton extends React.Component { + constructor (props) { + super(props) this.onClick = this.onClick.bind(this) + this.onAuxClick = this.onAuxClick.bind(this) this.onMouseOver = this.onMouseOver.bind(this) this.onDragStart = this.onDragStart.bind(this) this.onDragEnd = this.onDragEnd.bind(this) @@ -42,38 +51,45 @@ class BookmarkToolbarButton extends ImmutableComponent { this.onDragLeave = this.onDragLeave.bind(this) this.onDragOver = this.onDragOver.bind(this) this.onContextMenu = this.onContextMenu.bind(this) + this.openContextMenu = this.openContextMenu.bind(this) + this.clickBookmarkItem = this.clickBookmarkItem.bind(this) + this.showBookmarkFolderMenu = this.showBookmarkFolderMenu.bind(this) } + componentDidMount () { - this.bookmarkNode.addEventListener('auxclick', this.onAuxClick.bind(this)) + this.bookmarkNode.addEventListener('auxclick', this.onAuxClick) } + get activeFrame () { return windowStore.getFrame(this.props.activeFrameKey) } + onAuxClick (e) { if (e.button === 1) { this.onClick(e) } } + onClick (e) { - if (!this.props.clickBookmarkItem(this.props.bookmark, e) && - this.props.bookmark.get('tags').includes(siteTags.BOOKMARK_FOLDER)) { + if (!this.clickBookmarkItem(e) && this.props.isFolder) { if (this.props.contextMenuDetail) { windowActions.setContextMenuDetail() return } + e.target = ReactDOM.findDOMNode(this) - this.props.showBookmarkFolderMenu(this.props.bookmark, e) + 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')) { + if (this.props.isFolder && this.props.selectedFolderId !== this.props.folderId) { // Auto-expand the menu if user mouses over another folder e.target = ReactDOM.findDOMNode(this) - this.props.showBookmarkFolderMenu(this.props.bookmark, e) - } else if (!this.isFolder && this.props.selectedFolderId !== -1) { + this.showBookmarkFolderMenu(e) + } else if (!this.props.isFolder && this.props.selectedFolderId !== -1) { // Hide the currently expanded menu if user mouses over a non-folder windowActions.setBookmarksToolbarSelectedFolderId(-1) windowActions.setContextMenuDetail() @@ -91,10 +107,10 @@ class BookmarkToolbarButton extends ImmutableComponent { onDragEnter (e) { // Bookmark specific DND code to expand hover when on a folder item - if (this.isFolder) { + if (this.props.isFolder) { e.target = ReactDOM.findDOMNode(this) if (dnd.isMiddle(e.target, e.clientX)) { - this.props.showBookmarkFolderMenu(this.props.bookmark, e) + this.showBookmarkFolderMenu(e) appActions.draggedOver({ draggingOverKey: this.props.bookmark, draggingOverType: dragTypes.BOOKMARK, @@ -105,9 +121,9 @@ class BookmarkToolbarButton extends ImmutableComponent { } } - onDragLeave (e) { + onDragLeave () { // Bookmark specific DND code to expand hover when on a folder item - if (this.isFolder) { + if (this.props.isFolder) { appActions.draggedOver({ draggingOverKey: this.props.bookmark, draggingOverType: dragTypes.BOOKMARK, @@ -118,49 +134,63 @@ class BookmarkToolbarButton extends ImmutableComponent { } onDragOver (e) { - dnd.onDragOver(dragTypes.BOOKMARK, this.bookmarkNode.getBoundingClientRect(), this.props.bookmark, this.draggingOverData, e) + dnd.onDragOver( + dragTypes.BOOKMARK, + this.bookmarkNode.getBoundingClientRect(), + this.props.bookmark, + this.props.draggingOverData, + e + ) } - get draggingOverData () { - if (!this.props.draggingOverData || - !Immutable.is(this.props.draggingOverData.get('draggingOverKey'), this.props.bookmark)) { - return - } - - return this.props.draggingOverData + onContextMenu (e) { + this.openContextMenu(e) } - get isDragging () { - return Immutable.is(this.props.bookmark, dnd.getInterBraveDragData()) + openContextMenu (e) { + contextMenus.onSiteDetailContextMenu(this.props.bookmark, this.activeFrame, e) } - get isDraggingOverLeft () { - if (!this.draggingOverData) { - return false - } - return this.draggingOverData.get('draggingOverLeftHalf') + clickBookmarkItem (e) { + return bookmarkActions.clickBookmarkItem(this.props.bookmarks, this.props.bookmark, this.activeFrame, e) } - get isExpanded () { - if (!this.props.draggingOverData) { - return false - } - return this.props.draggingOverData.get('expanded') + showBookmarkFolderMenu (e) { + windowActions.setBookmarksToolbarSelectedFolderId(this.props.folderId) + contextMenus.onShowBookmarkFolderMenu(this.props.bookmarks, this.props.bookmark, this.activeFrame, e) } - get isDraggingOverRight () { - if (!this.draggingOverData) { - return false - } - return this.draggingOverData.get('draggingOverRightHalf') - } + 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 = bookmarkState.getDNDBookmarkData(state, bookmark) - get isFolder () { - return siteUtil.isFolder(this.props.bookmark) - } + const props = {} + // used in renderer + props.showFavicon = btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || + btbMode === bookmarksToolbarMode.FAVICONS_ONLY + props.showOnlyFavicon = btbMode === bookmarksToolbarMode.FAVICONS_ONLY + props.favIcon = bookmark.get('favicon') + props.title = bookmark.get('customTitle', bookmark.get('title')) + props.location = bookmark.get('location') + props.isFolder = siteUtil.isFolder(bookmark) + 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) - onContextMenu (e) { - this.props.openContextMenu(this.props.bookmark, e) + // 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.activeFrameKey = activeFrame.get('key') + props.selectedFolderId = currentWindow.getIn(['ui', 'bookmarksToolbar', 'selectedFolderId']) + props.folderId = bookmark.get('folderId') + + return props } render () { @@ -171,36 +201,30 @@ class BookmarkToolbarButton extends ImmutableComponent { } if (showingFavicon) { - let icon = this.props.bookmark.get('favicon') - - if (icon) { + if (this.props.favIcon) { iconStyle = Object.assign(iconStyle, { - backgroundImage: `url(${icon})`, + backgroundImage: `url(${this.props.favIcon})`, backgroundSize: iconSize, height: iconSize }) - } else if (!this.isFolder) { + } else if (!this.props.isFolder) { showingFavicon = false } } - const siteDetailTitle = this.props.bookmark.get('customTitle') || this.props.bookmark.get('title') - const siteDetailLocation = this.props.bookmark.get('location') - let hoverTitle - if (this.isFolder) { - hoverTitle = siteDetailTitle - } else { - hoverTitle = siteDetailTitle - ? siteDetailTitle + '\n' + siteDetailLocation - : siteDetailLocation + let hoverTitle = this.props.title + if (!this.props.isFolder) { + hoverTitle = this.props.title + ? this.props.title + '\n' + this.props.location + : this.props.location } return { - this.isFolder && this.props.showFavicon + this.props.isFolder && this.props.showFavicon ? { - (this.isFolder ? false : (this.props.showFavicon && this.props.showOnlyFavicon)) + (this.props.isFolder ? false : (this.props.showFavicon && this.props.showOnlyFavicon)) ? '' - : siteDetailTitle || siteDetailLocation + : this.props.title || this.props.location } { - this.isFolder + this.props.isFolder ? 0) { Array.from(e.dataTransfer.files).forEach((file) => appActions.addSite({ location: file.path, title: file.name }, siteTags.BOOKMARK)) @@ -98,19 +106,9 @@ class BookmarksToolbar extends ImmutableComponent { .forEach((url) => appActions.addSite({ location: url }, siteTags.BOOKMARK)) } - openContextMenu (bookmark, e) { - contextMenus.onSiteDetailContextMenu(bookmark, this.activeFrame, e) - } - clickBookmarkItem (bookmark, e) { - return bookmarkActions.clickBookmarkItem(this.bookmarks, bookmark, this.activeFrame, e) - } - showBookmarkFolderMenu (bookmark, e) { - windowActions.setBookmarksToolbarSelectedFolderId(bookmark.get('folderId')) - contextMenus.onShowBookmarkFolderMenu(this.bookmarks, bookmark, this.activeFrame, e) - } + updateBookmarkData (props) { - // TODO(darkdh): Remove siteSort when we have #9030 landed - this.bookmarks = siteUtil.getBookmarks(props.sites).sort(siteUtil.siteSort) + this.bookmarks = siteUtil.getBookmarks(props.sites) const noParentItems = this.bookmarks .filter((bookmark) => !bookmark.get('parentFolderId')) @@ -119,18 +117,23 @@ class BookmarksToolbar extends ImmutableComponent { // Dynamically calculate how many bookmark items should appear on the toolbar // before it is actually rendered. - this.maxWidth = domUtil.getStyleConstants('bookmark-item-max-width') - this.padding = domUtil.getStyleConstants('bookmark-item-padding') * 2 + const maxWidth = domUtil.getStyleConstants('bookmark-item-max-width') + const padding = domUtil.getStyleConstants('bookmark-item-padding') * 2 // Toolbar padding is only on the left - this.toolbarPadding = domUtil.getStyleConstants('bookmarks-toolbar-padding') - this.bookmarkItemMargin = domUtil.getStyleConstants('bookmark-item-margin') * 2 + const toolbarPadding = domUtil.getStyleConstants('bookmarks-toolbar-padding') + const bookmarkItemMargin = domUtil.getStyleConstants('bookmark-item-margin') * 2 // No margin for show only favicons - this.chevronMargin = domUtil.getStyleConstants('bookmark-item-chevron-margin') - this.fontSize = domUtil.getStyleConstants('bookmark-item-font-size') - this.fontFamily = domUtil.getStyleConstants('default-font-family') - this.chevronWidth = this.chevronMargin + this.fontSize - const margin = props.showFavicon && props.showOnlyFavicon ? 0 : this.bookmarkItemMargin - widthAccountedFor += this.toolbarPadding + const chevronMargin = domUtil.getStyleConstants('bookmark-item-chevron-margin') + const fontSize = domUtil.getStyleConstants('bookmark-item-font-size') + const fontFamily = domUtil.getStyleConstants('default-font-family') + const chevronWidth = chevronMargin + fontSize + const margin = props.showFavicon && props.showOnlyFavicon ? 0 : bookmarkItemMargin + widthAccountedFor += toolbarPadding + + // TODO + // Maybe move this calculation in state so we only calculate it when you add or edit bookmark, + // this way we will calculate it only once + // also need to recalculate it when you toggle option show only fav icons // Loop through until we fill up the entire bookmark toolbar width let i @@ -140,12 +143,12 @@ class BookmarksToolbar extends ImmutableComponent { if (props.showFavicon && !noParentItems.getIn([i, 'folderId']) && !noParentItems.getIn([i, 'favicon'])) { iconWidth -= 3 } - const chevronWidth = props.showFavicon && noParentItems.getIn([i, 'folderId']) ? this.chevronWidth : 0 + const currentChevronWidth = props.showFavicon && noParentItems.getIn([i, 'folderId']) ? chevronWidth : 0 if (props.showFavicon && props.showOnlyFavicon) { - widthAccountedFor += this.padding + iconWidth + chevronWidth + widthAccountedFor += padding + iconWidth + currentChevronWidth } else { const text = noParentItems.getIn([i, 'customTitle']) || noParentItems.getIn([i, 'title']) || noParentItems.getIn([i, 'location']) - widthAccountedFor += Math.min(calculateTextWidth(text, `${this.fontSize} ${this.fontFamily}`) + this.padding + iconWidth + chevronWidth, this.maxWidth) + widthAccountedFor += Math.min(calculateTextWidth(text, `${fontSize} ${fontFamily}`) + padding + iconWidth + currentChevronWidth, maxWidth) } widthAccountedFor += margin if (widthAccountedFor >= props.windowWidth - overflowButtonWidth) { @@ -156,9 +159,12 @@ class BookmarksToolbar extends ImmutableComponent { // Show at most 100 items in the overflow menu this.overflowBookmarkItems = noParentItems.skip(i).take(100) } + + // TODO remove componentWillMount () { this.updateBookmarkData(this.props) } + componentWillUpdate (nextProps) { if (nextProps.sites !== this.props.sites || nextProps.windowWidth !== this.props.windowWidth || @@ -167,6 +173,7 @@ class BookmarksToolbar extends ImmutableComponent { this.updateBookmarkData(nextProps) } } + onDragEnter (e) { if (dndData.hasDragData(e.dataTransfer, dragTypes.BOOKMARK)) { if (Array.from(e.target.classList).includes('overflowIndicator')) { @@ -174,6 +181,7 @@ class BookmarksToolbar extends ImmutableComponent { } } } + onDragOver (e) { const sourceDragData = dndData.getDragData(e.dataTransfer, dragTypes.BOOKMARK) if (sourceDragData) { @@ -181,7 +189,7 @@ class BookmarksToolbar extends ImmutableComponent { e.preventDefault() return } - // console.log(e.dataTransfer.types, e.dataTransfer.getData('text/plain'), e.dataTransfer.getData('text/uri-list'), e.dataTransfer.getData('text/html')) + let intersection = e.dataTransfer.types.filter((x) => ['text/plain', 'text/uri-list', 'text/html', 'Files'].includes(x)) if (intersection.length > 0) { @@ -189,23 +197,53 @@ class BookmarksToolbar extends ImmutableComponent { e.preventDefault() } } + onMoreBookmarksMenu (e) { contextMenus.onMoreBookmarksMenu(this.activeFrame, this.bookmarks, this.overflowBookmarkItems, e) } + onContextMenu (e) { const closest = dnd.closestFromXOffset(this.bookmarkRefs.filter((x) => !!x), e.clientX).selectedRef - contextMenus.onTabsToolbarContextMenu(this.activeFrame.get('title'), this.activeFrame.get('location'), (closest && closest.props.bookmark) || undefined, closest && closest.isDroppedOn, e) + contextMenus.onTabsToolbarContextMenu( + this.props.title, + this.props.location, + (closest && closest.props.bookmark) || undefined, + closest && closest.isDroppedOn, + e + ) } - render () { - let showFavicon = this.props.showFavicon - let showOnlyFavicon = this.props.showOnlyFavicon + mergeProps (state, ownProps) { + const currentWindow = state.get('currentWindow') + const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() + const btbMode = getSetting(settings.BOOKMARKS_TOOLBAR_MODE) + + const props = {} + // used in renderer + props.showOnlyFavicon = btbMode === bookmarksToolbarMode.FAVICONS_ONLY + props.showFavicon = btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || + btbMode === bookmarksToolbarMode.FAVICONS_ONLY + props.shouldAllowWindowDrag = windowState.shouldAllowWindowDrag(state, currentWindow, activeFrame, isFocused()) && + !isWindows() + + // used in other functions + props.activeFrameKey = activeFrame.get('key') + props.title = activeFrame.get('title') + props.location = activeFrame.get('location') + props.windowWidth = window.innerWidth + + props.sites = state.get('sites') // TODO (nejc) only primitives + + return props + } + + render () { this.bookmarkRefs = [] return
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} - showFavicon={this.props.showFavicon} - showOnlyFavicon={this.props.showOnlyFavicon} - selectedFolderId={this.props.selectedFolderId} />) + key={`toolbar-button-${i}`} + bookmark={bookmark} />) } { this.overflowBookmarkItems.size !== 0 @@ -277,4 +306,4 @@ const styles = StyleSheet.create({ } }) -module.exports = BookmarksToolbar +module.exports = ReduxComponent.connect(BookmarksToolbar) diff --git a/app/renderer/components/main/main.js b/app/renderer/components/main/main.js index f60800eab8b..f06217bde72 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -44,10 +44,8 @@ const CheckDefaultBrowserDialog = require('./checkDefaultBrowserDialog') const appConfig = require('../../../../js/constants/appConfig') const messages = require('../../../../js/constants/messages') const settings = require('../../../../js/constants/settings') -const dragTypes = require('../../../../js/constants/dragTypes') const keyCodes = require('../../../common/constants/keyCodes') const keyLocations = require('../../../common/constants/keyLocations') -const {bookmarksToolbarMode} = require('../../../common/constants/settingsEnums') // State handling const basicAuthState = require('../../../common/state/basicAuthState') @@ -624,9 +622,6 @@ class Main extends ImmutableComponent { const nonPinnedFrames = frameStateUtil.getNonPinnedFrames(this.props.windowState) const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) const showBookmarksToolbar = getSetting(settings.SHOW_BOOKMARKS_TOOLBAR) - const btbMode = getSetting(settings.BOOKMARKS_TOOLBAR_MODE) - const showFavicon = (btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || btbMode === bookmarksToolbarMode.FAVICONS_ONLY) - const showOnlyFavicon = btbMode === bookmarksToolbarMode.FAVICONS_ONLY const siteInfoIsVisible = this.props.windowState.getIn(['ui', 'siteInfo', 'isVisible']) && !isSourceAboutUrl(activeFrame.get('location')) const braveryPanelIsVisible = shieldState.braveShieldsEnabled(activeFrame) && this.props.windowState.get('braveryPanelDetail') @@ -649,8 +644,6 @@ class Main extends ImmutableComponent { const notificationBarIsVisible = activeOrigin && this.props.appState.get('notifications').filter((item) => item.get('frameOrigin') ? activeOrigin === item.get('frameOrigin') : true).size > 0 - const appStateSites = this.props.appState.get('sites') - return
{ showBookmarksToolbar - ? + ? : null }