Move note between folders #374
Conversation
browser/main/SideNav/StorageItem.js
Outdated
| type: noteData.type, | ||
| updatedAt: noteData.updatedAt, | ||
| description: noteData.description, | ||
| snippets: noteData.snippets |
There was a problem hiding this comment.
Object.assign() is better in this situation. const newNoteData = Object.assign({}, noteData, { storage: storage, folder: folder.key })
browser/main/SideNav/StorageItem.js
Outdated
| e.target.style.opacity = '1' | ||
| e.target.style.backgroundColor = e.dataTransfer.getData('defaultColor') | ||
| const noteData = JSON.parse(e.dataTransfer.getData('note')) | ||
| if (folder.key !== noteData.folder) { |
There was a problem hiding this comment.
To reduce nest, I prefer to use if (folder.key === noteData.folder) return
browser/main/SideNav/StorageItem.js
Outdated
| @@ -7,6 +7,7 @@ import CreateFolderModal from 'browser/main/modals/CreateFolderModal' | |||
| import RenameFolderModal from 'browser/main/modals/RenameFolderModal' | |||
| import dataApi from 'browser/main/lib/dataApi' | |||
| import StorageItemChild from 'browser/components/StorageItem' | |||
| import ee from 'browser/main/lib/eventEmitter' | |||
There was a problem hiding this comment.
Could you change ee to eventEmitter? We're trying to use the name in full.
There was a problem hiding this comment.
Thank you for commenting. I'll change it.
browser/main/SideNav/StorageItem.js
Outdated
| query: {key: note.storage + '-' + note.key} | ||
| }) | ||
| }) | ||
|
|
There was a problem hiding this comment.
We need to handle rollback this copy process just in case it does not work. In this state, only to be copied the note without delete if something happens in copying.
There was a problem hiding this comment.
Thanks commenting @asmsuechan .I agree with your opinion. So, I changed to Deleting the folder only when the folder creation succeeds.
|
|
||
| handleDragLeave (e) { | ||
| e.target.style.opacity = '1' | ||
| e.target.style.backgroundColor = e.dataTransfer.getData('defaultColor') |
There was a problem hiding this comment.
Could you change the background-color? Because it's hard to find the target folder.
There was a problem hiding this comment.
@sosukesuzuki what do you think about this?
There was a problem hiding this comment.
Thanks for commenting @asmsuechan !
Even now, the background color of the folder that is hover while dragging is supposed to change. How do you want the background color to change?
There was a problem hiding this comment.
Oh, I didn't confirm the color changed.
|
I was about to post this as a feature request as many people need this feature too. So it's nice to see you already did it here! |
browser/main/SideNav/StorageItem.js
Outdated
| .then((note) => { | ||
| dataApi | ||
| .deleteNote(noteData.storage, noteData.key) | ||
| .then((data) => { |
There was a problem hiding this comment.
Thanks for commenting ! I'll fix it soon.
browser/main/SideNav/StorageItem.js
Outdated
| }) | ||
| hashHistory.push({ | ||
| pathname: location.pathname, | ||
| query: {key: note.storage + '-' + note.key} |
There was a problem hiding this comment.
you can use template literals like ${note.storage}-${note.key}.
There was a problem hiding this comment.
Thanks for commenting. I'll fix it soon.
However, such a code is often used in Boostnote. For example, there are many other codes that use for, not forEach. We should fix it. I'll do it before long.
I made it allows move note between folders by drag and drop.
