Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Removes all non-primitive types from BookmarksToolbar and BookmarkTool… #9713

Merged
merged 1 commit into from
Jul 10, 2017
Merged

Removes all non-primitive types from BookmarksToolbar and BookmarkTool… #9713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions app/common/lib/bookmarkUtil.js
View file
Open in desktop
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ on filter before sort

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(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand this change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of sending the whole object we are sending only keys. So we have list of keys and this way we are not triggering re-renders. We send this key to the child components which gets the whole object from the state.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, makes sense

// 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
Open in desktop
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
32 changes: 12 additions & 20 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
View file
Open in desktop
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,14 @@ 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)
dnd.onDragEnd()
}
return
}
Expand Down Expand Up @@ -128,7 +120,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 +146,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 +174,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