Feature multiselect notes delete and move to another folder #1070

voidsatisfaction · 2017-11-05T12:49:20Z

WHY?

#1058

HOW?

multi selection with shift key
multi delete
multi drag and drop

I made it by giving state to NoteList

I changed lots of codes so that there would be mistakes.. If so, please let me know then I will fix it.

kazup01 · 2017-11-05T15:05:53Z

I crucially wanted this feature. Thank you for your great contribution every time!
We will check it!

tatoosh · 2017-11-06T08:34:09Z

Please commit

voidsatisfaction · 2017-11-06T09:28:58Z

Todo

Support multiple notes shortcut delete
Support multiple pin to top
~~Refactor from saving state of note-id to note object~~ refactor codes

voidsatisfaction · 2017-11-07T14:18:02Z

I implemented follows in addition to multi deletions and multi move notes to other folder

Multi Pins to top

Multi Select with `shift + arrow-down(up as well)` + Multi Delete with `shift + d`

I tried Ctrl + leftClick to multi selection. But it pops up context menu therefore I thought it is also okay to use shift instead

voidsatisfaction · 2017-11-07T14:19:38Z

@kazup01 Please review this pull request ^^

kazup01 · 2017-11-07T14:20:40Z

Great! Thank you :)

sota1235 · 2017-11-13T14:00:08Z

browser/main/NoteList/index.js


+  focusNote (selectedNoteKeys, noteKey) {
+    let { router } = this.context
+    let { location } = this.props


plz use const instead of let 🙏

(we know there are many let in existing code. but it doesn't make sense, so we want to use const as possible as we can)

I strongly agree with your suggestion! I changed

sota1235 · 2017-11-13T14:02:36Z

browser/main/NoteList/index.js

-    })
+    if (!shiftKeyDown) { selectedNoteKeys = [] }
+    const priorNote = Object.assign({}, this.notes[targetIndex])
+    const priorNoteKey = `${priorNote.storage}-${priorNote.key}`


You can use getNoteKey for creating note key.

I used that. Thanks.

sota1235 · 2017-11-13T14:03:20Z

browser/main/NoteList/index.js

+    if (!shiftKeyDown) { selectedNoteKeys = [] }
+    const priorNote = Object.assign({}, this.notes[targetIndex])
+    const priorNoteKey = `${priorNote.storage}-${priorNote.key}`
+    if (selectedNoteKeys.includes(priorNoteKey)) {


You can use findNoteByKey to check that note key exists in array or not.

@sota1235
No, It findNoteByKeyis only for find note not keys.

Currently I have state that only save selectedNoteKeys not notes

sota1235 · 2017-11-13T14:04:27Z

browser/main/NoteList/index.js

-    if (targetIndex === this.notes.length - 1) {
+    if (targetIndex === this.notes.length - 1 && shiftKeyDown) {
+      return
+    } else if (targetIndex === this.notes.length - 1) {


Please add some variables to keep value of targetIndex === this.notes.length - 1

I added variable. Thank you!

sota1235 · 2017-11-13T14:22:38Z

btw, this feature is so useful :) Thank you so much for your contributon!

voidsatisfaction · 2017-11-13T14:25:04Z

@sota1235 Thank you for review! I will check as soon as possible ^^

voidsatisfaction · 2017-11-15T14:24:34Z

@sota1235 @kazup01 I reviewed feedbacks.

kazup01 · 2017-11-15T14:33:06Z

Thank you @voidsatisfaction :)

voidsatisfaction · 2017-11-07T01:07:19Z

browser/main/NoteList/index.js


    let targetIndex = this.getTargetIndex()

    if (targetIndex === this.notes.length - 1) {


should add && shiftKeyDown

voidsatisfaction · 2017-11-15T13:39:20Z

browser/main/NoteList/index.js


+  focusNote (selectedNoteKeys, noteKey) {
+    let { router } = this.context
+    let { location } = this.props


I strongly agree with your suggestion! I changed

voidsatisfaction · 2017-11-15T13:39:48Z

browser/main/NoteList/index.js

-    })
+    if (!shiftKeyDown) { selectedNoteKeys = [] }
+    const priorNote = Object.assign({}, this.notes[targetIndex])
+    const priorNoteKey = `${priorNote.storage}-${priorNote.key}`


I used that. Thanks.

voidsatisfaction · 2017-11-15T13:40:55Z

browser/main/NoteList/index.js

+    if (!shiftKeyDown) { selectedNoteKeys = [] }
+    const priorNote = Object.assign({}, this.notes[targetIndex])
+    const priorNoteKey = `${priorNote.storage}-${priorNote.key}`
+    if (selectedNoteKeys.includes(priorNoteKey)) {


@sota1235
No, It findNoteByKeyis only for find note not keys.

Currently I have state that only save selectedNoteKeys not notes

voidsatisfaction · 2017-11-15T13:41:19Z

browser/main/NoteList/index.js

-    if (targetIndex === this.notes.length - 1) {
+    if (targetIndex === this.notes.length - 1 && shiftKeyDown) {
+      return
+    } else if (targetIndex === this.notes.length - 1) {


I added variable. Thank you!

voidsatisfaction · 2017-11-15T14:00:01Z

browser/main/NoteList/index.js

-    })
+    const { selectedNoteKeys } = this.state
+    const note = findNoteByKey(this.notes, uniqueKey)
+    const noteKey = `${note.storage}-${note.key}`


Thank you for notice!

voidsatisfaction · 2017-11-15T14:09:57Z

browser/main/NoteList/index.js

+  deleteNote () {
+    const { dispatch } = this.props
+    const { selectedNoteKeys } = this.state
+    const notes = this.notes.map((note) => Object.assign({}, note))


@sota1235
If I simply put const notes = this.notes and then delete note, It will not update store because a note is object and note.isTrashed = true makes store's note value also change(mutable)

So, I made new note objects with same content not to influence store's note

voidsatisfaction · 2017-11-15T14:10:38Z

browser/main/NoteList/index.js

+  deleteNote () {
+    const { dispatch } = this.props
+    const { selectedNoteKeys } = this.state
+    const notes = this.notes.map((note) => Object.assign({}, note))


It is hard to explain but You will notice why I did this if you just try it :D

voidsatisfaction · 2017-11-15T14:12:12Z

browser/main/NoteList/index.js

+        })
+      })
+      .catch((err) => {
+        console.error('Cannot Delete note: ' + err)


Great suggestion! however, I think making new modal window task should be done with another branch because it only handles multi-selection and such things..

voidsatisfaction · 2017-11-15T14:14:59Z

browser/main/SideNav/StorageItem.js

+  dropNote (storage, folder, dispatch, location, noteData) {
+    noteData = noteData.filter((note) => folder.key !== note.folder)
+    const newNoteData = noteData.map((note) => Object.assign({}, note, {storage: storage, folder: folder.key}))
+    if (noteData.length === 0) return


Thank you for notice!

kazup01 · 2017-11-26T18:03:46Z

Hi @voidsatisfaction , I'm so sorry for our long absence. Could you fix conflicts?

voidsatisfaction · 2017-11-27T04:05:15Z

@kazup01 sure. I will do that.

voidsatisfaction · 2017-11-27T05:36:57Z

@kazup01 I fixed conflicts please check it :D

kazup01 · 2017-11-27T05:41:50Z

Thanks @voidsatisfaction ;)

sota1235

Thank you for fixing :)

kazup01

LGTM

kazup01 · 2017-11-28T03:41:21Z

Merged. Thanks @voidsatisfaction !
I will write this pull request to the v0.8.18 release note :)

voidsatisfaction · 2017-11-28T07:15:36Z

@kazup01 I'm really glad to hear that ^^

ghost · 2018-08-22T07:18:49Z

multiselect with shift works, but as soon as I try to drag I lose my selected notes.

voidsatisfaction added 5 commits November 5, 2017 18:11

feat: add shiftkey multiselect notes and delete

2d0e14c

fix up and down key rendering problems

a504a45

feat change name and add deletion logic

a7e458b

refactor: inner codes

70b69a3

feat multi drag and drop for changing folder

bcb1fb4

voidsatisfaction changed the title ~~Feature add multiselect notes then delete and move to another folder~~ Feature multiselect notes delete and move to another folder Nov 5, 2017

kazup01 requested a review from sota1235 November 5, 2017 15:06

feat: add multiple pin to top and rename activateNote to focusNote

9139495

voidsatisfaction changed the title ~~Feature multiselect notes delete and move to another folder~~ [WIP]Feature multiselect notes delete and move to another folder Nov 6, 2017

voidsatisfaction changed the title ~~[WIP]Feature multiselect notes delete and move to another folder~~ [WIP] Feature multiselect notes delete and move to another folder Nov 6, 2017

add multi selection with arrow short cut

9095fe9

kazup01 added the awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. label Nov 7, 2017

voidsatisfaction added 4 commits November 7, 2017 22:38

refactor: add utils

e57fef2

resolve conflict with master

c876306

Merge branch 'master' into feature/add_multiselect_notes_delete

f0f23ed

Merge branch 'master' into feature/add_multiselect_notes_delete

037ff2e

voidsatisfaction changed the title ~~[WIP] Feature multiselect notes delete and move to another folder~~ Feature multiselect notes delete and move to another folder Nov 7, 2017

fix: pintotop error

84f18ce

kazup01 added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Nov 7, 2017

sota1235 reviewed Nov 13, 2017

View reviewed changes

kazup01 added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Nov 14, 2017

refactor: add new function and fix problems from feedbacks

a36e779

kazup01 added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Nov 15, 2017

voidsatisfaction commented Nov 16, 2017

View reviewed changes

voidsatisfaction added 2 commits November 27, 2017 14:05

Merge with master resolve conflict

eee212f

fix: from let to const

c33f9d8

sota1235 approved these changes Nov 27, 2017

View reviewed changes

sota1235 requested a review from kazup01 November 27, 2017 15:12

kazup01 approved these changes Nov 28, 2017

View reviewed changes

kazup01 merged commit edfc8d9 into BoostIO:master Nov 28, 2017

BoostnoteBot added the Next Release label Nov 28, 2017

kazup01 added next release (v0.8.18) and removed awaiting review ❇️ Pull request is awaiting a review. Next Release labels Nov 28, 2017

voidsatisfaction deleted the feature/add_multiselect_notes_delete branch November 28, 2017 07:15

kohei-takata mentioned this pull request Dec 3, 2017

v0.8.18 #1230

Merged

kazup01 removed the next release (v0.8.18) label Dec 4, 2017


		let targetIndex = this.getTargetIndex()

		if (targetIndex === this.notes.length - 1) {

Uh oh!

Feature multiselect notes delete and move to another folder #1070

Feature multiselect notes delete and move to another folder #1070

Conversation

voidsatisfaction commented Nov 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY?

HOW?

Uh oh!

kazup01 commented Nov 5, 2017

Uh oh!

tatoosh commented Nov 6, 2017

Uh oh!

voidsatisfaction commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

voidsatisfaction commented Nov 7, 2017

Multi Pins to top

Multi Select with shift + arrow-down(up as well) + Multi Delete with shift + d

Uh oh!

voidsatisfaction commented Nov 7, 2017

Uh oh!

kazup01 commented Nov 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sota1235 commented Nov 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

voidsatisfaction commented Nov 13, 2017

Uh oh!

voidsatisfaction commented Nov 15, 2017

Uh oh!

kazup01 commented Nov 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kazup01 commented Nov 26, 2017

Uh oh!

voidsatisfaction commented Nov 27, 2017

Uh oh!

voidsatisfaction commented Nov 27, 2017

Uh oh!

kazup01 commented Nov 27, 2017

Uh oh!

sota1235 left a comment

voidsatisfaction commented Nov 5, 2017 •

edited

Loading

voidsatisfaction commented Nov 6, 2017 •

edited

Loading

Multi Select with `shift + arrow-down(up as well)` + Multi Delete with `shift + d`

sota1235 commented Nov 13, 2017 •

edited

Loading