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

@@ -124,27 +144,49 @@ class NoteList extends React.Component {
    }
  }

+  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

@@ -176,22 +182,30 @@ class NoteList extends React.Component {
    }
    let { router } = this.context
    let { location } = this.props
-    let { selectedNoteKeys } = this.state
+    let { selectedNoteKeys, shiftKeyDown } = this.state

    let targetIndex = this.getTargetIndex()

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


should add && shiftKeyDown

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

browser/main/NoteList/index.js

@@ -124,27 +144,49 @@ class NoteList extends React.Component {
    }
  }

+  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

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

WHY?

HOW?

kazup01 commented Nov 5, 2017

tatoosh commented Nov 6, 2017

voidsatisfaction commented Nov 6, 2017 • edited

voidsatisfaction commented Nov 7, 2017

Multi Pins to top

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

voidsatisfaction commented Nov 7, 2017

kazup01 commented Nov 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sota1235 commented Nov 13, 2017 • edited

voidsatisfaction commented Nov 13, 2017

voidsatisfaction commented Nov 15, 2017

kazup01 commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazup01 commented Nov 26, 2017

voidsatisfaction commented Nov 27, 2017

voidsatisfaction commented Nov 27, 2017

kazup01 commented Nov 27, 2017

sota1235 left a comment

Choose a reason for hiding this comment

kazup01 left a comment

Choose a reason for hiding this comment

kazup01 commented Nov 28, 2017

voidsatisfaction commented Nov 28, 2017

ghost commented Aug 22, 2018

voidsatisfaction commented Nov 5, 2017 •

edited

voidsatisfaction commented Nov 6, 2017 •

edited

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

sota1235 commented Nov 13, 2017 •

edited