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

Commit

Permalink
move bookmark drop logic in bm toolbar to the store
Browse files Browse the repository at this point in the history
-
this allow us to add more precise tests
fix #11226
  • Loading branch information
cezaraugusto committed Jan 15, 2018
1 parent 86c1ac4 commit 653ab1d
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 38 deletions.
41 changes: 40 additions & 1 deletion app/browser/reducers/bookmarkToolbarReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ const appConstants = require('../../../js/constants/appConstants')
const bookmarksState = require('../../common/state/bookmarksState')
const bookmarkFoldersState = require('../../common/state/bookmarkFoldersState')

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

// Constants
const siteTags = require('../../../js/constants/siteTags')

// Util
const {makeImmutable} = require('../../common/state/immutableUtil')
const textCalc = require('../../browser/api/textCalc')
// const dnd = require('../../../js/dnd')

const bookmarkToolbarReducer = (state, action, immutableAction) => {
action = immutableAction || makeImmutable(action)
Expand All @@ -32,6 +37,40 @@ const bookmarkToolbarReducer = (state, action, immutableAction) => {
}
break
case appConstants.APP_ON_DROP_BOOKMARK: {
let bookmark = action.get('bookmark')
let tabDrop = false

// When we have key null is only when we are getting data from TAB transfer type
if (bookmark.get('key') == null) {
tabDrop = true
}

const isDestinationParent = action.get('isFolder') && action.get('isDroppedOn')

// tabDrop is the action of dropping a bookmark from the urlbar
if (tabDrop) {
const parentKey = isDestinationParent ? action.get('droppedOnKey') : null
bookmark = bookmark.set('parentFolderId', parentKey)
appActions.addBookmark(bookmark)
} else {
// if not a tabdrop, then user is just moving bookmarks around
// in this case, check whether dragged element is a folder or not
if (bookmark.get('type') === siteTags.BOOKMARK_FOLDER) {
appActions.moveBookmarkFolder(
bookmark.get('key'),
action.get('droppedOnKey'),
action.get('isRightSide'),
isDestinationParent
)
} else {
appActions.moveBookmark(
bookmark.get('key'),
action.get('droppedOnKey'),
action.get('isRightSide'),
isDestinationParent
)
}
}
break
}
}
Expand Down
11 changes: 11 additions & 0 deletions app/common/lib/bookmarkUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const bookmarkLocationCache = require('../cache/bookmarkLocationCache')
const {getSetting} = require('../../../js/settings')
const UrlUtil = require('../../../js/lib/urlutil')
const {makeImmutable} = require('../state/immutableUtil')
const dnd = require('../../../js/dnd')

const bookmarkHangerHeading = (editMode, isAdded) => {
if (isAdded) {
Expand Down Expand Up @@ -64,6 +65,15 @@ const getDNDBookmarkData = (state, bookmarkKey) => {
return data.get('draggingOverKey') === bookmarkKey ? data : Immutable.Map()
}

const getClosestFromPos = (refs, clientX, sourceKey) => {
return dnd.closestFromXOffset(refs.filter((bookmarkRef) => {
if (!bookmarkRef) {
return false
}
return bookmarkRef.props.bookmarkKey !== sourceKey
}), clientX)
}

const getDetailFromFrame = (frame) => {
if (frame == null) {
return null
Expand Down Expand Up @@ -226,6 +236,7 @@ module.exports = {
showOnlyFavicon,
showFavicon,
getDNDBookmarkData,
getClosestFromPos,
getDetailFromFrame,
isLocationBookmarked,
toCreateProperties,
Expand Down
47 changes: 18 additions & 29 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const bookmarkToolbarState = require('../../../common/state/bookmarkToolbarState

// Constants
const dragTypes = require('../../../../js/constants/dragTypes')
const siteTags = require('../../../../js/constants/siteTags')

// Utils
const {isFocused} = require('../../currentWindow')
Expand All @@ -32,7 +31,6 @@ const dndData = require('../../../../js/dndData')
const isWindows = require('../../../common/lib/platformUtil').isWindows()
const frameStateUtil = require('../../../../js/state/frameStateUtil')
const bookmarkUtil = require('../../../common/lib/bookmarkUtil')
const bookmarkDndUtil = require('../../../common/lib/bookmarkDndUtil')
const {elementHasDataset} = require('../../../../js/lib/eventUtil')
const {getCurrentWindowId} = require('../../currentWindow')

Expand All @@ -51,35 +49,26 @@ class BookmarksToolbar extends React.Component {

onDrop (e) {
e.preventDefault()
let bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
if (bookmark) {
// Figure out the droppedOn element filtering out the source drag item
let bookmarkKey = bookmark.get('key')
let tabDrop = false

// When we have key null is only when we are getting data from TAB transfer type
if (bookmarkKey == null) {
tabDrop = true
}

const droppedOn = bookmarkDndUtil.getClosestFromPos(this.bookmarkRefs, e.clientX, bookmarkKey)
if (droppedOn.selectedRef) {
const isRightSide = !dnd.isLeftSide(ReactDOM.findDOMNode(droppedOn.selectedRef), e.clientX)
const droppedOnKey = droppedOn.selectedRef.props.bookmarkKey
const isDestinationParent = droppedOn.selectedRef.state.isFolder && droppedOn && droppedOn.isDroppedOn
if (tabDrop) {
const parentKey = isDestinationParent ? droppedOnKey : null
bookmark = bookmark.set('parentFolderId', parentKey)
appActions.addBookmark(bookmark)
} else {
if (bookmark.get('type') === siteTags.BOOKMARK_FOLDER) {
appActions.moveBookmarkFolder(bookmarkKey, droppedOnKey, isRightSide, isDestinationParent)
} else {
appActions.moveBookmark(bookmarkKey, droppedOnKey, isRightSide, isDestinationParent)
}
}
dnd.onDragEnd()
const droppedOn = bookmarkUtil.getClosestFromPos(
this.bookmarkRefs,
e.clientX,
bookmark.get('key')
)
const currentNode = ReactDOM.findDOMNode(droppedOn.selectedRef)
const isRightSide = dnd.isRightSide(currentNode, e.clientX)

if (droppedOn && droppedOn.selectedRef) {
appActions.onDropBookmark(
bookmark,
droppedOn.selectedRef.props.bookmarkKey,
droppedOn.selectedRef.state.isFolder,
droppedOn.isDroppedOn,
isRightSide
)
}
dnd.onDragEnd()
}
}

Expand Down
8 changes: 6 additions & 2 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1583,10 +1583,14 @@ const appActions = {
})
},

onDropBookmark: function (folderKey) {
onDropBookmark: function (bookmark, droppedOnKey, isFolder, isDroppedOn, isRightSide) {
dispatch({
actionType: appConstants.APP_ON_DROP_BOOKMARK,
folderKey
bookmark,
droppedOnKey,
isFolder,
isDroppedOn,
isRightSide
})
},

Expand Down
16 changes: 10 additions & 6 deletions js/dnd.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const dragTypes = require('./constants/dragTypes')
const siteTags = require('./constants/siteTags')
const appStoreRenderer = require('./stores/appStoreRenderer')
const {getCurrentWindowId} = require('../app/renderer/currentWindow')
const {ESC} = require('../app/common/constants/keyCodes.js')
// const {ESC} = require('../app/common/constants/keyCodes.js')

module.exports.getInterBraveDragData = () => {
return appStoreRenderer.state.getIn(['dragData', 'data'])
Expand All @@ -30,11 +30,11 @@ module.exports.onDragStart = (dragType, data, e) => {
appActions.dragStarted(getCurrentWindowId(), dragType, data)
}

document.addEventListener('keyup', (e) => {
if (e.keyCode === ESC) {
appActions.dragCancelled()
}
}, true)
// document.addEventListener('keyup', (e) => {
// if (e.keyCode === ESC) {
// appActions.dragCancelled()
// }
// }, true)

module.exports.onDragEnd = () => {
windowActions.setContextMenuDetail()
Expand Down Expand Up @@ -127,6 +127,10 @@ module.exports.isLeftSide = (domNode, clientX) => {
return clientX < boundingRect.left + ((boundingRect.right - boundingRect.left) / 2)
}

module.exports.isRightSide = (domNode, clientX) => {
return !module.exports.isLeftSide(domNode, clientX)
}

module.exports.isMiddle = (domNode, clientX) => {
const boundingRect = domNode.getBoundingClientRect()
const isLeft = clientX < boundingRect.left + ((boundingRect.right - boundingRect.left) / 3)
Expand Down

0 comments on commit 653ab1d

Please sign in to comment.