dock: Improvements to DockArea for nested Docks and styling

[landaire]

I'm stretching the "1 PR to solve 1 problem" request a bit, but unfortunately GitHub does not yet support stacked PRs across forks which would make trying to upstream these independently somewhat annoying.

Summary of changes: Adds some improvements to the DockArea to make managing docks, creating nested docks, and styling docks a bit easier.

Description

• fix(input): tolerate a window whose window_handle() panics: upstream gpui panics (unreachable!) when attempting to get a TestWindow handle. This catches the panic and gracefully recovers. I don't like the solution here, but it works for now. gpui-component does not hit this panic today because macos_accessibility is disabled in tests, but it may be required for some tests to do hit detection.
• feat(dock): expose remove_panel_id publicly on DockArea: makes DockArea::remove_panel_id public to support scenarios where you may have a panel ID but not a reference to the entity.
• fix(dock): ignore move_panel of a panel this area does not own: nested docks have a weird issue where trying to drag tabs within these nested panels could cause ghost tabs and some other strange things. This resolves that issue.
• feat(dock): expose per-node bounds for spatial overlays: during pre_paint, records leaf nodes render areas to allow for possibilities like e.g. vimimum style tab pickers (shown below).
• feat(dock): customizable tabs via render_tab, with a built-in close button: adds a PanelView::render_tab callback which allows for full customization of the Tab. Changes default behavior so that closeable tabs have a "Close Tab"-icon suffix button for closing.

Media

Screenshot of the tabs

Before	After

The DockArea::move_panel changes help allow this:

move_panels_compressed.mp4

And the overlay mentioned (in my application, but unblocked by these changes):

overlay_compressed.mp4

How to Test

Added new tests.

Checklist

• I have read the CONTRIBUTING document and followed the guidelines.
• Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
• Passed cargo run for story tests related to the changes.
• Tested macOS, Windows and Linux platforms performance (if the change is platform-specific)

[huacnlee]

Thanks for this PR! The tests are careful, and the move_panel ownership guard is a good catch.

I have one concern about the new close button.

In tab_panel.rs the button shows when panel.closable(cx) is true. But TabGroup::close_panel checks two more things before it closes anything:

if !self.constraints.is_closable() { return; }
if !self.draggable(cx) { return; }   // = !is_locked() && !is_last_panel()

So the X button can be drawn but do nothing when:

• the layout is locked, or
• the group is alone and has only one panel — a common start state for many apps.

This is the same trap the tab_drag comment just above already warns about:

a tab bar that forgets to ask TabGroupContext::is_draggable makes a group that has nowhere to go — a dock's last group, a locked dock — draggable anyway.

The smallest fix is to ask group.is_draggable() too. It covers both cases:

.when(group.is_draggable() && panel.closable(cx), |this| { ... })

The current tests do not catch this. drew_close_button builds a group with two panels, so is_last_panel is false, and no test uses a locked layout.

One note if you also want to cover the constraints.is_closable() gate: TabGroupContext::is_closable() is not the right call. It means "the active panel is closable", so it would hide the X on the other tabs. Base would need to expose that bit on its own.

[huacnlee]

Thanks for putting this together. I found two issues that should be addressed before merging:

1. DockArea::node_bounds can return stale bounds for removed nodes. Its public documentation says it returns None when a node is not a rendered tab group, but entries are never removed from node_bounds. After a leaf is removed, a caller retaining its NodeId can still receive an obsolete on-screen rectangle. Please clear stale entries or otherwise make the implementation match the documented contract, and add a regression test covering node removal.

2. The accessibility workaround catches panics around the entire ns_view(window) call. This can silently hide unrelated failures in handle matching or pointer conversion. Please narrow the catch_unwind scope to HasWindowHandle::window_handle, as the native input implementation does. A regression test for the expected test-window panic path would also be valuable.

There is also a testing gap in the new close button behavior: the current tests verify only that the button is rendered. Please add an interaction test that clicks the close button on a non-active tab and verifies that the correct PanelId is removed without selecting or otherwise triggering the tab click handler.

The targeted tests currently included in this PR pass locally.

[huacnlee]

Thanks for the thorough writeup and especially for the tests — the node_bounds probe-ordering regression guard is exactly the kind of test that pays for itself, and the comment explaining why the on_prepaint canvas must precede the content child is correct (it inserts an .absolute().size_full() canvas, so its static position depends on where it sits in the flow).

I verified the branch builds, cargo clippy --all-targets --deny warnings is clean, and the dock suites pass (179 in gpui-base, 23 in gpui-component). Findings below, most severe first.

Blocking: the branch is not rustfmt-clean

cargo fmt --check fails on this PR's own new code, so CI will reject it:

crates/base/src/dock/dock_area.rs:2503
crates/ui/src/dock/dock.rs:393
crates/ui/src/dock/dock.rs:416

node_bounds returns stale rects for nodes that stopped being painted

node_bounds documents "None if the node is not a rendered tab group", but reconcile prunes only nodes removed from the tree. A node can stop being painted while staying in the tree, and then the map keeps the rectangle it last had:

• Zoom. Render for DockArea matches on zoomed_view() and, when it is Some, renders frame.child(view) and never calls render_node at all. No probe fires for any node, so every entry goes stale at once.
• An all-invisible slot. ResizablePanel::render returns a bare div() and drops its children, the probe with them.
• A closed dock.

Confirmed on this branch: an 800x600 window with an h-split of two leaves, then set_zoomed_in(left_id) and two full redraws. node_bounds(left_id) still reports 400x600 @ (0,0) for a pane that is now 800x600, and node_bounds(right_id) still reports 400x600 @ (400,0) for a pane that is not on screen. The stated use case is a vimium-style pane picker, so this badges a hidden pane and mis-places the badge on the zoomed one — the exact failure the feature is meant to avoid.

Simplest fix: record a frame counter alongside each rect and treat entries older than the current frame as None. Otherwise the zoomed view needs the same probe and invisible slots need explicit eviction.

The close button's gate is not the gate that closes

The comment says is_draggable is "the same gate TabGroup::close_panel checks". It is not. close_panel requires three things:

constraints.is_closable() && self.draggable(cx) && panel.closable(cx)

The new gate is group.is_draggable() && panel.closable(cx) and drops the constraint term. TabGroupConstraints::closable(false) is a public builder whose own doc says "a dock's last group sets this false so the dock cannot be emptied", so any container using it gets an X on every tab that does nothing when clicked. Today DockArea always builds in_split(alone) (closable true) and never calls .closable(false), so this is latent rather than live — but the comment asserts an equivalence that does not hold, and the same file already gates the ellipsis-menu Close entry on group.is_closable() (tab_panel.rs:300), so the two Close affordances can disagree.

TabGroupContext exposes no per-panel-correct reader for this today, so the fix likely needs a new accessor rather than a one-line change.

Close buttons are drawn on a collapsed group's tab strip

render_tab_bar routes on visible-panel count only, so a collapsed group with two or more visible panels lands in render_tabs and now grows close buttons. The surrounding code deliberately gates every rearrange interaction behind .when(!collapsed, …), on the stated reasoning that "a collapsed group is a strip of tabs with no content: the strip is a way back in, not a selection". close_panel does not check collapsed either (collapsed does not imply locked), so a user reaching for a collapsed bottom dock can hit the X and destroy the panel instead of reopening it. The gate needs !collapsed.

The catch_unwind probe pays its cost every frame

sync_native_content_type is called from Input::render whenever the input is focused, so on a macOS test window this is a panic, a full unwind, and a default-panic-hook message on every rendered frame — not once. That is slow and it floods the very test output the change is meant to make usable.

The clean fix is upstream and one line: TestWindow::window_handle should return Err(HandleError::NotSupported) instead of unimplemented!(). raw-window-handle has that variant for exactly this case ("the underlying window system does not support any of the representative C window handles"), and the existing .ok()? then works with zero cost and no catch_unwind. Worth sending to Zed; it is a smaller change than this workaround.

If the workaround has to stay in the meantime, cache the answer per window (a thread-local keyed by window id, or a Once) so the panic is paid at most once, and log when the probe catches. As written it also silently swallows a genuine panic on a real window, turning a crash into a quiet loss of accessibility wiring and native autofill with no diagnostic.

Smaller points

• No ## Breaking Changes section. PanelView is a public trait and gains render_tab with no default, which breaks any downstream impl PanelView. CLAUDE.md requires that section with diff blocks when the public API of the component crate changes. The default tab appearance also changes for every existing consumer (tab widths and hit targets move), which belongs in the same section.
• Panel::render_tab takes &mut self while its siblings dropdown_menu, zoom_control and inner_padding all take &self, and the object-safe PanelView::render_tab takes &self but internally calls self.update(...). That puts a mutable borrow on the panel entity during the tab bar's render. The documented purpose is final styling tweaks, so &self would do and would remove a re-entrancy hazard for custom implementations.
• The close button has no tooltip and no keyboard path. The adjacent zoom control uses .tooltip_with_action(...), and the Dock.Close i18n key already exists and is translated. An unlabeled icon-only destructive control sitting next to a labeled one is inconsistent, and .tab_stop(false) makes it keyboard-unreachable.
• Scope. You flagged this yourself, and it is a real cost here: the catch_unwind change and the tab redesign have nothing to do with each other, and the tab redesign is the one that needs a design decision from the maintainers rather than a code review. Splitting it out would let the other four land quickly.

Reviewed and found sound

• The move_panel ownership guard. self.panels is the authoritative registration map, populated by add_panel and pruned by reconcile, so the guard cannot reject a legitimately owned panel, and the only caller path targets exactly the cross-area case you describe.
• remove_panel_id made public. It is already what remove_panel delegates to, and it early-returns when the id is not found.
• The on_prepaint wrapper's layout impact. TabGroupSkin::frame is already v_flex().size_full(), so the extra div().size_full() is layout-neutral, and the split-filling tests still pass.
• stop_propagation on the close click correctly prevents the tab from also activating.

---

🤖 Review assisted by Claude Code