From ef96419bec78d541b8699def1fae219178fd7e86 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 21:29:46 +0000 Subject: [PATCH] Appearance: the Light theme repaints Effect Controls headers, Timeline wells and the expression field (#644) Effect Controls painted effect headers with fixed dark fills (#2a2a2a, #2f3a52 when active) under the theme's text colour, so on Light the effect names were dark on dark (1.1:1). The Timeline kept the selected layer's time-graph row, the A/V and switch wells, the time navigator, the work area bar and the expression field on fixed dark fills too. Those surfaces are now theme tokens (fx_header, fx_header_active, tl_graph_selected, switch_well, tl_nav_bar, tl_work_area_bar, expr_bg, expr_text) with the dark values unchanged and light ones for Light. The expression editor's syntax colours come from the theme as well: on the light field a darker set keeps every category, enabled/disabled expressions, matched brackets and line numbers at 4.5:1 or better (contrast test, with a negative control showing the dark set fails there). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f --- crates/ui-egui/src/panels/effect_controls.rs | 2 +- crates/ui-egui/src/panels/expr_editor.rs | 133 ++++++++++++++++--- crates/ui-egui/src/panels/timeline.rs | 29 ++-- crates/ui-egui/src/theme.rs | 36 +++++ 4 files changed, 161 insertions(+), 39 deletions(-) diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index fb388845..8df4447b 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1102,7 +1102,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let open = !app.ui.fx_closed.contains(&g.uid); let selected = app.session.state.selected_props.iter().any(|(l, u)| *l == layer.id && *u == g.uid); let active = effect_active(g, layer.id, &app.session.state.selected_props); - bp.rect_filled(r, 0.0, if active { Color32::from_rgb(0x2f, 0x3a, 0x52) } else { Color32::from_rgb(0x2a, 0x2a, 0x2a) }); + bp.rect_filled(r, 0.0, if active { t.fx_header_active } else { t.fx_header }); let fxr = Rect::from_center_size(pos2(r.min.x + 14.0, r.center().y), vec2(16.0, 16.0)); if widgets::icon_toggle(ui, fxr, Icon::Fx, g.enabled, &t, egui::Id::new(("ec-fx", g.uid)), Sense::click()).clicked() { actions.push(("effect.toggle".into(), json!({"layer": layer.id.0, "effect": g.uid}))); diff --git a/crates/ui-egui/src/panels/expr_editor.rs b/crates/ui-egui/src/panels/expr_editor.rs index 1efd6876..ffb61738 100644 --- a/crates/ui-egui/src/panels/expr_editor.rs +++ b/crates/ui-egui/src/panels/expr_editor.rs @@ -9,13 +9,42 @@ use egui::{Color32, FontId, Rect, Stroke, pos2, vec2}; use crate::theme::Tokens; -/// Syntax colours. -const KEYWORD: Color32 = Color32::from_rgb(0xc6, 0x8a, 0xe6); -const NUMBER: Color32 = Color32::from_rgb(0xe0, 0xb4, 0x6c); -const STRING: Color32 = Color32::from_rgb(0x9c, 0xd0, 0x7a); -const COMMENT: Color32 = Color32::from_rgb(0x80, 0x88, 0x90); -const API: Color32 = Color32::from_rgb(0x6c, 0xb8, 0xf0); -const BRACKET_BG: Color32 = Color32::from_rgb(0x4a, 0x55, 0x6a); +/// Syntax colours, and the matched brackets' background. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct Syntax { + pub keyword: Color32, + pub number: Color32, + pub string: Color32, + pub comment: Color32, + pub api: Color32, + pub bracket_bg: Color32, +} + +impl Syntax { + /// The colours for the theme: on the Light theme's light expression field, darker ones that + /// stay readable (#644). + pub fn for_tokens(t: &Tokens) -> Syntax { + if t.kind.is_light() { + Syntax { + keyword: Color32::from_rgb(0x7a, 0x2f, 0xa8), + number: Color32::from_rgb(0x8a, 0x4b, 0x00), + string: Color32::from_rgb(0x2c, 0x6a, 0x10), + comment: Color32::from_rgb(0x52, 0x5a, 0x62), + api: Color32::from_rgb(0x0a, 0x57, 0x94), + bracket_bg: Color32::from_rgb(0xd0, 0xdc, 0xf0), + } + } else { + Syntax { + keyword: Color32::from_rgb(0xc6, 0x8a, 0xe6), + number: Color32::from_rgb(0xe0, 0xb4, 0x6c), + string: Color32::from_rgb(0x9c, 0xd0, 0x7a), + comment: Color32::from_rgb(0x80, 0x88, 0x90), + api: Color32::from_rgb(0x6c, 0xb8, 0xf0), + bracket_bg: Color32::from_rgb(0x4a, 0x55, 0x6a), + } + } + } +} const KEYWORDS: &[&str] = &[ "var", @@ -184,7 +213,7 @@ pub fn completions(text: &str, cursor: usize) -> (String, Vec<&'static str>) { } /// Colour runs for JavaScript: (byte start, byte end, colour). -pub fn highlight(text: &str, base: Color32) -> Vec<(usize, usize, Color32)> { +pub fn highlight(text: &str, base: Color32, sx: &Syntax) -> Vec<(usize, usize, Color32)> { let b = text.as_bytes(); let mut out = vec![]; let mut i = 0; @@ -195,35 +224,35 @@ pub fn highlight(text: &str, base: Color32) -> Vec<(usize, usize, Color32)> { while i < b.len() && b[i] != b'\n' { i += 1; } - COMMENT + sx.comment } else if c == b'/' && b.get(i + 1) == Some(&b'*') { i += 2; while i < b.len() && !(b[i] == b'*' && b.get(i + 1) == Some(&b'/')) { i += 1; } i = (i + 2).min(b.len()); - COMMENT + sx.comment } else if c == b'"' || c == b'\'' || c == b'`' { i += 1; while i < b.len() && b[i] != c { i += if b[i] == b'\\' { 2 } else { 1 }; } i = (i + 1).min(b.len()); - STRING + sx.string } else if c.is_ascii_digit() { while i < b.len() && (b[i].is_ascii_alphanumeric() || b[i] == b'.') { i += 1; } - NUMBER + sx.number } else if c.is_ascii_alphabetic() || c == b'_' || c == b'$' { while i < b.len() && (b[i].is_ascii_alphanumeric() || b[i] == b'_' || b[i] == b'$') { i += 1; } let w = &text[start..i]; if KEYWORDS.contains(&w) { - KEYWORD + sx.keyword } else if COMPLETIONS.contains(&w) { - API + sx.api } else { base } @@ -246,9 +275,9 @@ fn byte_of(text: &str, ci: usize) -> usize { } /// The layout job of the editor text: syntax colours, the matched brackets' background, wrap. -fn layout(text: &str, font: FontId, base: Color32, p: &Scripting, brackets: Option<(usize, usize)>, wrap: f32) -> LayoutJob { +fn layout(text: &str, font: FontId, base: Color32, sx: &Syntax, p: &Scripting, brackets: Option<(usize, usize)>, wrap: f32) -> LayoutJob { let mut job = LayoutJob::default(); - let runs = if p.syntax_highlighting { highlight(text, base) } else { vec![(0, text.len(), base)] }; + let runs = if p.syntax_highlighting { highlight(text, base, sx) } else { vec![(0, text.len(), base)] }; let marks: Vec = brackets.filter(|_| p.bracket_matching).map(|(a, b)| vec![byte_of(text, a), byte_of(text, b)]).unwrap_or_default(); for (s, e, col) in runs { // Split runs at the marked brackets. @@ -263,7 +292,7 @@ fn layout(text: &str, font: FontId, base: Color32, p: &Scripting, brackets: Opti cuts.sort_unstable(); cuts.dedup(); for w in cuts.windows(2) { - let bg = if marks.contains(&w[0]) { BRACKET_BG } else { Color32::TRANSPARENT }; + let bg = if marks.contains(&w[0]) { sx.bracket_bg } else { Color32::TRANSPARENT }; job.append(&text[w[0]..w[1]], 0.0, TextFormat { font_id: font.clone(), color: col, background: bg, ..Default::default() }); } } @@ -281,8 +310,9 @@ pub fn editor(ui: &mut egui::Ui, id: egui::Id, buf: &mut String, rect: Rect, p: let cursor = egui::TextEdit::load_state(ui.ctx(), id).and_then(|s| s.cursor.char_range()).map(|r| r.primary.index.0); let brackets = cursor.and_then(|c| matching_bracket(buf, c)); let pc = p.clone(); + let sx = Syntax::for_tokens(t); let mut layouter = move |ui: &egui::Ui, text: &dyn egui::TextBuffer, wrap: f32| { - let job = layout(text.as_str(), font.clone(), color, &pc, brackets, wrap); + let job = layout(text.as_str(), font.clone(), color, &sx, &pc, brackets, wrap); ui.fonts_mut(|f| f.layout_job(job)) }; let mut child = ui.new_child(egui::UiBuilder::new().max_rect(er)); @@ -392,3 +422,70 @@ pub fn error_banner(app: &mut crate::EffectcraftApp, ui: &egui::Ui, area: Rect) } } } + +#[cfg(test)] +mod contrast_tests { + use super::*; + use crate::theme::ThemeKind; + + /// WCAG contrast ratio of two opaque sRGB colours. + fn contrast(a: Color32, b: Color32) -> f32 { + let lum = |c: Color32| { + let f = |v: u8| { + let v = v as f32 / 255.0; + if v <= 0.04045 { v / 12.92 } else { ((v + 0.055) / 1.055).powf(2.4) } + }; + 0.2126 * f(c.r()) + 0.7152 * f(c.g()) + 0.0722 * f(c.b()) + }; + let (x, y) = (lum(a), lum(b)); + (x.max(y) + 0.05) / (x.min(y) + 0.05) + } + + /// #644: on every theme, effect names stay readable on Effect Controls' headers, and the + /// expression field's text (every syntax colour, enabled and disabled expressions, matched + /// brackets, line numbers) on its background; the switch icons show on their wells. + #[test] + fn custom_painted_surfaces_keep_text_readable_on_every_theme() { + for kind in [ThemeKind::Dark, ThemeKind::Darker, ThemeKind::Light] { + let t = Tokens::for_kind(kind); + let sx = Syntax::for_tokens(&t); + for (what, bg) in [("effect header", t.fx_header), ("active effect header", t.fx_header_active)] { + assert!(contrast(t.text, bg) >= 4.5, "{kind:?}: effect name on {what}: {:.2}", contrast(t.text, bg)); + } + assert!(contrast(t.icon, t.switch_well) >= 3.0, "{kind:?}: switch icons on their wells"); + if !kind.is_light() { + continue; + } + let fg = [ + ("keyword", sx.keyword), + ("number", sx.number), + ("string", sx.string), + ("comment", sx.comment), + ("api", sx.api), + ("enabled expression", t.expr_text), + ("disabled expression", t.text_dim), + ("line numbers", t.text_faint), + ]; + for (what, c) in fg { + assert!(contrast(c, t.expr_bg) >= 4.5, "{kind:?}: {what} on the expression field: {:.2}", contrast(c, t.expr_bg)); + assert!(contrast(c, sx.bracket_bg) >= 4.0, "{kind:?}: {what} on a matched bracket: {:.2}", contrast(c, sx.bracket_bg)); + } + // The dark theme's syntax colours would not do on the light field. + let dark = Syntax::for_tokens(&Tokens::for_kind(ThemeKind::Dark)); + assert!([dark.keyword, dark.number, dark.string, dark.api].iter().all(|c| contrast(*c, t.expr_bg) < 4.5)); + } + } + + /// Syntax highlighting uses the theme's colours. + #[test] + fn highlight_uses_the_theme_palette() { + let t = Tokens::for_kind(ThemeKind::Light); + let sx = Syntax::for_tokens(&t); + let text = "var amount = 12; // note"; + let runs = highlight(text, t.expr_text, &sx); + let col = |s: &str| runs.iter().find(|(a, e, _)| text.get(*a..*e) == Some(s)).map(|r| r.2); + assert_eq!(col("var"), Some(sx.keyword)); + assert_eq!(col("12"), Some(sx.number)); + assert_eq!(col("// note"), Some(sx.comment)); + } +} diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index b28c0d23..a5305df9 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -58,11 +58,6 @@ fn classic_geometry_row(comp: &Comp, l: &Layer) -> bool { comp.renderer == effectcraft_engine::project::Renderer::Classic3D && l.is_3d() && matches!(l.source, LayerSource::Text | LayerSource::Shape) } -/// Time navigator's visible-span bar and the work area bar. -const NAV_BAR: Color32 = Color32::from_rgb(0x55, 0x55, 0x55); -const WORK_AREA_BAR: Color32 = Color32::from_rgb(0x5c, 0x5c, 0x5c); -/// Fill of the timeline's switch / A/V boxes (dark wells, as in After Effects). -const SWITCH_BOX: Color32 = Color32::from_rgb(0x19, 0x19, 0x19); /// Background of the selected layer's name cell. const SELECTED_NAME_BG: Color32 = Color32::from_rgb(0xa6, 0xa6, 0xa6); @@ -1335,7 +1330,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let vis1 = (tm.t(graph_x1) / comp.duration.seconds()) as f32; let nav_vis = Rect::from_min_max(pos2(nav.min.x + nav.width() * vis0.clamp(0.0, 1.0), nav.min.y), pos2(nav.min.x + nav.width() * vis1.clamp(0.0, 1.0), nav.max.y)); - p.rect_filled(nav_vis, 0.0, NAV_BAR); + p.rect_filled(nav_vis, 0.0, t.tl_nav_bar); for x in [nav_vis.min.x, nav_vis.max.x] { p.rect_filled(Rect::from_min_max(pos2(x - 3.0, nav.min.y - 1.0), pos2(x + 3.0, nav.max.y + 1.0)), 3.0, t.accent); } @@ -1375,7 +1370,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let wa_row = Rect::from_min_max(pos2(graph_x0, top + header_h), pos2(rect.max.x, top + header_h + colhdr_h)); let (wa0, wa1) = (comp.work_area.0.seconds(), comp.work_area.1.seconds()); let wa = Rect::from_min_max(pos2(tm.x(wa0), wa_row.min.y + 3.0), pos2(tm.x(wa1), wa_row.max.y - 6.0)); - p.with_clip_rect(wa_row).rect_filled(wa, 0.0, WORK_AREA_BAR); + p.with_clip_rect(wa_row).rect_filled(wa, 0.0, t.tl_work_area_bar); let shift = ui.input(|i| i.modifiers.shift); let bar = wa.intersect(wa_row); let bresp = ui.interact(bar, egui::Id::new("wa-bar"), Sense::click_and_drag()); @@ -1736,7 +1731,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // (outline and time graph alike), not as alternating stripes. let _ = ri; lp.rect_filled(left, 0.0, if is_sel { t.row_selected } else { t.row_alt }); - gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, if is_sel { Color32::from_rgb(0x2a, 0x2a, 0x2a) } else { t.tl_bg }); + gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, if is_sel { t.tl_graph_selected } else { t.tl_bg }); lp.line_segment([pos2(rect.min.x, r.max.y - 0.5), pos2(graph_x0, r.max.y - 0.5)], Stroke::new(1.0, t.app_bg)); gp.line_segment([pos2(graph_x0, r.max.y - 0.5), pos2(r.max.x, r.max.y - 0.5)], Stroke::new(1.0, t.app_bg)); // A/V features. @@ -1752,7 +1747,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if !applicable || !vis.av { continue; } - lp.rect_filled(br.shrink(0.5), 1.0, SWITCH_BOX); + lp.rect_filled(br.shrink(0.5), 1.0, t.switch_well); layer_switch(ui, &t, r, br, icon, on, egui::Id::new(("av", layer.id.0, i)), layer.id.0, name, &mut actions); app.auto.add(&format!("timeline.layer.{}.{name}", layer.id.0), br, name); } @@ -1878,7 +1873,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { continue; } let br = Rect::from_center_size(pos2(cw.switches + SW * i as f32 + SW / 2.0, cy), vec2(16.0, 16.0)); - lp.rect_filled(br.shrink(1.0), 1.0, SWITCH_BOX); + lp.rect_filled(br.shrink(1.0), 1.0, t.switch_well); layer_switch(ui, &t, r, br, icon, on, egui::Id::new(("sw", layer.id.0, i)), layer.id.0, name, &mut actions); app.auto.add(&format!("timeline.layer.{}.switch.{name}", layer.id.0), br, name); } @@ -2233,7 +2228,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let en_r = Rect::from_center_size(pos2(cw.switches + 10.0, cy), vec2(16.0, 16.0)); let en = ui.interact(en_r, egui::Id::new(("expr-en", uid)), Sense::click()).on_hover_text(crate::i18n::tr("Enable Expression")); lp.rect_stroke(en_r.shrink(1.0), 2.0, Stroke::new(1.0, t.separator), StrokeKind::Inside); - let expr_col = Color32::from_rgb(0xe8, 0x7c, 0x5c); + let expr_col = t.expr_text; lp.text( en_r.center(), Align2::CENTER_CENTER, @@ -2318,7 +2313,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } // The editor in the time-graph area. ui.set_clip_rect(right_clip); - gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, Color32::from_rgb(0x1a, 0x1a, 0x1a)); + gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, t.expr_bg); let er = Rect::from_min_max(pos2(graph_x0 + 8.0, r.min.y + 3.0), pos2(rect.max.x - 14.0, r.max.y - 3.0)); let mut buf: String = ctx.data(|d| d.get_temp(buf_id)).unwrap_or_else(|| ex.text.clone()); if let Some(after) = ctx.data_mut(|d| d.remove_temp::(expr_refocus_id(*uid))) { @@ -2437,13 +2432,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let is_tr_rot = layer.transform().and_then(|t| t.get("rotation")).is_some_and(|r| r.uid == prop.uid); let base = if is_tr_rot && prop.name == "Rotation" && layer.is_3d() { "Z Rotation".to_string() } else { prop.name.clone() }; let pname = if prop.has_expression() { format!("{base} =") } else { base }; - lp.text( - pos2(name_x, cy), - Align2::LEFT_CENTER, - &pname, - Tokens::ui(12.0), - if prop.has_expression() { Color32::from_rgb(0xe8, 0x7c, 0x5c) } else { t.text }, - ); + lp.text(pos2(name_x, cy), Align2::LEFT_CENTER, &pname, Tokens::ui(12.0), if prop.has_expression() { t.expr_text } else { t.text }); let name_rect = Rect::from_min_max(pos2(name_x, r.min.y), pos2(cw.switches - 2.0, r.max.y)); let name_resp = ui.interact(name_rect, egui::Id::new(("pname", uid)), Sense::click_and_drag()); app.auto.add(&format!("timeline.prop.{uid}.name"), name_rect, &prop.name); @@ -2488,7 +2477,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { Align2::LEFT_CENTER, format!("= {}", e.text.lines().next().unwrap_or("")), Tokens::mono(11.0), - Color32::from_rgb(0xe8, 0x7c, 0x5c), + t.expr_text, ); } // Keyframes. diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index febd1991..953bf66d 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -81,6 +81,20 @@ pub struct Tokens { pub timecode: Color32, pub danger: Color32, pub warning: Color32, + /// Effect Controls effect headers, and the header of the effect being worked on (it or one + /// of its properties selected). + pub fx_header: Color32, + pub fx_header_active: Color32, + /// Timeline: a selected layer's row in the time graph, the boxes behind the A/V and layer + /// switches, the time navigator and the work area bar. + pub tl_graph_selected: Color32, + pub switch_well: Color32, + pub tl_nav_bar: Color32, + pub tl_work_area_bar: Color32, + /// The Timeline's expression field and the colour of enabled expressions (their text and + /// the names of properties that have one). + pub expr_bg: Color32, + pub expr_text: Color32, /// Label colours (`Label::ALL` order), from Settings ▸ Labels. pub labels: [Color32; 17], pub radius: f32, @@ -153,6 +167,14 @@ impl Tokens { timecode: Color32::from_rgb(0x3d, 0x8f, 0xf5), danger: Color32::from_rgb(0xe0, 0x4a, 0x3c), warning: Color32::from_rgb(0xe8, 0x9a, 0x2c), + fx_header: Color32::from_rgb(0x2a, 0x2a, 0x2a), + fx_header_active: Color32::from_rgb(0x2f, 0x3a, 0x52), + tl_graph_selected: Color32::from_rgb(0x2a, 0x2a, 0x2a), + switch_well: Color32::from_rgb(0x19, 0x19, 0x19), + tl_nav_bar: Color32::from_rgb(0x55, 0x55, 0x55), + tl_work_area_bar: Color32::from_rgb(0x5c, 0x5c, 0x5c), + expr_bg: Color32::from_rgb(0x1a, 0x1a, 0x1a), + expr_text: Color32::from_rgb(0xe8, 0x7c, 0x5c), labels: default_labels(), radius: 6.0, radius_sm: 3.0, @@ -202,6 +224,16 @@ impl Tokens { pasteboard: Color32::from_rgb(0xa8, 0xa8, 0xa8), // Keyframes have no outline: dark enough to stand out on the light time graph. keyframe: Color32::from_rgb(0x6c, 0x6c, 0x6c), + // Custom-painted surfaces in the Light palette, so dark text stays readable on + // them (#644). + fx_header: Color32::from_rgb(0xd8, 0xd8, 0xd8), + fx_header_active: Color32::from_rgb(0xb0, 0xcf, 0xec), + tl_graph_selected: Color32::from_rgb(0xc2, 0xc2, 0xc2), + switch_well: Color32::from_rgb(0xf2, 0xf2, 0xf2), + tl_nav_bar: Color32::from_rgb(0x9a, 0x9a, 0x9a), + tl_work_area_bar: Color32::from_rgb(0x94, 0x94, 0x94), + expr_bg: Color32::from_rgb(0xf2, 0xf2, 0xf2), + expr_text: Color32::from_rgb(0xa3, 0x34, 0x17), ..dark }, } @@ -268,6 +300,10 @@ impl Tokens { &mut self.tl_ruler_bg, &mut self.pasteboard, &mut self.work_area, + &mut self.fx_header, + &mut self.tl_graph_selected, + &mut self.switch_well, + &mut self.expr_bg, ] { adj(c); }