Implement dragging actions for vertical tab groups#12000
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds vertical tab group drag-and-drop support plus nested-draggable deferral plumbing. I found two blocking correctness issues in the tab/group drag behavior. No security-specific findings were identified, and the provided spec context did not include approved requirements to compare against.
Concerns
- Dragging a tab into a non-adjacent expanded group only flips
group_idand returns, while the renderer only groups consecutive tabs. This can split one logical group into multiple rendered containers and leave later group-block operations working on an invalid range. - Skipping the per-tab draggable for a sole group member means the only tab in a group cannot be dragged out to dissolve/prune that group.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
Can we verify what the behavior is when cross-window tab dragging is enabled? Two things I wanna test:
|
|
@peicodes To answer your questions:
I don't think any of these are blocking, IMO. I want to land this by eod so it can be on dogfood tmw for feedback. I will address these bugs as fast follows. Loom for reference: https://www.loom.com/share/ccb7253b50c74e2f95a7b9f8ccbfef09 |
f564a5f to
b290b3f
Compare
22bae9e to
2672d09
Compare
2672d09 to
03eeb47
Compare
Description
Adds drag-and-drop reordering for vertical tab groups. Users can drag a group to reorder it as a block. Single tabs can also be dragged into and out of an expanded group.
Changes
New actions (
app/src/workspace/action.rs):StartGroupDrag,DragGroup,DropGroupGroup drag handler (
app/src/workspace/view.rs):on_group_drag— block reorder viamove_group_blockwhen the dragged header's midpoint crosses an above/below neighbor's midpointtarget_group_at_y— hit-tests group container rects (with a 6px edge margin so the ungrouped zone between adjacent groups is reachable)neighbor_drag_rect— substitutes the group container's rect for stale per-tab rects when a neighbor lives inside a collapsed group.assign_tab_to_group— flips tabs[i].group_id for cross-group transitions during a tab dragNested-Draggable support (
crates/warpui_core/src/elements/drag/draggable.rs,crates/warpui_core/src/presenter.rs):Draggable::with_defer_to_handled_child_mouse_down()builder. When the flag is set, the Draggable yields its mouse-down to any descendant Draggable that already claimed the event.EventContext::descendant_draggable_initiated flag— set unconditionally when a Draggable claims a mouse-down, read only by Draggables that opt into deferral.Linked Issue
https://linear.app/warpdotdev/issue/APP-4619/vertical-tab-group-dragging-support
Testing
./script/runScreenshots / Videos
Demo video