fix(NFilePicker): Added multiSelection choice#2253
Conversation
Ly-sec
left a comment
Ly-sec
left a comment
There was a problem hiding this comment.
Overall it's quite good @spiros132 but as you said, maybe change it from Shift to Control given it makes more sense in terms of UX. If it's not too hard we should perhaps implement the Shift selection too - up to you though.
| filePickerPanel.currentSelection = [model.filePath]; | ||
| // Deep copy otherwise the isSelected might not get updated. | ||
| if (root.allowMultiSelection && (mouse.modifiers & Qt.ShiftModifier)) { | ||
| var list = JSON.parse(JSON.stringify(filePickerPanel.currentSelection)); |
There was a problem hiding this comment.
We think it would be better to replace this with something like var list = filePickerPanel.currentSelection.slice() for performance reasons, less json, less computing and all that.
There was a problem hiding this comment.
Thank you, I didn't know that was an option!
| filePickerPanel.currentSelection = [model.filePath]; | ||
| if (root.allowMultiSelection && (mouse.modifiers & Qt.ShiftModifier)) { | ||
| // Deep copy otherwise the isSelected might not get updated. | ||
| var list = JSON.parse(JSON.stringify(filePickerPanel.currentSelection)); |
…th having different functionality
|
@Ly-sec I've reworked it a bit to make it easier to read, reduce the repetitive code and added so that shift can select multiple files at the same time. |
|
Awesome @spiros132 this is much nicer now. Going to merge it, thank you very much! |
2 participants
Pull Request
Added the choice to have a
NFilePickerwhere you can choose multiple items at the same timeMotivation
The
NFilePickercomponent currently only supports choosing one item but I believe it would be good for the developers to have the choice for multiple items to be chosen at the same time. The boolean to enable multiple choices at the same time is false by default since that's how it has been working so far. Right now you can select multiple items with the Shift button.Type of Change
Mark the relevant option with an "x".
Related Issue
No related issue found.
Testing
The testing was done by using it, opening multiple file pickers and seeing that it works.
Screenshots / Videos
A screenshot showing multiple folders being selected at the same time.
Checklist
Additional Notes
This PR only adds support for selecting multiple files / folders with holding the shift button. Some people are used to having the shift button select from the first item in a list to the last item in the list, so in that case maybe ctrl might have been better but I'm not really sure tbh.
Also the code is very repetitive, in that case I can refactor it to put everything under one function instead to make it less repetitive if that's wished for.