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

Commit

Permalink
Refactor of bookmarks dnd
Browse files Browse the repository at this point in the history
  • Loading branch information
NejcZdovc committed Jun 27, 2017
1 parent 6293152 commit 47f9dca
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 53 deletions.
17 changes: 6 additions & 11 deletions app/common/lib/bookmarkUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ const displayBookmarkName = (detail) => {
}

const isBookmarkNameValid = (detail, isFolder) => {
const title = detail.get('title') || detail.get('customTitle')
const location = detail.get('location')
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.trim().length > 0
: location.trim().length > 0
}

const showOnlyFavicon = () => {
Expand All @@ -56,16 +56,11 @@ 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) => {
Expand Down
35 changes: 21 additions & 14 deletions app/renderer/components/bookmarks/bookmarkToolbarButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,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 @@ -99,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 @@ -112,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 @@ -124,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 Down Expand Up @@ -155,15 +162,17 @@ class BookmarkToolbarButton extends React.Component {
e.stopPropagation()
}

// TODO merge this two actions into one
windowActions.onShowBookmarkFolderMenu(this.props.bookmarkKey, left, top)
windowActions.setBookmarksToolbarSelectedFolderId(this.props.folderId)
}

mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()
const bookmark = state.getIn(['sites', ownProps.bookmarkKey], Immutable.Map())
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
Expand All @@ -176,15 +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.bookmarkKey = ownProps.bookmarkKey
props.bookmarkKey = bookmarkKey
props.activeFrameKey = activeFrame.get('key')
props.tabId = activeFrame.get('tabId')
props.contextMenuDetail = !!currentWindow.get('contextMenuDetail')
props.bookmark = bookmark // 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')

Expand Down
13 changes: 5 additions & 8 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const BookmarkToolbarButton = require('./bookmarkToolbarButton')

// Actions
const appActions = require('../../../../js/actions/appActions')
const windowActions = require('../../../../js/actions/windowActions')

// State
const windowState = require('../../../common/state/windowState')
Expand All @@ -24,7 +25,6 @@ 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 @@ -35,7 +35,6 @@ const bookmarkUtil = require('../../../common/lib/bookmarkUtil')

// Styles
const globalStyles = require('../styles/global')
var windowActions = require('../../../../js/actions/windowActions.js')

class BookmarksToolbar extends React.Component {
constructor (props) {
Expand All @@ -56,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
42 changes: 23 additions & 19 deletions app/renderer/reducers/contextMenuReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const contextMenuState = require('../../common/state/contextMenuState')

// Actions
const appActions = require('../../../js/actions/appActions')
const windowAction = require('../../../js/actions/windowActions')
const windowActions = require('../../../js/actions/windowActions')
const bookmarkActions = require('../../../js/actions/bookmarkActions')

// Utils
const eventUtil = require('../../../js/lib/eventUtil')
Expand All @@ -33,11 +34,9 @@ const bookmarkUtil = require('../../common/lib/bookmarkUtil')
const siteUtil = require('../../../js/state/siteUtil')
const dnd = require('../../../js/dnd')
const menuUtil = require('../../common/lib/menuUtil')
var urlUtil = require('../../../js/lib/urlutil.js')
var windowActions = require('../../../js/actions/windowActions.js')
var frameStateUtil = require('../../../js/state/frameStateUtil.js')
var bookmarkActions = require('../../../js/actions/bookmarkActions.js')
var dndData = require('../../../js/dndData.js')
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')
Expand Down Expand Up @@ -167,7 +166,7 @@ const addBookmarkMenuItem = (label, siteDetail, closestDestinationDetail, isPare
accelerator: 'CmdOrCtrl+D',
click: () => {
if (isParent) {
siteDetail = siteDetail.set('parentFolderId', closestDestinationDetail && (closestDestinationDetail.get('folderId') || closestDestinationDetail.get('parentFolderId')))
siteDetail = siteDetail.set('parentFolderId', closestDestinationDetail && closestDestinationDetail.get('folderId', closestDestinationDetail.get('parentFolderId')))
}
if (siteDetail.constructor !== Immutable.Map) {
siteDetail = Immutable.fromJS(siteDetail)
Expand All @@ -184,7 +183,7 @@ const addFolderMenuItem = (closestDestinationDetail, isParent) => {
click: () => {
let emptyFolder = Immutable.fromJS({ tags: [siteTags.BOOKMARK_FOLDER] })
if (isParent) {
emptyFolder = emptyFolder.set('parentFolderId', closestDestinationDetail && (closestDestinationDetail.get('folderId') || closestDestinationDetail.get('parentFolderId')))
emptyFolder = emptyFolder.set('parentFolderId', closestDestinationDetail && closestDestinationDetail.get('folderId', closestDestinationDetail.get('parentFolderId')))
}
windowActions.setBookmarkDetail(emptyFolder, undefined, closestDestinationDetail, false)
}
Expand Down Expand Up @@ -367,6 +366,7 @@ const bookmarkItemsInit = (state, items) => {
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')) {
Expand All @@ -375,17 +375,21 @@ const bookmarkItemsInit = (state, items) => {
const templateItem = {
bookmark: site,
draggable: true,
label: site.get('customTitle') || site.get('title') || site.get('location'),
label: site.get('customTitle', site.get('title', site.get('location'))),
icon: showFavicon ? site.get('favicon') : undefined,
faIcon,
contextMenu: function (e) {
onSiteDetailMenu(state, item[0], e)
contextMenu: function () {
windowActions.onSiteDetailMenu(siteKey)
},
dragEnd: function (e) {
dnd.onDragEnd(dragTypes.BOOKMARK, site, e)
dragEnd: function () {
dnd.onDragEnd()
},
dragStart: function (e) {
dnd.onDragStart(dragTypes.BOOKMARK, site, 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()
Expand All @@ -396,15 +400,15 @@ const bookmarkItemsInit = (state, items) => {
const bookmarkItem = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
if (bookmarkItem) {
const bookmarkItemSiteKey = siteUtil.getSiteKey(bookmarkItem)
appActions.moveSite(bookmarkItemSiteKey, item[0], dndData.shouldPrependVerticalItem(e.target, e.clientY))
appActions.moveSite(bookmarkItemSiteKey, siteKey, dndData.shouldPrependVerticalItem(e.target, e.clientY))
}
},
click: function (e) {
bookmarkActions.clickBookmarkItem(item[0], activeFrame.get('tabId'), e)
bookmarkActions.clickBookmarkItem(siteKey, activeFrame.get('tabId'), e)
}
}
if (isFolder) {
templateItem.submenu = showBookmarkFolderInit(state, item[0])
templateItem.submenu = showBookmarkFolderInit(state, siteKey)
}
template.push(templateItem)
}
Expand Down Expand Up @@ -490,7 +494,7 @@ const onLongBackHistory = (state, action) => {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
windowActions.setContextMenuDetail()
}
})

Expand Down Expand Up @@ -543,7 +547,7 @@ const onLongForwardHistory = (state, action) => {
appActions.createTabRequested({
url: 'about:history'
})
windowAction.setContextMenuDetail()
windowActions.setContextMenuDetail()
}
})

Expand Down
2 changes: 1 addition & 1 deletion js/dnd.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,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)
}
Expand Down

0 comments on commit 47f9dca

Please sign in to comment.