Merge pull request #1911 from TrevorBrown-dev/fix/layers-component-contrast

Layers panel: component rows in the theme's component tint, at AA contrast (#1903)

crates/ui-egui/src/layers_component_rows_tests.rs

@@ -0,0 +1,131 @@
+//! Component rows in the Layers panel (#1903), drawn headless in both themes: a component's name
+//! reads at WCAG AA (4.5:1) on the panel and on its selected row, and the rows inside a selected
+//! component take a tone of its row, not the plain selection's green.
+
+use egui::{Color32, Pos2, Shape};
+use figcraft_doc::NodeId;
+use figcraft_engine::Session;
+use serde_json::json;
+
+use crate::layers_drag_tests::Ui;
+use crate::theme::Tokens;
+
+fn id(v: serde_json::Value) -> NodeId {
+    NodeId(v["id"].as_u64().unwrap())
+}
+
+fn color(x: Color32) -> figcraft_color::Color {
+    figcraft_color::Color::new(f32::from(x.r()) / 255.0, f32::from(x.g()) / 255.0, f32::from(x.b()) / 255.0, 1.0)
+}
+
+/// The Layers panel with the component "Button" (holding the rectangle "Label") selected and
+/// expanded, in the `dark` or light theme: the frame's shapes and the theme's tokens.
+fn drawn(dark: bool) -> (Vec<Shape>, Tokens) {
+    let mut s = Session::new();
+    let f = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 100, "height": 40, "props": {"name": "Button"}})).unwrap());
+    s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 20, "height": 20, "parent": f, "props": {"name": "Label"}})).unwrap();
+    let c = id(s.execute("component.create", &json!({"ids": [f]})).unwrap());
+    s.execute(if dark { "view.themeDark" } else { "view.themeLight" }, &json!({})).unwrap();
+    s.execute("select.set", &json!({"ids": [c]})).unwrap();
+    let mut u = Ui::new(s);
+    u.app.ui.expanded.insert(c);
+    u.frame(vec![]);
+    let shapes = crate::live_tests::flat(&u.frame_output(vec![]).shapes);
+    (shapes, Tokens::get(&u.ctx))
+}
+
+/// The colour the name `s` is painted in.
+fn name_colour(shapes: &[Shape], s: &str) -> Color32 {
+    shapes
+        .iter()
+        .find_map(|sh| match sh {
+            Shape::Text(t) if t.galley.text() == s => Some(t.fallback_color),
+            _ => None,
+        })
+        .unwrap_or_else(|| panic!("no name {s:?} painted"))
+}
+
+/// Where the name `s` is painted (its top left).
+fn name_pos(shapes: &[Shape], s: &str) -> Pos2 {
+    shapes
+        .iter()
+        .find_map(|sh| match sh {
+            Shape::Text(t) if t.galley.text() == s => Some(t.pos),
+            _ => None,
+        })
+        .unwrap_or_else(|| panic!("no name {s:?} painted"))
+}
+
+/// Whether a rectangle filled in `c` is painted under `p`.
+fn rect_under(shapes: &[Shape], c: Color32, p: Pos2) -> bool {
+    shapes.iter().any(|s| matches!(s, Shape::Rect(r) if r.fill == c && r.rect.contains(p)))
+}
+
+#[test]
+fn component_names_read_at_aa_on_the_panel_and_on_their_selected_row() {
+    for dark in [false, true] {
+        let (shapes, t) = drawn(dark);
+        assert_eq!(t.dark, dark);
+        let col = name_colour(&shapes, "Button");
+        assert_eq!(col, t.component, "the theme's component tint (dark {dark})");
+        for (what, bg) in [("the panel", t.panel), ("the selected component row", t.component_row), ("a hovered row", t.hover)] {
+            let ratio = color(col).contrast(color(bg));
+            assert!(ratio >= 4.5, "{ratio:.2}:1 on {what} (dark {dark})");
+        }
+        // The layer inside it is a plain layer: the theme's text colour.
+        assert_eq!(name_colour(&shapes, "Label"), t.text);
+    }
+}
+
+#[test]
+fn rows_inside_a_selected_component_take_a_tone_of_its_row() {
+    for dark in [false, true] {
+        let (shapes, t) = drawn(dark);
+        let button = name_pos(&shapes, "Button");
+        let label = name_pos(&shapes, "Label");
+        // Checked under each name, by row: the pages list paints the current page in the plain
+        // selection's `selected_row` too.
+        assert!(rect_under(&shapes, t.component_row, button), "the component's own row (dark {dark})");
+        assert!(rect_under(&shapes, t.component_row_child, label), "the row inside it (dark {dark})");
+        assert!(!rect_under(&shapes, t.selected_row, button), "not the plain selection's row (dark {dark})");
+        assert!(!rect_under(&shapes, t.selected_row_child, label), "not the plain selection's child tone (dark {dark})");
+    }
+}
+
+/// The Layers panel with the component "Button" hidden, and selected or not.
+fn drawn_hidden(dark: bool, selected: bool) -> (Vec<Shape>, Tokens) {
+    let mut s = Session::new();
+    let f = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 100, "height": 40, "props": {"name": "Button"}})).unwrap());
+    let c = id(s.execute("component.create", &json!({"ids": [f]})).unwrap());
+    s.execute(if dark { "view.themeDark" } else { "view.themeLight" }, &json!({})).unwrap();
+    s.execute("node.set", &json!({"id": c, "props": {"visible": false}})).unwrap();
+    s.execute("select.set", &json!({"ids": if selected { vec![c] } else { vec![] }})).unwrap();
+    let mut u = Ui::new(s);
+    u.frame(vec![]);
+    let shapes = crate::live_tests::flat(&u.frame_output(vec![]).shapes);
+    (shapes, Tokens::get(&u.ctx))
+}
+
+/// `fg` (premultiplied, as egui paints it) composited over the opaque `bg`.
+fn over(fg: Color32, bg: Color32) -> Color32 {
+    let k = 1.0 - f32::from(fg.a()) / 255.0;
+    let mix = |f: u8, b: u8| (f32::from(f) + f32::from(b) * k).round().min(255.0) as u8;
+    Color32::from_rgb(mix(fg.r(), bg.r()), mix(fg.g(), bg.g()), mix(fg.b(), bg.b()))
+}
+
+#[test]
+fn a_hidden_component_name_keeps_the_dim_text_floor_of_3_to_1() {
+    for dark in [false, true] {
+        for (selected, rows) in [(false, 0), (true, 1)] {
+            let (shapes, t) = drawn_hidden(dark, selected);
+            assert_eq!(t.dark, dark);
+            let col = name_colour(&shapes, "Button");
+            let all = [("the panel", t.panel), ("a hovered row", t.hover), ("the selected component row", t.component_row)];
+            // Unselected, the row shows the panel (or the hover); selected, its own tinted row.
+            for (what, bg) in &all[..2 + rows] {
+                let ratio = color(over(col, *bg)).contrast(color(*bg));
+                assert!(ratio >= 3.0, "hidden component name {col:?} is {ratio:.2}:1 on {what} (dark {dark}, selected {selected})");
+            }
+        }
+    }
+}

crates/ui-egui/src/lib.rs

@@ -138,6 +138,8 @@ mod inspect_fields_tests;
 #[cfg(test)]
 mod layers_behaviour_tests;
 #[cfg(test)]
+mod layers_component_rows_tests;
+#[cfg(test)]
 mod layers_drag_tests;
 #[cfg(test)]
 mod layers_keyboard_tests;

crates/ui-egui/src/panels/layers.rs

@@ -10,7 +10,7 @@ use serde_json::json;
 use crate::a11y::{self, Region, RowInfo, Tip};
 use crate::i18n::{tr, trf, trn};
 use crate::icons::{self, Icon};
-use crate::theme::{ACCENT, COMPONENT, Tokens, regular, semibold};
+use crate::theme::{ACCENT, Tokens, regular, semibold};
 use crate::{FigApp, LeftTab};
 
 pub const ROW: f32 = 32.0;
@@ -799,14 +799,22 @@ fn drag_autoscroll(ui: &mut Ui, dragging: bool) {
     }
 }
 
-/// A row's icon or name colour: the component tint (faded when hidden), else `normal` or `faded`.
-fn row_color(component: bool, shown: bool, normal: Color32, faded: Color32) -> Color32 {
-    if component {
-        COMPONENT.gamma_multiply(if shown { 1.0 } else { 0.5 })
-    } else if shown {
-        normal
-    } else {
+/// Whether a layer is a component, a variant set or an instance: its rows take the component tint.
+fn is_component(kind: &NodeKind) -> bool {
+    matches!(kind, NodeKind::Component { .. } | NodeKind::ComponentSet { .. } | NodeKind::Instance { .. })
+}
+
+/// A row's icon or name colour: the theme's component tint when shown (it reads at AA on the
+/// panel and on the selected row, where the canvas purple doesn't, #1903), else `normal` or, when
+/// hidden, `faded` (halving the tint's alpha would drop it to about 2.2:1, under the 3:1 floor
+/// for dim text).
+fn row_color(t: &Tokens, component: bool, shown: bool, normal: Color32, faded: Color32) -> Color32 {
+    if !shown {
         faded
+    } else if component {
+        t.component
+    } else {
+        normal
     }
 }
 
@@ -1042,13 +1050,14 @@ fn hierarchy_bg(roots: &[Option<NodeId>], i: usize, mut rr: Rect, gap: f32) -> O
 }
 
 /// A row's background: selected, inside a selected layer, or outlined when hovered.
-/// `state` is [selected, in_selected, component].
+/// `state` is [selected, in_selected, component]: a selected component's row takes the component
+/// tint, not the plain selection's green; the rows inside it are tinted by the caller (#1903).
 fn paint_row_bg(ui: &Ui, t: &Tokens, rr: Rect, state: [bool; 3], hovered: bool) {
     let [selected, in_selected, is_comp] = state;
     if selected {
         ui.painter().rect_filled(rr, CornerRadius::same(5), if is_comp { t.component_row } else { t.selected_row });
     } else if !in_selected && hovered {
-        ui.painter().rect_stroke(rr, CornerRadius::same(5), Stroke::new(1.0, if is_comp { COMPONENT } else { ACCENT }), egui::StrokeKind::Inside);
+        ui.painter().rect_stroke(rr, CornerRadius::same(5), Stroke::new(1.0, if is_comp { t.component } else { ACCENT }), egui::StrokeKind::Inside);
     }
 }
 
@@ -1183,12 +1192,14 @@ fn layers(app: &mut FigApp, ui: &mut Ui) {
                 locked: n.locked,
             };
             a11y::tree_item(&resp, if n.name.is_empty() { tr(n.kind.label()) } else { &n.name }, &info);
-            let is_comp = matches!(n.kind, NodeKind::Component { .. } | NodeKind::ComponentSet { .. } | NodeKind::Instance { .. });
+            let is_comp = is_component(&n.kind);
             let rr = r.shrink2(vec2(8.0, 0.0));
             let bg = hierarchy_bg(&roots, i - roots_start, rr, pitch - ROW);
             let in_selected = !selected && bg.is_some();
             if let Some((rect, corners)) = bg {
-                ui.painter().rect_filled(rect, corners, t.selected_row_child);
+                // Inside a selected component, variant set or instance the rows take its tint.
+                let in_comp = roots.get(i - roots_start).copied().flatten().and_then(|r| doc.node(r)).is_some_and(|r| is_component(&r.kind));
+                ui.painter().rect_filled(rect, corners, if in_comp { t.component_row_child } else { t.selected_row_child });
             }
             a11y::ring_around(ui, rr, &resp);
             // Over the row, its eye and padlock included (they sit on top of it).
@@ -1215,7 +1226,7 @@ fn layers(app: &mut FigApp, ui: &mut Ui) {
             }
             let icon = node_icon(&doc, row.id);
             let shown = n.visible && !row.dimmed;
-            let icon_col = row_color(is_comp || icon == Icon::Slot, shown, t.icon, t.text_tertiary);
+            let icon_col = row_color(&t, is_comp || icon == Icon::Slot, shown, t.icon, t.text_tertiary);
             icons::paint(ui.painter(), Rect::from_center_size(pos2(x0 + 22.0, r.center().y), vec2(14.0, 14.0)), icon, icon_col);
             let name_x = x0 + 36.0;
             if app.ui.rename.is_some_and(|rn| rn.id == row.id) {
@@ -1223,7 +1234,7 @@ fn layers(app: &mut FigApp, ui: &mut Ui) {
                     rename_done = Some((row.id, new));
                 }
             } else {
-                let col = row_color(is_comp, shown, t.text, t.text_tertiary);
+                let col = row_color(&t, is_comp, shown, t.text, t.text_tertiary);
                 let font = name_font(&doc, page, n);
                 let name = row_name(n).to_string();
                 // Room left of the lock and eye buttons (the row is wide enough for the whole

crates/ui-egui/src/theme.rs

@@ -29,8 +29,15 @@ pub struct Tokens {
     pub accent_text: Color32,
     pub selected_row: Color32,
     pub selected_row_child: Color32,
+    /// Component, variant and instance names and icons in the panels: at least 4.5:1 (WCAG AA
+    /// for text) on the panel, on a hovered row and on `component_row`. The canvas keeps
+    /// [`COMPONENT`] for its outlines and labels.
     pub component: Color32,
+    /// A selected component's row in the Layers panel.
     pub component_row: Color32,
+    /// The rows inside a selected component: a tone of its row (`selected_row_child` is the plain
+    /// selection's).
+    pub component_row_child: Color32,
     pub danger: Color32,
     pub success: Color32,
     pub warning: Color32,
@@ -101,7 +108,9 @@ impl Tokens {
             text: Color32::from_rgb(0x1E, 0x1E, 0x1E),
             // 4.7:1 on fields and hovered rows (#F5F5F5), 4.6:1 on selected rows: WCAG AA.
             text_secondary: Color32::from_rgb(0x6E, 0x6E, 0x6E),
-            text_tertiary: Color32::from_rgb(0x8A, 0x8A, 0x8A),
+            // Dim text: 3.2:1 on the selected component row (#F1E9FF, the tightest of the
+            // rows it lands on; #8A8A8A was 2.93:1 there), still dimmer than secondary text.
+            text_tertiary: Color32::from_rgb(0x84, 0x84, 0x84),
             icon: Color32::from_rgb(0x1E, 0x1E, 0x1E),
             icon_secondary: Color32::from_rgb(0x75, 0x75, 0x75),
             hover: Color32::from_rgb(0xF5, 0xF5, 0xF5),
@@ -115,8 +124,11 @@ impl Tokens {
             accent_text: Color32::from_rgb(0x00, 0x7A, 0x56),
             selected_row: Color32::from_rgb(0xE0, 0xFB, 0xF1),
             selected_row_child: Color32::from_rgb(0xF1, 0xFC, 0xF7),
-            component: COMPONENT,
+            // A shade darker than the canvas purple: 5.5:1 on white, 4.7:1 on the selected
+            // component row and 5.0:1 on a hovered row (the canvas purple reaches 3.8:1 and 4.1:1).
+            component: Color32::from_rgb(0x86, 0x38, 0xF0),
             component_row: Color32::from_rgb(0xF1, 0xE9, 0xFF),
+            component_row_child: Color32::from_rgb(0xF8, 0xF4, 0xFF),
             danger: Color32::from_rgb(0xF2, 0x48, 0x22),
             success: DEV_READY,
             warning: Color32::from_rgb(0xFF, 0xCD, 0x29),
@@ -155,8 +167,11 @@ impl Tokens {
             accent_text: Color32::from_rgb(0x52, 0xE9, 0xB7),
             selected_row: Color32::from_rgb(0x16, 0x42, 0x36),
             selected_row_child: Color32::from_rgb(0x12, 0x37, 0x2D),
+            // 5.4:1 on the panel, 4.6:1 on the selected component row, 4.5:1 on a hovered row
+            // (the canvas purple reaches 3.1:1 and 2.7:1, #1903).
             component: Color32::from_rgb(0xD2, 0x7C, 0xFF),
             component_row: Color32::from_rgb(0x3E, 0x30, 0x52),
+            component_row_child: Color32::from_rgb(0x36, 0x2B, 0x47),
             danger: Color32::from_rgb(0xF2, 0x48, 0x22),
             success: DEV_READY,
             warning: Color32::from_rgb(0xFF, 0xCD, 0x29),
@@ -494,7 +509,7 @@ mod tests {
             let theme = if t.dark { "dark" } else { "light" };
             // Disabled text (a disabled button's label, placeholders) on panels, fields and
             // selected rows, still dimmer than secondary text.
-            for bg in [t.panel, t.field, t.hover, t.selected_row, t.selected_row_child] {
+            for bg in [t.panel, t.field, t.hover, t.selected_row, t.selected_row_child, t.component_row, t.component_row_child] {
                 let ratio = c(t.text_tertiary).contrast(c(bg));
                 assert!(ratio >= 3.0, "{theme}: dim text {:?} on {bg:?} is {ratio:.2}:1", t.text_tertiary);
                 assert!(ratio < c(t.text_secondary).contrast(c(bg)), "{theme}: dim text reads dimmer than secondary text");