feat(worksheets): select several rows and drag them together - #181
Merged
Conversation
The sidebar could only ever move one worksheet at a time. Command- or control-click picks rows out, shift-click takes a range from the anchor, and grabbing any selected row now carries the whole group to the drop point — gathering a non-contiguous selection into a block that keeps its list order. With nothing picked out the open worksheet stands in for the selection, so the first modified click adds to the row you were on rather than silently dropping it. Modified clicks deliberately leave the editor alone: picking rows out to move them should not swap what you are looking at. The context menu follows the selection — "Delete 3 worksheets" behind one confirmation, Rename gone while it means nothing, and the same never-empty-the-list guard as a single row. A delete that only half works says how many of them failed instead of stranding the rest. `moveIds` and the selection math are pure and tested on their own; `useListReorder` takes the selection as an option, so the tab strip and the database explorer keep their single-row drag untouched.
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The sidebar could only ever move one worksheet at a time. Now several rows can be picked out and dragged as a group.
What you can do
Failed to delete 1 of 2 worksheetsrather than stranding the rest.A plain click still just opens one worksheet, and typing in the filter clears the selection — a filtered list cannot be dragged, and hidden rows should not be deletable.
Two decisions worth reviewing
The open worksheet is the implicit selection. With nothing picked out, a ⌘-click adds to the row you were already on rather than replacing it, so the first modified click cannot silently drop it. That fallback depends on view state the reducer does not have, so the slice is a holder (
worksheetSelectionChanged) and the math runs at the call site — the same shapeworksheetRenameStartedalready uses.Modified clicks do not open anything. Picking rows out to move them should not swap the editor under you.
Shape
src/app/list-move.ts—moveIds, the block move. Returns the list it was given, by reference, for every drop that means nothing (row gone mid-drag, drop inside the moving block), which is how the hook knows to skip the write.src/app/list-selection.ts— toggle / extend / prune, all pure.src/app/hooks/use-list-reorder.ts— takesselectedIdsas an option and addsisMoving. Single-row drags route through the samemoveIds, so the tab strip and the database explorer are untouched.src/app/hooks/use-worksheet-selection.ts— prunes rows that have gone and applies the open-worksheet fallback.Verification
Unit tests for both pure modules, multi-drag and
isMovingcases inuse-list-reorder.test.ts, and explorer tests for ⌘-click, Ctrl-click, shift-click, plain click, multi-delete, the all-selected guard, and the partial-failure message. Full suite green (1414 tests), typecheck and prettier clean.Also driven against the running app, since jsdom cannot do a real drag: ⌘-clicked three non-contiguous rows, dragged them to the top of the list, and confirmed
PUT /worksheets/orderwrote them as a contiguous block in list order; shift-click ranges and the "Delete 3 worksheets" menu both behave in a real browser, where Radix menus have misled this project's jsdom tests before.Deliberately not included
data-selectedfor styling and tests; making the list a reallistboxwould imply arrow-key navigation it does not have.