Skip to content

feat(dock): make tab bar size configurable - #2972

Open
hipstersmoothie wants to merge 2 commits into
longbridge:mainfrom
hipstersmoothie:feat/configurable-dock-tab-size
Open

feat(dock): make tab bar size configurable#2972
hipstersmoothie wants to merge 2 commits into
longbridge:mainfrom
hipstersmoothie:feat/configurable-dock-tab-size

Conversation

@hipstersmoothie

@hipstersmoothie hipstersmoothie commented Sep 5, 2026

Copy link
Copy Markdown

NOTE: This was completely written by AI but the issue is real. I was not able to find a way to set the height of the tab bar in any way.

Summary

  • add DockSkin::tab_size and DockSkin::set_tab_size so applications can size dock-owned tab bars
  • honor custom pixel sizes in Tab, including the outer height, inner surface, and proportional horizontal spacing
  • add a rendered dock test proving a taller tab bar leaves correspondingly less panel content

Motivation

TabBar implements Sizable, but Size::Size(Pixels) currently falls through to the medium tab metrics. In addition, applications cannot pass a size to the TabBar constructed internally by DockSkin. This makes dock tabs clip when an application increases its global text scale.

The default remains Size::Medium, so existing docks are unchanged. Applications that need larger UI chrome can call:

skin.set_tab_size(px(40.), cx);

Testing

  • cargo test -p gpui-component — 421 unit tests and all compatibility suites pass
  • cargo fmt --all -- --check — passes
  • cargo clippy -p gpui-component --all-targets — no diagnostics in changed files
  • cargo clippy ... -- -D warnings is currently blocked by two pre-existing clippy::nonminimal_bool diagnostics in crates/base/src/calendar.rs:131 under Rust 1.95

AI assistance

AI was used to inspect the component and dock rendering paths, draft the implementation and tests, and prepare this PR description. I reviewed the resulting API and diff and ran the test and formatting checks listed above.

TabBar already followed DockSkin::tab_size; the auto-style title used
when a group has one visible panel stayed hardcoded at 30px.
@huacnlee

huacnlee commented Sep 6, 2026

Copy link
Copy Markdown
Member

NOTE: This was completely written by AI but the issue is real. I was not able to find a way to set the height of the tab bar in any way.

Don't worry about this, we have totally accept AI written codes, even 100%.

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the report — the underlying problem is real, dock tab bars genuinely have no height escape hatch today. But I don't want to merge this API shape, and I think the diagnosis stops one level short of the actual cause.

1. tab_size does not name a size, it names a height

Size::Size(Pixels) has no single meaning in this library. Depending on the component it is:

  • a square edge — StyleSized::size_with.size(px), Icon, Avatar, OtpInput, Button's icon box
  • a widthNumberInputthis.min_w(size) (crates/component/src/input/number_input.rs:150)
  • a row height — Size::table_row_height
  • a font size — input_text_sizesize * 0.875

This PR adds a fifth meaning ("dock tab outer height") behind a public setter called set_tab_size, and the only way a caller learns which one they got is by reading the impl. If the dock needs to size its chrome, the API should name the thing it sizes: tab_bar_height() -> Pixels / set_tab_bar_height(Pixels). A Pixels in, a Pixels out, no enum to disambiguate.

2. The value silently becomes widths

The number passed as a height is then used for horizontal spacing in four places:

  • crates/component/src/tab/tab.rs:90inner_paddings: height * 0.375
  • crates/component/src/tab/tab_bar.rs:352default_gap: height * 0.375
  • crates/component/src/tab/tab_bar.rs:372 — segmented padding_x: height * 0.125
  • crates/component/src/tab/tab_bar.rs:388 — underline gap: height * 0.5

So "tab size" is a height in one place and a width in four, and the ratios do not reproduce the scale they claim to generalize. Checking them against the built-in steps:

ratio gives actual
default_gap @ Large (36) 13.5 16
inner_paddings @ Small (24) 9 10
inner_paddings @ Large (36) 13.5 16
underline gap @ Medium (36) 18 16
underline gap @ Small (30) 15 12
underline gap @ Large (44) 22 20

Size::Size(px(36.)) is therefore not Size::Large, and only Size::Size(px(32.)) happens to land on Size::Medium for the default variant. A custom value lands between the named steps by a rule no caller can predict, so these constants are unreviewable — there is nothing to check them against.

3. The pass is partial, which confirms it is a height and not a size

Size::Size reaches the outer height and the horizontal padding, but falls through to the Medium arm for inner_margins (tab.rs:108), radius / inner_radius / tab_bar_radius (tab.rs:351-386), the label font (_ => this.text_sm()), and the icon (_ => this.size_4()). A 44px tab is a medium tab with air around it. That may well be the right behaviour — but then the parameter is a height, and calling it a size promises scaling it does not deliver.

4. The motivation points at a different bug

This makes dock tabs clip when an application increases its global text scale.

Labels are rem-based (text_smrems(0.875)), tab heights are literal px(). That mismatch is the bug: the box does not follow the text. The library already solves exactly this in Icon, which derives its size from window.rem_size() (crates/component/src/icon.rs:150).

Making the dock chrome's fixed heights rem-derived fixes the clipping for every application without new public API, and without asking apps to recompute a pixel height each time they change scale — which is what this PR leaves them doing. I'd rather see that change. If an explicit per-app override is still wanted afterwards, it can be added on top, as set_tab_bar_height(Pixels).

5. A third height table in render_title

crates/component/src/dock/tab_panel.rs:373-378 introduces its own 20/24/36/30 mapping, while TabVariant::height maps 20/24/36/32. Medium disagrees, so set_tab_size(px(32.)) — the value that equals today's default tab bar — silently makes the single-panel title bar 2px taller than leaving the default alone. Whatever the final shape, this height needs to come from one place.

6. A closed bottom dock still clips

crates/base/src/dock/dock_area.rs:1932:

/// A closed bottom dock keeps this much, so its tab bar stays clickable.
pub const CLOSED_BOTTOM_STRIP: Pixels = px(29.);

With set_tab_size(px(44.)) that strip cuts off the tab bar it exists to preserve. Any height knob has to reach this constant too — another argument for fixing the scaling at the source rather than threading a value through the skin.

7. Docs and story

DockSkin's settings are documented in website/docs/dock.md and website/zh-CN/docs/dock.md; new public API has to land in both locales. There is also no story coverage for the new setting.

Minor

tab_size_changes_the_height_left_for_panel_content asserts medium_content - custom_content == px(12.), which encodes "the default bar is 32px" as an unexplained 12. Assert the two heights instead so a change to the default fails with a readable message.


Requesting changes. Happy to take a PR that makes the dock's fixed chrome heights follow rem_size; if we still want an explicit override after that, let's land it as set_tab_bar_height(Pixels) rather than overloading Size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants