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

Commit

Permalink
Respect bookmark position when adding bookmarks
Browse files Browse the repository at this point in the history 
Fix #11178

Auditors: @bsclifton
  • Loading branch information
@bbondy
bbondy committed Sep 27, 2017
1 parent 9874f96 commit 8e8a6f8
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 14 deletions.
5 changes: 3 additions & 2 deletions app/browser/reducers/bookmarksReducer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const bookmarksReducer = (state, action, immutableAction) => {
{
const closestKey = action.get('closestKey')
const bookmark = action.get('siteDetail')
const isLeftSide = action.get('isLeftSide')

if (bookmark == null) {
break
Expand All @@ -39,14 +40,14 @@ const bookmarksReducer = (state, action, immutableAction) => {
let bookmarkList = Immutable.List()
action.get('siteDetail', Immutable.List()).forEach((bookmark) => {
const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark)
state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey)
state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey, !isLeftSide)
state = syncUtil.updateObjectCache(state, bookmarkDetail, STATE_SITES.BOOKMARKS)
bookmarkList = bookmarkList.push(bookmarkDetail)
})
textCalc.calcTextList(bookmarkList)
} else {
const bookmarkDetail = bookmarkUtil.buildBookmark(state, bookmark)
state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey)
state = bookmarksState.addBookmark(state, bookmarkDetail, closestKey, !isLeftSide)
state = syncUtil.updateObjectCache(state, bookmarkDetail, STATE_SITES.BOOKMARKS)
textCalc.calcText(bookmarkDetail, siteTags.BOOKMARK)
}
Expand Down
4 changes: 2 additions & 2 deletions app/common/state/bookmarksState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const bookmarksState = {
return bookmarks
},

addBookmark: (state, bookmarkDetail, destinationKey) => {
addBookmark: (state, bookmarkDetail, destinationKey, isLeftSide) => {
state = validateState(state)
const key = bookmarkDetail.get('key')
if (key === null) {
Expand All @@ -86,7 +86,7 @@ const bookmarksState = {

if (!state.hasIn([STATE_SITES.BOOKMARKS, key])) {
state = bookmarkLocationCache.addCacheKey(state, bookmarkDetail.get('location'), key)
state = bookmarkOrderCache.addBookmarkToCache(state, bookmarkDetail.get('parentFolderId'), key, destinationKey)
state = bookmarkOrderCache.addBookmarkToCache(state, bookmarkDetail.get('parentFolderId'), key, destinationKey, isLeftSide)
}

state = state.setIn([STATE_SITES.BOOKMARKS, key], bookmarkDetail)
Expand Down
27 changes: 19 additions & 8 deletions app/renderer/components/bookmarks/bookmarksToolbar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,18 @@ class BookmarksToolbar extends React.Component {

onDrop (e) {
e.preventDefault()
const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
if (bookmark) {
// Figure out the droppedOn element filtering out the source drag item
let droppedOn = dnd.closestFromXOffset(this.bookmarkRefs.filter((bookmarkRef) => {
const getClosestFromPos = (clientX, sourceKey) =>
dnd.closestFromXOffset(this.bookmarkRefs.filter((bookmarkRef) => {
if (!bookmarkRef) {
return false
}
return bookmarkRef.props.bookmarkKey !== bookmark.get('key')
return bookmarkRef.props.bookmarkKey !== sourceKey
}), e.clientX)
const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
if (bookmark) {
// Figure out the droppedOn element filtering out the source drag item
const bookmarkKey = bookmark.get('key')
const droppedOn = getClosestFromPos(e.clientX, bookmarkKey)
if (droppedOn.selectedRef) {
const isRightSide = !dnd.isLeftSide(ReactDOM.findDOMNode(droppedOn.selectedRef), e.clientX)
const droppedOnKey = droppedOn.selectedRef.props.bookmarkKey
Expand All @@ -74,6 +77,14 @@ class BookmarksToolbar extends React.Component {
return
}

const droppedOn = getClosestFromPos(e.clientX, undefined)
let isLeftSide = false
let closestKey
if (droppedOn.selectedRef) {
closestKey = droppedOn.selectedRef.props.bookmarkKey
isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOn.selectedRef), e.clientX)
}

const droppedHTML = e.dataTransfer.getData('text/html')
if (droppedHTML) {
const parser = new window.DOMParser()
Expand All @@ -83,7 +94,7 @@ class BookmarksToolbar extends React.Component {
appActions.addBookmark(Immutable.fromJS({
title: a.innerText,
location: e.dataTransfer.getData('text/plain')
}))
}), closestKey, isLeftSide)
return
}
}
Expand All @@ -93,7 +104,7 @@ class BookmarksToolbar extends React.Component {
item.getAsString((name) => appActions.addBookmark(Immutable.fromJS({
location: item.type,
title: name
})))
}), closestKey, isLeftSide))
})
return
}
Expand All @@ -103,7 +114,7 @@ class BookmarksToolbar extends React.Component {
.map((x) => x.trim())
.filter((x) => !x.startsWith('#') && x.length > 0)
.forEach((url) =>
appActions.addBookmark(Immutable.fromJS({ location: url })))
appActions.addBookmark(Immutable.fromJS({ location: url }), closestKey, isLeftSide))
}

onDragEnter (e) {
Expand Down
5 changes: 3 additions & 2 deletions js/actions/appActions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1493,11 +1493,12 @@ const appActions = {
* @param siteDetail{Immutable.Map|Immutable.List} - Bookmark details that we want to add, this can be a List as well
* @param closestKey{string} - Key of the sibling where we would like to place this new bookmark
*/
addBookmark: function (siteDetail, closestKey) {
addBookmark: function (siteDetail, closestKey, isLeftSide = false) {
dispatch({
actionType: appConstants.APP_ADD_BOOKMARK,
siteDetail,
closestKey
closestKey,
isLeftSide
})
},

Expand Down
116 changes: 116 additions & 0 deletions test/unit/app/browser/reducers/bookmarksReducerTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,122 @@ describe('bookmarksReducer unit test', function () {
assert.equal(spyCalc.callCount, 1)
assert.deepEqual(newState.toJS(), expectedState.toJS())
})

it('add a bookmark with a close bookmark prepending', function () {
const newState = state.set('bookmarks', Immutable.fromJS({
'https://www.clifton.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.brave.com',
parentFolderId: 0
},
'https://www.bbondy.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.bbondy.io',
parentFolderId: 0
},
'https://www.bridiver.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.bridiver.io',
parentFolderId: 0
}
}))
.setIn(['cache', 'bookmarkOrder', '0'], Immutable.fromJS([
{
key: 'https://clifton.io/|0|0',
order: 0,
type: siteTags.BOOKMARK
},
{
key: 'https://www.bbondy.io|0|0',
order: 1,
type: siteTags.BOOKMARK
},
{
key: 'https://www.bridiver.io|0|0',
order: 2,
type: siteTags.BOOKMARK
}
]))
const action = {
actionType: appConstants.APP_ADD_BOOKMARK,
siteDetail: Immutable.fromJS({
parentFolderId: 0,
title: 'Brave',
location: 'https://www.brave.com'
}),
tag: siteTags.BOOKMARK,
closestKey: 'https://www.bbondy.io|0|0',
isLeftSide: true
}

const newBookmarks = {
'https://www.clifton.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.brave.com',
parentFolderId: 0
},
'https://www.bbondy.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.bbondy.io',
parentFolderId: 0
},
'https://www.brave.com|0|0': {
favicon: undefined,
key: 'https://www.brave.com|0|0',
objectId: null,
title: 'Brave',
location: 'https://www.brave.com',
parentFolderId: 0,
partitionNumber: 0,
skipSync: null,
themeColor: undefined,
type: 'bookmark',
width: 0
},
'https://www.bridiver.io|0|0': {
lastAccessedTime: 0,
objectId: null,
title: 'Brave',
location: 'https://www.bridiver.io',
parentFolderId: 0
}
}

const newBookmarksOrder = [
{
key: 'https://clifton.io/|0|0',
order: 0,
type: siteTags.BOOKMARK
},
{
key: 'https://www.brave.com|0|0',
order: 1,
type: siteTags.BOOKMARK
},
{
key: 'https://www.bbondy.io|0|0',
order: 2,
type: siteTags.BOOKMARK
},
{
key: 'https://www.bridiver.io|0|0',
order: 3,
type: siteTags.BOOKMARK
}]
const result = bookmarksReducer(newState, action)
assert.deepEqual(result.get('bookmarks').toJS(), newBookmarks)
assert.deepEqual(result.getIn(['cache', 'bookmarkOrder', '0']).toJS(), newBookmarksOrder)
})
})

describe('APP_EDIT_BOOKMARK', function () {
Expand Down

1 comment on commit 8e8a6f8

@bsclifton
Copy link
Member

Choose a reason for hiding this comment

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

++

Please sign in to comment.