Skip to content

Commit

Permalink
Removes all non-primitve types from BookmarksToolbar and BookmarkTool…
Browse files Browse the repository at this point in the history 
…barButton

Resolves brave#9712

Auditors: @bsclifton

Test Plan:
  • Loading branch information
@NejcZdovc
NejcZdovc committed Jul 9, 2017
1 parent dc76f0a commit 903ffd3
Show file tree
Hide file tree
Showing 12 changed files with 537 additions and 213 deletions.
19 changes: 7 additions & 12 deletions app/common/lib/bookmarkUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand All @@ -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()
Expand Down Expand Up @@ -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()
}
}

Expand Down
71 changes: 39 additions & 32 deletions app/renderer/components/bookmarks/bookmarkToolbarButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
)
}
Expand All @@ -146,45 +142,56 @@ 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')
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)
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')

Expand Down
31 changes: 11 additions & 20 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand All @@ -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) =>
<BookmarkToolbarButton
ref={(node) => this.bookmarkRefs.push(node)}
key={`toolbar-button-${i}`}
bookmark={bookmark} />)
bookmarkKey={bookmarkKey} />)
}
{
this.props.hiddenBookmarks.size !== 0
Expand Down
Loading

0 comments on commit 903ffd3

Please sign in to comment.