From f8d5099cdacba4d8f2ef4dbb321942c15996f083 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Sat, 19 Sep 2026 10:03:44 +0100 Subject: [PATCH] fix(block-palette): use theme-aware label colors with contrast coverage --- src/ui/theme_accessibility_tests.rs | 173 ++++++++++++++++++++++++++++ src/ui/window/block_palette.rs | 93 ++++++++++++--- 2 files changed, 249 insertions(+), 17 deletions(-) diff --git a/src/ui/theme_accessibility_tests.rs b/src/ui/theme_accessibility_tests.rs index 576d167f5..f8e6caecf 100644 --- a/src/ui/theme_accessibility_tests.rs +++ b/src/ui/theme_accessibility_tests.rs @@ -9,6 +9,8 @@ //! - Properties & Dock Panels (field labels, section headers, muted text) //! - Modals & Action Buttons (dialog body, Primary, Danger, Success buttons) //! - Dropdowns & Selection Overlays (visual style picker, item checkmarks) +//! - Block Palette (card labels in normal/hover/pressed/placing states, +//! header icon buttons, empty-state muted text) use crate::ui::style::common::{accessible_accent, accessible_accent_threshold, wcag_contrast}; use iced::{Color, Theme}; @@ -462,3 +464,174 @@ fn test_viewport_controls_toggle_buttons_contrast() { } } +#[test] +fn test_block_palette_contrast() { + use crate::ui::window::block_palette::{ + block_card_border, block_card_colors, block_icon_button_text_color, + }; + use iced::widget::button::Status as BtnStatus; + + for theme in Theme::ALL { + let p = theme.palette(); + + // ── 1. Card label pairs resolve to the theme's own text colors ── + // Regression guard for the light-theme bug where labels were hardcoded + // to `Color::WHITE` (unreadable white-on-light). If anyone reintroduces + // a hardcoded foreground, these equality checks fail on every theme + // whose surface text is not pure white. + let (normal_bg, normal_fg) = block_card_colors(theme, false, BtnStatus::Active); + assert_eq!( + normal_bg, p.background.base.color, + "Theme {:?} block card normal background drifted from base pair", + theme + ); + assert_eq!( + normal_fg, p.background.base.text, + "Theme {:?} block card normal label must use base text (was hardcoded WHITE)", + theme + ); + + let (_, hover_fg) = block_card_colors(theme, false, BtnStatus::Hovered); + assert_eq!( + hover_fg, p.background.strong.text, + "Theme {:?} block card hover label must use strong text (was hardcoded WHITE)", + theme + ); + + let (_, pressed_fg) = block_card_colors(theme, false, BtnStatus::Pressed); + assert_eq!( + pressed_fg, p.background.strong.text, + "Theme {:?} block card pressed label must use strong text (was hardcoded WHITE)", + theme + ); + + // Default-branch statuses (Active/Disabled/Focused/...) share the base pair. + let (_, disabled_fg) = block_card_colors(theme, false, BtnStatus::Disabled); + assert_eq!( + disabled_fg, p.background.base.text, + "Theme {:?} block card default-state label must use base text", + theme + ); + + // Placing (selected) cards always use the primary pair, regardless of + // hover — the background does not switch on hover while placing. + for status in [BtnStatus::Active, BtnStatus::Hovered, BtnStatus::Pressed] { + let (bg, fg) = block_card_colors(theme, true, status); + assert_eq!(bg, p.primary.base.color, "Theme {:?} placing card bg must be primary.base", theme); + assert_eq!(fg, p.primary.base.text, "Theme {:?} placing card label must be primary.base.text", theme); + } + + // ── 2. Card label contrast: normal / hover / pressed ── + // 11px single-line labels are normal text → WCAG AA >= 4.5:1. + let normal_contrast = wcag_contrast(normal_fg, normal_bg); + assert!( + normal_contrast >= 4.5, + "Theme {:?} block card label ({:.2}:1) fails WCAG AA on card background", + theme, normal_contrast + ); + + let (hover_bg, hover_fg) = block_card_colors(theme, false, BtnStatus::Hovered); + let hover_contrast = wcag_contrast(hover_fg, hover_bg); + assert!( + hover_contrast >= 4.5, + "Theme {:?} block card hovered label ({:.2}:1) fails WCAG AA on hovered background", + theme, hover_contrast + ); + + let (pressed_bg, pressed_fg) = block_card_colors(theme, false, BtnStatus::Pressed); + let pressed_contrast = wcag_contrast(pressed_fg, pressed_bg); + assert!( + pressed_contrast >= 4.5, + "Theme {:?} block card pressed label ({:.2}:1) fails WCAG AA on pressed background", + theme, pressed_contrast + ); + + // ── 3. Placing card contrast ── + // Selected-card label on `primary.base`: UI button component threshold + // (>= 3.0:1 per WCAG 1.4.11, matching the Primary-button test above). + let (placing_bg, placing_fg) = block_card_colors(theme, true, BtnStatus::Active); + let placing_contrast = wcag_contrast(placing_fg, placing_bg); + assert!( + placing_contrast >= 3.0, + "Theme {:?} placing block card label ({:.2}:1) fails button threshold (>=3.0:1)", + theme, placing_contrast + ); + // Placing + hover must not silently drop contrast (bg is sticky). + let (placing_hover_bg, placing_hover_fg) = + block_card_colors(theme, true, BtnStatus::Hovered); + let placing_hover_contrast = wcag_contrast(placing_hover_fg, placing_hover_bg); + assert!( + placing_hover_contrast >= 3.0, + "Theme {:?} placing+hover block card label ({:.2}:1) fails button threshold (>=3.0:1)", + theme, placing_hover_contrast + ); + + // ── 4. Card border is theme-driven (no invisible borders) ── + assert_eq!( + block_card_border(theme, false), + p.background.neutral.color, + "Theme {:?} block card border must use neutral color", + theme + ); + assert_eq!( + block_card_border(theme, true), + p.primary.base.color, + "Theme {:?} placing block card border must use primary color", + theme + ); + + // ── 5. Header icon buttons (glyphs inherit button text_color) ── + // Resting buttons are transparent over the dock (`base`) background. + let icon_rest = block_icon_button_text_color(theme, BtnStatus::Active); + assert_eq!(icon_rest, p.background.base.text); + let icon_rest_contrast = wcag_contrast(icon_rest, p.background.base.color); + assert!( + icon_rest_contrast >= 4.5, + "Theme {:?} block palette icon button ({:.2}:1) fails WCAG AA on dock background", + theme, icon_rest_contrast + ); + // Hovered/pressed buttons sit on the `strong` surface with its text. + for status in [BtnStatus::Hovered, BtnStatus::Pressed] { + let icon_fg = block_icon_button_text_color(theme, status); + assert_eq!( + icon_fg, p.background.strong.text, + "Theme {:?} hovered icon button must use strong text", + theme + ); + let c = wcag_contrast(icon_fg, p.background.strong.color); + assert!( + c >= 4.5, + "Theme {:?} hovered icon button ({:.2}:1) fails WCAG AA on hovered background", + theme, c + ); + } + + // ── 6. Empty-state muted text ("No blocks in this drawing") ── + // Rendered as `base.text` at 0.72 alpha over the dock `base` + // background → secondary text threshold >= 3.0:1. (The old hardcoded + // 0.55-gray failed this on light themes.) + let empty_text = composite_over(p.background.base.text.scale_alpha(0.72), p.background.base.color); + let empty_contrast = wcag_contrast(empty_text, p.background.base.color); + assert!( + empty_contrast >= 3.0, + "Theme {:?} block palette empty-state text ({:.2}:1) fails secondary text threshold (>=3.0:1)", + theme, empty_contrast + ); + + // ── 7. Hardcoded-white regression probe ── + // The exact reported bug: pure white labels on the light card + // background. Every theme's card background must NOT be near-white + // with white text — i.e. white-on-card must fail while the real + // theme pair passes (proves the fix matters on light themes and the + // test would catch a WHITE hardcode). + let white_on_normal = wcag_contrast(Color::WHITE, normal_bg); + if white_on_normal < 4.5 { + assert!( + normal_contrast >= 4.5, + "Theme {:?} needs theme-aware labels: white-on-card is {:.2}:1 but theme pair is {:.2}:1", + theme, white_on_normal, normal_contrast + ); + } + } +} + diff --git a/src/ui/window/block_palette.rs b/src/ui/window/block_palette.rs index bd8e7f351..f82ed09c8 100644 --- a/src/ui/window/block_palette.rs +++ b/src/ui/window/block_palette.rs @@ -264,7 +264,20 @@ pub fn view(palette: &BlockPalette, width: f32, auto_collapse: bool) -> Element< } else { "No matches" }; - container(text(crate::t!(msg)).size(12).color(iced::Color { r: 0.55, g: 0.55, b: 0.55, a: 1.0 })) + container( + text(crate::t!(msg)) + .size(12) + .style(|theme: &Theme| iced::widget::text::Style { + color: Some( + theme + .palette() + .background + .base + .text + .scale_alpha(0.72), + ), + }), + ) .center_x(Fill) .center_y(Fill) .width(Fill) @@ -307,6 +320,56 @@ pub fn view(palette: &BlockPalette, width: f32, auto_collapse: bool) -> Element< .into() } +/// Theme-aware foreground/background pair for a block card. +/// +/// Centralizes the card's text/background mapping so the contrast suite can +/// exercise the exact colors the UI renders (instead of re-implementing the +/// match). Normal cards use the `base` pair, hovered/pressed cards use the +/// `strong` pair, and the actively-placing card uses the `primary.base` pair. +pub(crate) fn block_card_colors( + theme: &Theme, + is_placing: bool, + status: button::Status, +) -> (Color, Color) { + let palette = theme.palette(); + if is_placing { + (palette.primary.base.color, palette.primary.base.text) + } else { + match status { + button::Status::Hovered | button::Status::Pressed => { + (palette.background.strong.color, palette.background.strong.text) + } + _ => ( + palette.background.base.color, + palette.background.base.text, + ), + } + } +} + +/// Border color for a block card in a given state (exposed for contrast tests). +pub(crate) fn block_card_border(theme: &Theme, is_placing: bool) -> Color { + let palette = theme.palette(); + if is_placing { + palette.primary.base.color + } else { + palette.background.neutral.color + } +} + +/// Theme-aware foreground for the palette's header icon buttons. +/// +/// Transparent resting state composites over the dock (`base`) background; +/// hovered/pressed state sits on the `strong` surface, so each state pairs +/// with its own background's text color to preserve contrast. +pub(crate) fn block_icon_button_text_color(theme: &Theme, status: button::Status) -> Color { + let palette = theme.palette(); + match status { + button::Status::Hovered | button::Status::Pressed => palette.background.strong.text, + _ => palette.background.base.text, + } +} + fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<'a, Message> { let is_placing = palette.placing.as_deref() == Some(block.name.as_str()); let preview = canvas(BlockPreviewCanvas { wires: &block.wires }) @@ -315,9 +378,13 @@ fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<' // Fixed height guarantees the label occupies exactly one line, so every // cell in a row keeps the same height even for long names (the elide budget // handles overflow, and this clips anything that still won't fit). + // + // NOTE: no explicit `.color(...)` here — the label inherits the button's + // `text_color` so normal/hover/placing states each get a theme-aware + // foreground with sufficient contrast on both light and dark themes. + // (A hardcoded `Color::WHITE` is unreadable on light themes.) let label = text(crate::ui::text_util::elide(&block.name, MAX_LABEL_CHARS)) .size(11) - .color(Color::WHITE) .width(Fill) .height(Length::Fixed(LABEL_LINE_H)) .center(); @@ -326,18 +393,8 @@ fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<' .on_press(Message::BlockPalette(BlockPaletteMsg::Insert(block.name.clone()))) .width(Fill) .style(move |theme: &Theme, status| { - let palette = theme.palette(); - let neutral = palette.background.neutral.color; - let base = palette.background.base.color; - let strong = palette.background.strong.color; - let (bg, border) = if is_placing { - (palette.primary.base.color, palette.primary.base.color) - } else { - match status { - button::Status::Hovered | button::Status::Pressed => (strong, neutral), - _ => (base, neutral), - } - }; + let (bg, fg) = block_card_colors(theme, is_placing, status); + let border = block_card_border(theme, is_placing); button::Style { background: Some(Background::Color(bg)), border: Border { @@ -345,7 +402,7 @@ fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<' width: 1.0, radius: 3.0.into(), }, - text_color: Color::WHITE, + text_color: fg, ..Default::default() } }) @@ -354,7 +411,9 @@ fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<' fn icon_button<'a>(icon: IconKind, msg: BlockPaletteMsg) -> Element<'a, Message> { let icon_el: Element<'_, Message> = match icon { - IconKind::Glyph(s) => text(s).size(15).color(Color::WHITE).into(), + // No explicit color — inherits the button's theme-aware `text_color` + // so glyphs stay legible on light themes. + IconKind::Glyph(s) => text(s).size(15).into(), IconKind::Svg(bytes) => crate::ui::icons::semantic(bytes, TOOL_H), }; button(icon_el) @@ -372,7 +431,7 @@ fn icon_button<'a>(icon: IconKind, msg: BlockPaletteMsg) -> Element<'a, Message> radius: 3.0.into(), ..Default::default() }, - text_color: Color::WHITE, + text_color: block_icon_button_text_color(theme, status), ..Default::default() }) .into()