-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement multi selection of datasets in folder tab #6683
Conversation
@daniel-wer If you can ignore 925a481 during review (e.g., by reviewing the commits before and after that commit separately), the diff should be easier to grasp 🙈 |
… in multi selection case
7a9ebab
to
db8f1b3
Compare
@daniel-wer The branch is now merge-conflict free and also includes the range selection via shift+click. Feel free to review and test now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, this will be very helpful to a lot of users!
Notes from my testing:
- The shift + left click selection behaves weirdly inside a folder (even after a reload). See this gif:
- Selecting datasets with ctrl + left click rather often does not work. The same sometimes happens even without the ctrl modifier. I click into the large white space in the tags column. Ah, I think the culprit might be this overly large div in the tags column (that I seem to hit pretty accurately 😅):
- When selecting multiple datasets, the context menu is a little misleading as it only executes the action for the dataset that was right-clicked on, although one could expect it to be executed for all selected datasets (e.g. "Reload"). This is also what happens in my file explorer when having selected multiple files. Maybe the context menu should be disabled for the multi-selection case.
@daniel-wer Thank you for your testing feedback 🙏 I incorporated everything now. Please have another look :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works very well! 👍
I found one more minor issue which could be fixed:
- When moving datasets into the same folder they already are in, the progress modal and success toast still appear. This should be ignored or one could show a warning toast. I'm leaning towards the warning toast to notify the user that the last action didn't make any sense and was probably unintended - but no hard opinion.
Done :) Will merge now unless you veto. |
🚢 it! |
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)