From 7ee0c47c6d2e828dc3314b7a45e6766cb50e3ddf Mon Sep 17 00:00:00 2001 From: kirillDevPro <113171057+kirillDevPro@users.noreply.github.com> Date: Sun, 6 Sep 2026 17:29:25 +0200 Subject: [PATCH] fix(chrome): lift tab strips, table headers and the scope chip one contrast step Chart tab strips, every MoonDataTable header and the pinned-scope chip draw their labels in text_muted, which sits at 3.5-3.8:1 against the panel in both themes, under the 4.5:1 floor at the user's font size. Add one design::chrome_label_color helper resolving to text_soft (6.7-7.0:1 light, 5.1-5.5:1 dark) and apply it at six tab strips, all nine table construction sites and the chip; three tab hosts had to learn (window, cx) to reach the themed render path. The dock tab strip itself is drawn inside MoonUI with no styling knob and is left as a MoonUI request. Claude-Session: https://claude.ai/code/session_01RaQ2tnAyr4pyiAiwGktPBv --- .../moon-ui-gpui/src/chart_tabs/main_stack.rs | 11 +- crates/moon-ui-gpui/src/chart_tabs/mod.rs | 2 +- crates/moon-ui-gpui/src/chart_tabs/strip.rs | 5 + crates/moon-ui-gpui/src/core_expert/render.rs | 140 ++++++++++++------ crates/moon-ui-gpui/src/design.rs | 129 +++++++++++++++- .../moon-ui-gpui/src/panels/alerts/table.rs | 1 + .../moon-ui-gpui/src/panels/assets/table.rs | 1 + crates/moon-ui-gpui/src/panels/common.rs | 2 +- .../src/panels/core_status/problems.rs | 3 +- .../src/panels/core_status/table.rs | 1 + .../src/panels/core_status/updates_list.rs | 3 +- .../src/panels/core_status/warnings.rs | 3 +- .../moon-ui-gpui/src/panels/orders/table.rs | 1 + .../moon-ui-gpui/src/panels/report/render.rs | 1 + crates/moon-ui-gpui/src/screener/view.rs | 1 + .../moon-ui-gpui/src/settings/hotkeys/tab.rs | 54 ++++--- crates/moon-ui-gpui/src/settings/render.rs | 2 +- .../moon-ui-gpui/src/shell/core_settings.rs | 10 ++ .../src/shell/core_settings_popup.rs | 71 +++++---- .../tests/theme_contract/dock_chrome.rs | 102 +++++++++++++ 20 files changed, 446 insertions(+), 97 deletions(-) diff --git a/crates/moon-ui-gpui/src/chart_tabs/main_stack.rs b/crates/moon-ui-gpui/src/chart_tabs/main_stack.rs index 5eefa061..bf1d1599 100644 --- a/crates/moon-ui-gpui/src/chart_tabs/main_stack.rs +++ b/crates/moon-ui-gpui/src/chart_tabs/main_stack.rs @@ -1512,12 +1512,13 @@ impl MainChartStack { /// back to the raw market key rather than rendering blank. /// /// Args: - /// _window: Unused; the strip is now in-flow and sizes itself. + /// window: Used only to render the strip through its own palette and scale tokens; the + /// strip is in-flow and still sizes itself. /// cx: Stack context, used to read each panel and to build the click handlers. /// /// Returns: /// The row, or `None` when fewer than two charts are open. - fn render_tab_row(&self, _window: &mut Window, cx: &mut Context) -> Option { + fn render_tab_row(&self, window: &mut Window, cx: &mut Context) -> Option { // Vacated slots are retained placeholders in COMPRESS layout — they hold a position, not a // chart, so they get no tab. let live: Vec<(CoreId, String, SharedString)> = self @@ -1591,6 +1592,12 @@ impl MainChartStack { }); } }); + // Same treatment as the Main/Add strip above it: MoonUI keys an inactive tab label off + // `text_muted` and offers no per-tab colour prop, so the lift arrives as a palette. + // `render_with_theme`, never `render_with_palette` — the latter substitutes default tokens + // and would drop the user's font delta, shrinking these labels away from `strip_h`. + let strip_palette = moon_ui::MoonPalette::active(cx); + let strip = crate::design::chrome_tab_strip(strip, strip_palette, window, cx); Some( div() .id("main-chart-tab-row") diff --git a/crates/moon-ui-gpui/src/chart_tabs/mod.rs b/crates/moon-ui-gpui/src/chart_tabs/mod.rs index 9582e928..9796eb0d 100644 --- a/crates/moon-ui-gpui/src/chart_tabs/mod.rs +++ b/crates/moon-ui-gpui/src/chart_tabs/mod.rs @@ -245,7 +245,7 @@ fn preferred_auto_workspace_market( /// font scaling cannot desynchronize the strip and underline from the tabs. With the default /// `ui = 1` and `font_delta = 2`, it returns the former constant value of 30. pub(super) fn chart_tab_strip_h(cx: &App) -> f32 { - crate::design::fit_h_value(cx, 28.0, 13.0, 7.5) + crate::design::tab_strip_h_value(cx) } /// Identity of a chart tab, ported from egui's `ContainerKind`. diff --git a/crates/moon-ui-gpui/src/chart_tabs/strip.rs b/crates/moon-ui-gpui/src/chart_tabs/strip.rs index 751251df..e6657742 100644 --- a/crates/moon-ui-gpui/src/chart_tabs/strip.rs +++ b/crates/moon-ui-gpui/src/chart_tabs/strip.rs @@ -192,6 +192,11 @@ impl Render for ChartTabs { // The active tab's layout control and adjacent scale dropdown are both per-tab. let popup_open = self.popup_shows(ChartPopup::Layout); let p_strip = MoonPalette::active(cx); + // MoonUI keys an inactive tab label off `text_muted` and exposes no per-tab colour prop, + // so the lift is handed in as a palette. `render_with_theme`, never `render_with_palette`: + // the latter substitutes default tokens and would silently drop the user's font delta and + // UI scale, shrinking the labels and pulling the strip off `chart_tab_strip_h`. + let strip = design::chrome_tab_strip(strip, p_strip, window, cx); let scale_dropdown = crate::controls::scale_dropdown_for_tabs( cx, self.active_scale_value(cx), diff --git a/crates/moon-ui-gpui/src/core_expert/render.rs b/crates/moon-ui-gpui/src/core_expert/render.rs index fd7cc92d..c8a14879 100644 --- a/crates/moon-ui-gpui/src/core_expert/render.rs +++ b/crates/moon-ui-gpui/src/core_expert/render.rs @@ -34,6 +34,10 @@ impl Render for CoreExpertView { // built below in this same frame. self.build_editors(window, cx); let p = MoonPalette::active(cx); + // Built before the tree: it needs `window` and `&mut cx`, which the builder chain below + // cannot lend it while it is also reading `cx` for its own scaled metrics. + let tab_strip = self.tab_strip(window, cx); + let body = self.body(p, window, cx).into_any_element(); let chrome_width = crate::window::windowing::responsive_width(window); v_flex() .size_full() @@ -50,8 +54,8 @@ impl Render for CoreExpertView { .track_focus(&self.focus) .child(title_bar(p, cx)) .child(self.switch_row(p, cx)) - .child(self.tab_strip(cx)) - .child(self.body(p, cx)) + .child(tab_strip) + .child(body) .child(self.footer(p, cx)) .child( MoonWindowFrame::tool("core-expert-frame-hit", chrome_width) @@ -137,30 +141,85 @@ impl CoreExpertView { } /// Moonbot's tab strip, in Moonbot's order. - fn tab_strip(&self, cx: &Context) -> impl IntoElement { + /// + /// Takes `window` because the strip is rendered through a lifted palette rather than the + /// active one: MoonUI keys an inactive tab label off `text_muted`, which sits under the body + /// contrast floor in both stock themes, and `render_with_theme` is the only way to hand it a + /// different palette. `AnyElement` for the same reason the chart strip boxes its own — the + /// returned element would otherwise hold the `&mut cx` borrow the caller still needs. + /// + /// Args: + /// window: Window that owns the strip's persistent overflow state. + /// cx: View context used to read the selected tab and render the themed strip. + /// + /// Returns: + /// The expert-tab strip in its fixed-height wrapper. + fn tab_strip(&self, window: &mut Window, cx: &mut Context) -> AnyElement { let view = cx.entity(); let selected = self.tab; let items: Vec = ExpertTab::ALL .iter() .map(|tab| MoonTabItem::new(tab.title()).selected(*tab == selected)) .collect(); + let strip_h = design::tab_strip_h(cx); + let p = MoonPalette::active(cx); + let strip = MoonTabStrip::new("core-expert-tabs") + .gap(4.0) + .overflow_menu(true) + .items(items) + .on_click(move |ix, _event, _window, app| { + let Some(next) = ExpertTab::at(ix) else { + return; + }; + view.update(app, |this, cx| this.set_tab(next, cx)); + }); + let strip = design::chrome_tab_strip(strip, p, window, cx); div() .w_full() .flex_none() - .h(design::fit_h_px(cx, 28.0, 13.0, 7.5)) - .child( - MoonTabStrip::new("core-expert-tabs") - .gap(4.0) - .overflow_menu(true) - .items(items) - .on_click(move |ix, _event, _window, app| { - let Some(next) = ExpertTab::at(ix) else { - return; - }; - view.update(app, |this, cx| this.set_tab(next, cx)); - }) - .render(), - ) + .h(strip_h) + .child(strip) + .into_any_element() + } + + /// The Hotkeys page's own sub-tab strip, lifted out of [`Self::body`]'s builder chain. + /// + /// It lives in its own method for the same reason [`Self::tab_strip`] takes `window`: the + /// strip is rendered through a lifted palette, which needs `window` and `&mut cx`, and the + /// `.children(...)` closure it used to sit inside can capture neither. + /// + /// Args: + /// window: Window that owns the strip's persistent overflow state. + /// cx: View context used to read the selected sub-tab and render the themed strip. + /// + /// Returns: + /// The Hotkeys sub-tab strip in its fixed-height wrapper. + fn hotkeys_sub_strip(&self, window: &mut Window, cx: &mut Context) -> AnyElement { + let view = cx.entity(); + let selected = self.hotkeys_sub; + let items: Vec = pages::HotkeysSub::ALL + .iter() + .map(|sub| MoonTabItem::new(sub.title()).selected(*sub == selected)) + .collect(); + let strip_h = design::fit_h_px(cx, 26.0, 13.0, 7.5); + let p = MoonPalette::active(cx); + let strip = MoonTabStrip::new("core-expert-hotkeys-tabs") + .gap(4.0) + .overflow_menu(true) + .items(items) + .on_click(move |ix, _event, _window, app| { + let Some(next) = pages::HotkeysSub::at(ix) else { + return; + }; + view.update(app, |this, cx| this.set_hotkeys_sub(next, cx)); + }); + let strip = design::chrome_tab_strip(strip, p, window, cx); + div() + .w_full() + .flex_none() + .h(strip_h) + .child(strip) + .into_any_element() } /// Body of the selected page. @@ -170,7 +229,20 @@ impl CoreExpertView { /// rather than by closing. With a page staged, a PORTED tab draws its rows. A tab that is not /// ported yet says so, and says separately when the reason is that nothing can ever arrive for /// it. - fn body(&self, p: MoonPalette, cx: &Context) -> impl IntoElement { + /// + /// Args: + /// p: Active palette used by the page body and its notices. + /// window: Window forwarded to the Hotkeys sub-tab strip when that page is selected. + /// cx: View context used to read state and build the selected page. + /// + /// Returns: + /// The scrollable body for the selected expert page. + fn body( + &self, + p: MoonPalette, + window: &mut Window, + cx: &mut Context, + ) -> impl IntoElement { let view = cx.entity(); // Read here, from the window's own `&self`: a page is built inside this render, where // reading the view back would panic. @@ -222,6 +294,11 @@ impl CoreExpertView { // A warning, through the shared component: a page whose values cannot arrive at all is not // the same news as one merely awaiting its port. let warn = self.state.can_send() && self.tab.source() != TabSource::Projected; + // Moonbot's Hotkeys page carries a strip of its own, above its body. Built HERE rather + // than inside the `.children(...)` closure below: it renders through a lifted palette, so + // it needs `window` and `&mut cx`, and a closure cannot capture either. + let hotkeys_strip = (self.tab == ExpertTab::Hotkeys && page.is_some()) + .then(|| self.hotkeys_sub_strip(window, cx)); v_flex() .id(self.tab.element_id()) .flex_1() @@ -250,32 +327,7 @@ impl CoreExpertView { .into_any_element() } })) - // Moonbot's Hotkeys page carries a strip of its own, above its body. - .children((self.tab == ExpertTab::Hotkeys && page.is_some()).then(|| { - let view = cx.entity(); - let selected = self.hotkeys_sub; - let items: Vec = pages::HotkeysSub::ALL - .iter() - .map(|sub| MoonTabItem::new(sub.title()).selected(*sub == selected)) - .collect(); - div() - .w_full() - .flex_none() - .h(design::fit_h_px(cx, 26.0, 13.0, 7.5)) - .child( - MoonTabStrip::new("core-expert-hotkeys-tabs") - .gap(4.0) - .overflow_menu(true) - .items(items) - .on_click(move |ix, _event, _window, app| { - let Some(next) = pages::HotkeysSub::at(ix) else { - return; - }; - view.update(app, |this, cx| this.set_hotkeys_sub(next, cx)); - }) - .render(), - ) - })) + .children(hotkeys_strip) .children(page) } diff --git a/crates/moon-ui-gpui/src/design.rs b/crates/moon-ui-gpui/src/design.rs index 911efb5d..85c0a3b9 100644 --- a/crates/moon-ui-gpui/src/design.rs +++ b/crates/moon-ui-gpui/src/design.rs @@ -5,7 +5,7 @@ use gpui::*; use moon_core::util::fmt::DeltaSign; -use moon_ui::{MoonMetrics, MoonPalette, MoonTheme, MoonTone, rgba_from}; +use moon_ui::{MoonMetrics, MoonPalette, MoonTableStyle, MoonTheme, MoonTone, rgba_from}; use std::collections::HashMap; use std::sync::{Arc, LazyLock}; @@ -160,6 +160,133 @@ pub fn chrome_toggle_label_color(p: MoonPalette, on: bool, caution: bool) -> u32 if on && caution { p.amber } else { p.text_soft } } +/// Foreground for a SECONDARY chrome label that still has to be read at a glance: an inactive tab +/// title, a table column header, the pinned-scope chip. +/// +/// One step above `p.text_muted`, which measures 3.5-3.8:1 against every chrome surface in both +/// stock themes and therefore sits under the 4.5:1 body floor; `p.text_soft` measures 6.7-7.0:1 on +/// light and 5.1-5.5:1 on dark. `p.text_dim` is NOT the step up: the dark palette defines it as the +/// same value as `p.text`, so using it here would erase the active/inactive distinction in dark +/// mode while looking correct in light mode. +/// +/// This is the label tone only. The ACTIVE member of a pair keeps `p.text`, so raising the +/// inactive one narrows the gap rather than closing it. +/// +/// Args: +/// p: Active palette whose secondary label tone is being resolved. +/// +/// Returns: +/// The accessible secondary-label colour. +pub fn chrome_label_color(p: MoonPalette) -> u32 { + p.text_soft +} + +/// Height of a chrome tab strip, in pixels at the current UI and font scale. +/// +/// One source for the triple `(28, 13, 7.5)`, which is `MoonTabStrip`'s own tab height: a strip +/// whose row is a different height from the tabs inside it puts the active-tab underline off the +/// row's bottom edge. Every strip in the app resolves it here — the two chart strips through +/// [`chart_tab_strip_h`](crate::chart_tabs::chart_tab_strip_h), which delegates to +/// [`tab_strip_h_value`], and the four window strips directly. +/// +/// Args: +/// cx: Application context supplying the current UI and font scales. +/// +/// Returns: +/// The scaled tab-strip height. +pub fn tab_strip_h(cx: &App) -> Pixels { + px(tab_strip_h_value(cx)) +} + +/// [`tab_strip_h`] as a bare number, for a caller that needs the value rather than a length. +/// +/// Args: +/// cx: Application context supplying the current UI and font scales. +/// +/// Returns: +/// The scaled tab-strip height as a raw number. +pub fn tab_strip_h_value(cx: &App) -> f32 { + fit_h_value(cx, 28.0, 13.0, 7.5) +} + +/// Renders one chrome tab strip through the lifted label palette and the LIVE theme tokens. +/// +/// The one place the three-line incantation lives, because getting it wrong is silent: +/// `render_with_palette` is one argument shorter, compiles, and substitutes default tokens, which +/// drops the user's font delta and UI scale — the labels shrink and the strip stops matching +/// [`tab_strip_h`], putting the active-tab underline out of line. The palette and tokens are read +/// into locals first because they cannot be read from `cx` in the same argument list that hands +/// `cx` over mutably, and the result is boxed because it would otherwise hold that `&mut cx` +/// borrow for as long as the element lives, which every caller still needs. +/// +/// Returns the strip BARE, with no sizing wrapper: a caller inside an already-sized row wants it +/// that way, and one placing it as a top-level child adds its own [`tab_strip_h`] box. +/// +/// Args: +/// strip: Configured tab-strip builder to render. +/// p: Active palette whose muted label tone will be lifted. +/// window: Window that owns the strip's persistent overflow state. +/// cx: Application context supplying the current theme tokens. +/// +/// Returns: +/// The themed, lifted tab strip without a sizing wrapper. +pub fn chrome_tab_strip( + strip: moon_ui::MoonTabStrip, + p: MoonPalette, + window: &mut Window, + cx: &mut App, +) -> AnyElement { + let palette = chrome_label_palette(p); + let tokens = MoonTheme::active_tokens(cx); + strip + .render_with_theme(window, cx, palette, tokens) + .into_any_element() +} + +/// `p` with its muted text lifted to [`chrome_label_color`]. +/// +/// For MoonUI chrome that keys an inactive label off `text_muted` and exposes no per-item colour +/// prop — `MoonTabStrip` is the case that needs it. Exactly one field moves, so every other colour +/// the widget draws still comes from the same palette as the row around it. +/// +/// Args: +/// p: Active palette to copy. +/// +/// Returns: +/// A copy with only `text_muted` raised to the chrome label colour. +pub fn chrome_label_palette(p: MoonPalette) -> MoonPalette { + MoonPalette { + text_muted: chrome_label_color(p), + ..p + } +} + +/// The one table style every `MoonDataTable` in this crate attaches: the palette's own fills and +/// selection, with the column-header text lifted to [`chrome_label_color`]. +/// +/// `MoonDataTable::render` re-themes any style it is handed, but that pass only replaces a field +/// still holding the stock dark default, and the lifted `header_text` never equals it — so the +/// lift survives. Do not call `themed` here; it would be a no-op today and a silent reset the day +/// the defaults move. +/// +/// One consequence was weighed and accepted rather than overlooked: MoonUI already draws the +/// SORTED column's header at this same tone, bypassing `header_text` entirely, so raising the +/// unsorted ones makes both read alike and leaves the sort arrow as the only sorted cue. The arrow +/// is part of the header's own text run and unambiguous; the alternative was leaving nine tables' +/// headings under the contrast floor to preserve a second, weaker signal. +/// +/// Args: +/// p: Active palette supplying the table's non-header colours. +/// +/// Returns: +/// The table style with an accessible column-header colour. +pub fn table_style(p: MoonPalette) -> MoonTableStyle { + MoonTableStyle { + header_text: chrome_label_color(p), + ..MoonTableStyle::for_palette(p) + } +} + /// Icon standing for "which columns does this table show", on every column selector in the app. /// /// All six pickers — Orders, Figures, Assets, the Screener, the Analytics tuner list and the diff --git a/crates/moon-ui-gpui/src/panels/alerts/table.rs b/crates/moon-ui-gpui/src/panels/alerts/table.rs index 65d6bee8..3de55e70 100644 --- a/crates/moon-ui-gpui/src/panels/alerts/table.rs +++ b/crates/moon-ui-gpui/src/panels/alerts/table.rs @@ -108,6 +108,7 @@ impl AlertsPanel { .state(&table_state) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) // Row selection carries no meaning here — the marked row is the one whose settings are // open — so a click clears the fork's three coupled selection fields immediately. // Only when one of them is actually set: an unconditional `notify` here would wake the diff --git a/crates/moon-ui-gpui/src/panels/assets/table.rs b/crates/moon-ui-gpui/src/panels/assets/table.rs index c746861e..379fa2ee 100644 --- a/crates/moon-ui-gpui/src/panels/assets/table.rs +++ b/crates/moon-ui-gpui/src/panels/assets/table.rs @@ -993,6 +993,7 @@ pub(super) fn assets_table( .state(state) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) // A header click re-sorts the cached rows; the action column is not sortable. .on_sort(move |key, ascending, _window, app| { let key = key.to_string(); diff --git a/crates/moon-ui-gpui/src/panels/common.rs b/crates/moon-ui-gpui/src/panels/common.rs index 9b68e1f7..0441ffc1 100644 --- a/crates/moon-ui-gpui/src/panels/common.rs +++ b/crates/moon-ui-gpui/src/panels/common.rs @@ -608,7 +608,7 @@ pub(crate) fn pinned_scope_label( .border_color(design::moon_alpha(p.border, 0.42)) .font_family(design::mono()) .text_size(design::text_px(cx, design::ACTION_LABEL_BASE)) - .text_color(rgb(p.text_muted)) + .text_color(rgb(design::chrome_label_color(p))) .child(div().flex_none().child(design::PINNED_SCOPE_GLYPH)) .child(div().min_w_0().truncate().child(label)) .into_any_element() diff --git a/crates/moon-ui-gpui/src/panels/core_status/problems.rs b/crates/moon-ui-gpui/src/panels/core_status/problems.rs index a967effc..c6170a77 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/problems.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/problems.rs @@ -236,7 +236,8 @@ pub(super) fn problems_view( .columns(columns()) .state(state) .header_height(design::TABLE_HEAD_H) - .row_height(design::TABLE_ROW_H), + .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)), )) } diff --git a/crates/moon-ui-gpui/src/panels/core_status/table.rs b/crates/moon-ui-gpui/src/panels/core_status/table.rs index 1de32d88..641b9bb7 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/table.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/table.rs @@ -254,6 +254,7 @@ pub(super) fn core_status_table( .state(state) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) .on_right_click_row(move |ix, window, app| { let core = match menu_lines.get(ix) { Some(FlatLine::Core(row)) => menu_rows.get(*row), diff --git a/crates/moon-ui-gpui/src/panels/core_status/updates_list.rs b/crates/moon-ui-gpui/src/panels/core_status/updates_list.rs index 719278ed..32d8bb1e 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/updates_list.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/updates_list.rs @@ -78,7 +78,8 @@ pub(super) fn updates_table( .columns(columns()) .state(state) .header_height(design::TABLE_HEAD_H) - .row_height(design::TABLE_ROW_H), + .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)), ) } diff --git a/crates/moon-ui-gpui/src/panels/core_status/warnings.rs b/crates/moon-ui-gpui/src/panels/core_status/warnings.rs index f78cd5a4..40aa2c03 100644 --- a/crates/moon-ui-gpui/src/panels/core_status/warnings.rs +++ b/crates/moon-ui-gpui/src/panels/core_status/warnings.rs @@ -76,7 +76,8 @@ pub(super) fn warnings_table( .columns(columns()) .state(state) .header_height(design::TABLE_HEAD_H) - .row_height(design::TABLE_ROW_H), + .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)), ) } diff --git a/crates/moon-ui-gpui/src/panels/orders/table.rs b/crates/moon-ui-gpui/src/panels/orders/table.rs index 05189881..3a404935 100644 --- a/crates/moon-ui-gpui/src/panels/orders/table.rs +++ b/crates/moon-ui-gpui/src/panels/orders/table.rs @@ -129,6 +129,7 @@ pub(super) fn orders_table( .state(state) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) .on_sort(move |key, ascending, _window, app| { let key = key.to_string(); sort_view.update(app, |this, cx| { diff --git a/crates/moon-ui-gpui/src/panels/report/render.rs b/crates/moon-ui-gpui/src/panels/report/render.rs index eb48a8d3..5faf049b 100644 --- a/crates/moon-ui-gpui/src/panels/report/render.rs +++ b/crates/moon-ui-gpui/src/panels/report/render.rs @@ -89,6 +89,7 @@ impl ReportPanel { .controlled_row_selection(true) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) .on_select_row(move |row, window, app| { let modifiers = window.modifiers(); view_click.update(app, |this, cx| this.select_report_row(row, modifiers, cx)); diff --git a/crates/moon-ui-gpui/src/screener/view.rs b/crates/moon-ui-gpui/src/screener/view.rs index 04daf8f2..43d2daad 100644 --- a/crates/moon-ui-gpui/src/screener/view.rs +++ b/crates/moon-ui-gpui/src/screener/view.rs @@ -440,6 +440,7 @@ impl ScreenerView { .state(&self.table_state) .header_height(design::TABLE_HEAD_H) .row_height(design::TABLE_ROW_H) + .style(design::table_style(p)) .on_sort(move |key, ascending, _window, app| { let key = key.to_string(); sort_view.update(app, |t, cx| t.set_sort(&key, !ascending, cx)); diff --git a/crates/moon-ui-gpui/src/settings/hotkeys/tab.rs b/crates/moon-ui-gpui/src/settings/hotkeys/tab.rs index 55a8bc80..f295a2cd 100644 --- a/crates/moon-ui-gpui/src/settings/hotkeys/tab.rs +++ b/crates/moon-ui-gpui/src/settings/hotkeys/tab.rs @@ -34,7 +34,24 @@ const ROW_TITLE_WIDTH: f32 = 160.0; const ROW_DESCRIPTION_MAX_WIDTH: f32 = 640.0; impl SettingsView { - pub(in crate::settings) fn hotkeys_tab(&self, cx: &Context) -> impl IntoElement { + /// Builds the Settings Hotkeys tab, including its lifted-contrast group strip. + /// + /// The strip needs `window` because it is rendered through a lifted palette rather than the + /// active one: MoonUI keys an inactive tab label off `text_muted`, which sits under the body + /// contrast floor in both stock themes, and `render_with_theme` is the only way to hand it a + /// different palette. + /// + /// Args: + /// window: Window that owns the strip's persistent overflow state. + /// cx: Settings context used to read the hotkey draft and build callbacks. + /// + /// Returns: + /// The complete Hotkeys tab content. + pub(in crate::settings) fn hotkeys_tab( + &self, + window: &mut Window, + cx: &mut Context, + ) -> impl IntoElement { let hotkeys = { let b = self.backend.read(cx); b.preview.as_ref().unwrap_or(&b.config).hotkeys.clone() @@ -76,28 +93,27 @@ impl SettingsView { // Reuse the main window's chart-tab control (`MoonTabStrip` + `MoonTabItem`) for // normal-case labels. Overflow-menu defaults off, so a short group list stays chevron-free. let entity = cx.entity(); - let strip_h = design::fit_h_px(cx, 28.0, 13.0, 7.5); + let strip_h = design::tab_strip_h(cx); let items: Vec = HotkeyGroup::ALL .iter() .map(|g| MoonTabItem::new(g.title()).selected(self.hotkeys_group == *g)) .collect(); - let switcher = div().w_full().h(strip_h).child( - MoonTabStrip::new("hotkeys-group-strip") - .gap(4.0) - .items(items) - .on_click(move |ix, _event, _window, app| { - let Some(g) = HotkeyGroup::ALL.get(ix).copied() else { - return; - }; - entity.update(app, |this, c| { - if this.hotkeys_group != g { - this.hotkeys_group = g; - c.notify(); - } - }); - }) - .render(), - ); + let strip = MoonTabStrip::new("hotkeys-group-strip") + .gap(4.0) + .items(items) + .on_click(move |ix, _event, _window, app| { + let Some(g) = HotkeyGroup::ALL.get(ix).copied() else { + return; + }; + entity.update(app, |this, c| { + if this.hotkeys_group != g { + this.hotkeys_group = g; + c.notify(); + } + }); + }); + let strip = design::chrome_tab_strip(strip, p, window, cx); + let switcher = div().w_full().h(strip_h).child(strip); let body = v_flex() .w_full() diff --git a/crates/moon-ui-gpui/src/settings/render.rs b/crates/moon-ui-gpui/src/settings/render.rs index 3c36b6e4..00831f84 100644 --- a/crates/moon-ui-gpui/src/settings/render.rs +++ b/crates/moon-ui-gpui/src/settings/render.rs @@ -67,7 +67,7 @@ impl Render for SettingsView { let content = match self.active { Tab::Interface => self.interface_tab(cx).into_any_element(), Tab::General => self.general_tab(cx).into_any_element(), - Tab::Hotkeys => self.hotkeys_tab(cx).into_any_element(), + Tab::Hotkeys => self.hotkeys_tab(window, cx).into_any_element(), Tab::Lines => self.lines_tab(cx).into_any_element(), Tab::Badges => self.badges_tab(cx).into_any_element(), Tab::Connections => self.connections_tab(cx).into_any_element(), diff --git a/crates/moon-ui-gpui/src/shell/core_settings.rs b/crates/moon-ui-gpui/src/shell/core_settings.rs index e4dbd34e..e7eea9ff 100644 --- a/crates/moon-ui-gpui/src/shell/core_settings.rs +++ b/crates/moon-ui-gpui/src/shell/core_settings.rs @@ -278,6 +278,15 @@ impl Shell { input: &self.blacklist_input, area: &self.blacklist_area, }; + // Built here, not inside the content builder: the strip renders through a lifted palette, + // which needs `window` and `&mut cx` that `core_settings_content` does not hold. + let tab_strip = core_settings_popup::core_settings_tab_strip( + self.core_settings_tab, + p, + &view, + window, + cx, + ); core_settings_popup::core_settings_content( &ctx, self.core_settings_tab, @@ -287,6 +296,7 @@ impl Shell { self.core_settings_bl_expanded, self.core_settings_cancel_confirm, &view, + tab_strip, cx, move |app| cancel_view.update(app, |this, cx| this.core_settings_cancel_all_click(cx)), move |window, app| { diff --git a/crates/moon-ui-gpui/src/shell/core_settings_popup.rs b/crates/moon-ui-gpui/src/shell/core_settings_popup.rs index 6b3b3205..2ba63c2d 100644 --- a/crates/moon-ui-gpui/src/shell/core_settings_popup.rs +++ b/crates/moon-ui-gpui/src/shell/core_settings_popup.rs @@ -142,6 +142,49 @@ pub(crate) fn slider_specs( specs } +/// The popup's tab strip, built by the CALLER and handed in. +/// +/// It is not built inside [`core_settings_content`] because it is rendered through a lifted +/// palette: MoonUI keys an inactive tab label off `text_muted`, which sits under the body contrast +/// floor in both stock themes, and `render_with_theme` is the only way to hand it a different +/// palette — and that needs `&mut Window` and `&mut App`, which the content builder does not hold. +/// Reuses the main window's chart-tab control, as the Settings window's hotkey groups do. +/// +/// Args: +/// tab: Core-settings tab that the strip marks as selected. +/// p: Active palette whose muted label tone is lifted for the strip. +/// view: Shell entity updated when the user selects another tab. +/// window: Window that owns the strip's persistent overflow state. +/// cx: Application context used to render the themed strip. +/// +/// Returns: +/// The core-settings tab strip in its fixed-height wrapper. +pub(crate) fn core_settings_tab_strip( + tab: CoreSettingsTab, + p: MoonPalette, + view: &Entity, + window: &mut Window, + cx: &mut App, +) -> AnyElement { + let view = view.clone(); + let items: Vec = CoreSettingsTab::ALL + .iter() + .map(|t| MoonTabItem::new(t.title()).selected(tab == *t)) + .collect(); + let strip_h = design::tab_strip_h(cx); + let strip = MoonTabStrip::new("core-settings-tabs") + .gap(4.0) + .items(items) + .on_click(move |ix, _event, _window, app| { + let Some(next) = CoreSettingsTab::ALL.get(ix).copied() else { + return; + }; + view.update(app, |this, cx| this.set_core_settings_tab(next, cx)); + }); + let strip = design::chrome_tab_strip(strip, p, window, cx); + div().w_full().h(strip_h).child(strip).into_any_element() +} + /// Builds core-settings popover content. /// /// Args: @@ -153,6 +196,7 @@ pub(crate) fn slider_specs( /// blacklist_expanded: Whether to render the multiline blacklist editor. /// cancel_confirm: Whether Cancel All Orders is awaiting confirmation. /// view: Shell entity used by tab switching, staging, OK, and Cancel. +/// tab_strip: Pre-rendered contrast-safe tab strip, built where a mutable window is available. /// cx: Application context used to read state and render controls. /// on_cancel_all: Callback for the staged Cancel All Orders action. /// on_toggle_blacklist: Callback that toggles the blacklist editor mode. @@ -169,6 +213,7 @@ pub(crate) fn core_settings_content( blacklist_expanded: bool, cancel_confirm: bool, view: &Entity, + tab_strip: AnyElement, cx: &App, on_cancel_all: impl Fn(&mut App) + 'static, on_toggle_blacklist: impl Fn(&mut Window, &mut App) + 'static, @@ -217,30 +262,6 @@ pub(crate) fn core_settings_content( let actions = action_row(&cs, cancel_confirm, ctx, cx, on_cancel_all); - // Reuse the main window's chart-tab control, as the Settings window's hotkey groups do. - let strip = { - let view = view.clone(); - let items: Vec = CoreSettingsTab::ALL - .iter() - .map(|t| MoonTabItem::new(t.title()).selected(tab == *t)) - .collect(); - div() - .w_full() - .h(design::fit_h_px(cx, 28.0, 13.0, 7.5)) - .child( - MoonTabStrip::new("core-settings-tabs") - .gap(4.0) - .items(items) - .on_click(move |ix, _event, _window, app| { - let Some(next) = CoreSettingsTab::ALL.get(ix).copied() else { - return; - }; - view.update(app, |this, cx| this.set_core_settings_tab(next, cx)); - }) - .render(), - ) - }; - let body = match draft { Some(draft) => match tab { CoreSettingsTab::General => general::general_tab( @@ -266,7 +287,7 @@ pub(crate) fn core_settings_content( }; root.child(actions) - .child(strip) + .child(tab_strip) .child(body) .when(draft.is_some(), |r| r.child(footer(view, cx))) .into_any_element() diff --git a/crates/moon-ui-gpui/tests/theme_contract/dock_chrome.rs b/crates/moon-ui-gpui/tests/theme_contract/dock_chrome.rs index b4e4aece..b96204cf 100644 --- a/crates/moon-ui-gpui/tests/theme_contract/dock_chrome.rs +++ b/crates/moon-ui-gpui/tests/theme_contract/dock_chrome.rs @@ -215,3 +215,105 @@ fn detects_toolbar_keeps_the_shared_band_and_drops_tabbar() { "p.tabbar is expected to have no remaining use once Goal C lands; found in {hits:?}" ); } + +/// Every `MoonDataTable::new` builder must apply `design::table_style(p)`. +/// +/// Breakage this pins: add a tenth table, or remove this adapter from one of the nine named +/// builders. Its column headers would then fall back to the low-contrast muted ink while every +/// sibling table remains readable, making the missing call look like a panel-local rendering bug. +#[test] +fn every_data_table_applies_the_chrome_header_style() { + const TABLES: [(&str, &str); 9] = [ + ("panels/alerts/table.rs", "pub(super) fn table("), + ("panels/assets/table.rs", "pub(super) fn assets_table("), + ( + "panels/core_status/problems.rs", + "pub(super) fn problems_view(", + ), + ( + "panels/core_status/table.rs", + "pub(super) fn core_status_table(", + ), + ( + "panels/core_status/updates_list.rs", + "pub(super) fn updates_table(", + ), + ( + "panels/core_status/warnings.rs", + "pub(super) fn warnings_table(", + ), + ("panels/orders/table.rs", "pub(super) fn orders_table("), + ("panels/report/render.rs", "pub(super) fn table_el("), + ("screener/view.rs", "fn table(&self, cx: &Context)"), + ]; + + let mut sources = Vec::new(); + rust_sources( + &Path::new(env!("CARGO_MANIFEST_DIR")).join("src"), + &mut sources, + ); + let table_count: usize = sources + .iter() + .map(|path| { + let source = fs::read_to_string(path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); + code_only(&source).matches("MoonDataTable::new(").count() + }) + .sum(); + assert_eq!( + table_count, 9, + "the nine planned MoonDataTable builders are the complete app-side table surface" + ); + + for (path, signature) in TABLES { + let source = code_only(&read_src(path)); + assert_eq!( + source.matches("MoonDataTable::new(").count(), + 1, + "{path} must contain exactly one MoonDataTable builder" + ); + let body = braced_body(&source, signature); + assert_eq!( + body.matches("MoonDataTable::new(").count(), + 1, + "{path}:{signature} must retain its one table builder" + ); + let _builder_chain = chain_between( + body, + "MoonDataTable::new(", + ".style(design::table_style(p))", + path, + ); + } +} + +/// `design::chrome_label_color` and `design::table_style` must preserve the contrast lift. +/// +/// Breakage this pins: simplify the label helper back to muted or dim ink, or theme the table +/// style before MoonUI applies its runtime palette. Muted ink loses the light-theme contrast floor, +/// while dim ink can erase the active-versus-inactive distinction in the dark palette. +#[test] +fn chrome_label_helpers_keep_the_single_contrast_lift() { + let design = code_only(&read_src("design.rs")); + let label_color = braced_body(&design, "fn chrome_label_color("); + assert!( + label_color.contains("p.text_soft"), + "chrome_label_color must select text_soft for the chrome contrast lift" + ); + for rejected in ["p.text_muted", "p.text_dim"] { + assert!( + !label_color.contains(rejected), + "chrome_label_color must not regress to {rejected}" + ); + } + + let table_style = braced_body(&design, "fn table_style("); + assert!( + table_style.contains("header_text: chrome_label_color(p)"), + "table_style must route header ink through chrome_label_color" + ); + assert!( + !table_style.contains(".themed("), + "table_style must leave runtime palette resolution to MoonDataTable" + ); +}