dock: fix nested moves and add opt-in tab close controls - #3197
Merged
Merged
Conversation
gpui's TestWindow panics (unimplemented!) in its HasWindowHandle impl instead of returning Err, so the graceful `.ok()?` bail in the input and accessibility ns_view helpers never fires -- a downstream crate's tests that build a Root or render an Input over a test window panic. Probe the handle under catch_unwind and no-op when there is no backing AppKit window.
Lets a consumer close a panel it only holds a PanelId for (e.g. an unresolved/InvalidPanel leaf from a restored layout), where remove_panel would need a live Entity<P>.
A cross-DockArea drag -- dragging a tab between an outer dock and a nested dock hosted inside one of its own panels -- would insert a PanelId with no backing entity into the target tree while the panel stayed registered in its real owner: a ghost tab in one, a duplicate in the other. Guard move_panel to no-op when the panel is not owned here, so the panel stays in its source. Covered by a_move_of_an_unowned_panel_is_ignored.
Record each tab-group leaf's on-screen rect during render (via on_prepaint) into a node_bounds map, exposed as DockArea::node_bounds. Lets a host paint spatial overlays over panes -- e.g. a vimium-style pane picker badging each pane -- which the pure-data tree cannot express. The wrapper that captures the rect carries no sizing (that stays on the parent resizable_panel), so split layout is unchanged.
…utton Panels can override Panel::render_tab to have the final say over their own tab -- restyle it, swap the label, add a prefix icon, or add/drop a suffix -- while the tab bar keeps its layout, drag/drop, and activation. The tab group builds the fully wired Tab and routes it through the panel; the default returns it unchanged. Closable panels get a built-in close (X) button suffix. It stops click propagation so closing never also selects the tab, and closes by panel id so it targets its own tab regardless of which is active.
…a11y scope - Prune node_bounds on reconcile so a removed leaf's NodeId reports None instead of a stale rect, matching the documented contract - Gate the per-tab close button on group.is_draggable(), the same check TabGroup::close_panel runs, so it never draws when a click could not close the panel (locked layout or a group's last visible panel) - Narrow the macOS accessibility catch_unwind to window_handle(), as the native Input does, so failures in handle matching or pointer conversion still surface - Add tests covering node removal and a non-active tab close-button click
- Gate the per-tab close button on the full close_panel condition: add TabGroupContext::is_close_permitted (the container-level closable bit) and require !collapsed, so the X never draws on a locked or last-panel group, nor on a collapsed dock strip where a click would destroy a panel instead of reopening the dock. Correct the comment that wrongly called is_draggable the same gate close_panel checks. - Make Panel::render_tab take &self/&App instead of &mut self, removing the mutable panel-entity borrow held across the tab bar's render. - Give the close button a Dock.Close tooltip. - Cache the macOS window-handle probe per window and warn once on a panic, so ns_view no longer pays a catch_unwind plus unwind on every focused frame over a test window, and a genuine failure on a real window is not silently swallowed. - Test the collapsed-group close-button gate.
3 of 4 tasks
huacnlee
changed the title
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
2 participants
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.
Supersedes #2840 for review. This branch builds on @landaire's original PR and contains the Dock follow-up fixes. The original PR's source fork does not allow maintainer edits, so the final code is pushed from
longbridge/gpui-kit.Summary
move_panelrequest when this dock does not own the panel, preventing duplicate or ghost tabs in nested docks.TabGroup::close_panel. The Dock Story exposes it under Options, off by default; Editor demonstrates a non-closable panel.TabGroupRenderercan build tabs and capture group bounds inframewithon_prepaintandgroup.node(). The dock no longer stores bounds onDockArea.Public interface
TabGroupContext::is_panel_closable(panel, cx)answers whether a specific tab can close, without exposing the container's internal close flag.DockSkin::set_close_button_visible(visible, cx)shows the built-in tab close controls; they remain hidden by default.Verification
cargo test -p gpui-base -p gpui-component dock --libcargo fmt --all -- --checkcargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets -- --deny warnings