forked from 1b-agent/opencadstudio-upstream
fix(block-palette): use theme-aware label colors with contrast coverage
This commit is contained in:
1 parent
4fdef48502
commit
f8d5099cda
2 files changed
+249
-17
No files matched your search
@@ -9,6 +9,8 @@
|
|||||||
//! - Properties & Dock Panels (field labels, section headers, muted text)
|
//! - Properties & Dock Panels (field labels, section headers, muted text)
|
||||||
//! - Modals & Action Buttons (dialog body, Primary, Danger, Success buttons)
|
//! - Modals & Action Buttons (dialog body, Primary, Danger, Success buttons)
|
||||||
//! - Dropdowns & Selection Overlays (visual style picker, item checkmarks)
|
//! - 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 crate::ui::style::common::{accessible_accent, accessible_accent_threshold, wcag_contrast};
|
||||||
use iced::{Color, Theme};
|
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
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@@ -264,7 +264,20 @@ pub fn view(palette: &BlockPalette, width: f32, auto_collapse: bool) -> Element<
|
|||||||
} else {
|
} else {
|
||||||
"No matches"
|
"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_x(Fill)
|
||||||
.center_y(Fill)
|
.center_y(Fill)
|
||||||
.width(Fill)
|
.width(Fill)
|
||||||
@@ -307,6 +320,56 @@ pub fn view(palette: &BlockPalette, width: f32, auto_collapse: bool) -> Element<
|
|||||||
.into()
|
.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> {
|
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 is_placing = palette.placing.as_deref() == Some(block.name.as_str());
|
||||||
let preview = canvas(BlockPreviewCanvas { wires: &block.wires })
|
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
|
// 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
|
// 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).
|
// 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))
|
let label = text(crate::ui::text_util::elide(&block.name, MAX_LABEL_CHARS))
|
||||||
.size(11)
|
.size(11)
|
||||||
.color(Color::WHITE)
|
|
||||||
.width(Fill)
|
.width(Fill)
|
||||||
.height(Length::Fixed(LABEL_LINE_H))
|
.height(Length::Fixed(LABEL_LINE_H))
|
||||||
.center();
|
.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())))
|
.on_press(Message::BlockPalette(BlockPaletteMsg::Insert(block.name.clone())))
|
||||||
.width(Fill)
|
.width(Fill)
|
||||||
.style(move |theme: &Theme, status| {
|
.style(move |theme: &Theme, status| {
|
||||||
let palette = theme.palette();
|
let (bg, fg) = block_card_colors(theme, is_placing, status);
|
||||||
let neutral = palette.background.neutral.color;
|
let border = block_card_border(theme, is_placing);
|
||||||
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),
|
|
||||||
}
|
|
||||||
};
|
|
||||||
button::Style {
|
button::Style {
|
||||||
background: Some(Background::Color(bg)),
|
background: Some(Background::Color(bg)),
|
||||||
border: Border {
|
border: Border {
|
||||||
@@ -345,7 +402,7 @@ fn block_card<'a>(palette: &'a BlockPalette, block: &'a BlockEntry) -> Element<'
|
|||||||
width: 1.0,
|
width: 1.0,
|
||||||
radius: 3.0.into(),
|
radius: 3.0.into(),
|
||||||
},
|
},
|
||||||
text_color: Color::WHITE,
|
text_color: fg,
|
||||||
..Default::default()
|
..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> {
|
fn icon_button<'a>(icon: IconKind, msg: BlockPaletteMsg) -> Element<'a, Message> {
|
||||||
let icon_el: Element<'_, Message> = match icon {
|
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),
|
IconKind::Svg(bytes) => crate::ui::icons::semantic(bytes, TOOL_H),
|
||||||
};
|
};
|
||||||
button(icon_el)
|
button(icon_el)
|
||||||
@@ -372,7 +431,7 @@ fn icon_button<'a>(icon: IconKind, msg: BlockPaletteMsg) -> Element<'a, Message>
|
|||||||
radius: 3.0.into(),
|
radius: 3.0.into(),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
},
|
},
|
||||||
text_color: Color::WHITE,
|
text_color: block_icon_button_text_color(theme, status),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
.into()
|
.into()
|
||||||
|
|||||||
Reference in new issue
Block a user