collective
Browse / github.com/storytold/deckcraft / patch
Format: OK in the Font dialog keeps a preset or system text color
Format: OK in the Font dialog keeps a preset or system text color The Font dialog got a preset text color as its bare name, which dropped the color's transforms, and a system color as its last RGB value, which dropped the system color. Pressing OK sent that value back, so it replaced the run's color: a preset `red` at 50% luminance came back as plain red, and a system color became a fixed RGB color. What changes: - `color_command_value` in `crates/engine/src/cmd/format.rs` writes `{"preset": name}` and `{"system": name, "last": "#RRGGBB"}`. The color's transforms go in order under `mods`, in the form the model stores them, so repeats such as `[inv, inv]` a
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- dotCooCoo (imported from pull request #1345, open)
- Base
14f40c408c67d2cb43e30a47dd179aada6dba53a- Size
- +85 −10 in 4 files
- Review
- pending
- Shared
- 2026-10-28
- Id
e891e3b6068376cda831b631
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/format.rs crates/engine/src/cmd/mod.rs crates/ui-egui/src/dialogs.rs crates/ui-egui/src/font_dialog_tests.rs
@@ -590,23 +590,31 @@ fn font_edits(p: &Value) -> Result<FontEdits> { fn color_command_value(c: &deckcraft_model::ColorRef) -> Value { use deckcraft_model::ColorBase; let mut o = serde_json::Map::new();- for m in &c.mods {- if let Some(v) = m.value() {- o.insert(m.xml_name().to_string(), json!(v));- }+ // An ordered list, so the order of the transforms and repeated ones such as `[inv, inv]` are kept.+ if !c.mods.is_empty() {+ o.insert("mods".into(), json!(c.mods)); }+ // `#RRGGBB`, or `#RRGGBBAA` for a color that is not opaque.+ let hex = |c: &deckcraft_color::Rgba| if c.a == 255 { format!("#{}", c.hex()) } else { format!("#{}{:02X}", c.hex(), c.a) }; match &c.base {- ColorBase::Rgb { rgb } if o.is_empty() => json!(format!("#{}", rgb.hex())),+ ColorBase::Rgb { rgb } if o.is_empty() => json!(hex(rgb)), ColorBase::Rgb { rgb } => {- o.insert("rgb".into(), json!(format!("#{}", rgb.hex())));+ o.insert("rgb".into(), json!(hex(rgb))); Value::Object(o) } ColorBase::Scheme { slot } => { o.insert("scheme".into(), json!(slot.xml_name())); Value::Object(o) }- ColorBase::Preset { name } => json!(name),- ColorBase::System { last, .. } => json!(format!("#{}", last.hex())),+ ColorBase::Preset { name } => {+ o.insert("preset".into(), json!(name));+ Value::Object(o)+ }+ ColorBase::System { name, last } => {+ o.insert("system".into(), json!(name));+ o.insert("last".into(), json!(hex(last)));+ Value::Object(o)+ } } } @@ -1538,6 +1546,31 @@ mod tests { assert!(r.size.is_none() && r.fill.is_none()); } + #[test]+ fn font_dialog_ok_keeps_the_text_color() {+ use deckcraft_color::Rgba;+ let preset = ColorRef { base: ColorBase::Preset { name: "red".into() }, mods: vec![] }.with(ColorTransform::LumMod(50_000));+ let system = ColorRef { base: ColorBase::System { name: "windowText".into(), last: Rgba::rgb(0x12, 0x34, 0x56) }, mods: vec![] }+ .with(ColorTransform::Alpha(60_000));+ // A transform without a value, and a color that is not opaque.+ let inverted = ColorRef { base: ColorBase::Preset { name: "red".into() }, mods: vec![] }.with(ColorTransform::Inv);+ let translucent = ColorRef { base: ColorBase::System { name: "windowText".into(), last: Rgba::rgba(0x12, 0x34, 0x56, 0x80) }, mods: vec![] };+ let rgb = ColorRef::rgb(Rgba::rgba(0x12, 0x34, 0x56, 0x80));+ // A repeated transform, and two transforms in an order that is not alphabetical.+ let twice = inverted.clone().with(ColorTransform::Inv);+ let ordered = ColorRef::scheme(SchemeSlot::Accent2).with(ColorTransform::LumOff(20_000)).with(ColorTransform::LumMod(50_000));+ for color in [preset, system, inverted, translucent, rgb, twice, ordered] {+ let mut s = shape();+ let fill = Fill::Solid { color: color.clone() };+ format_run(&mut s, &|r| r.fill = Some(fill.clone())).unwrap();+ s.ui_requests.clear();+ s.execute("format.fontDialog", &json!({})).unwrap();+ let Some(UiRequest::Dialog { params, .. }) = s.ui_requests.last().cloned() else { panic!("dialog request") };+ s.execute("format.fontDialog", &json!({"color": params["color"]})).unwrap();+ assert_eq!(props(&s).fill, Some(Fill::Solid { color }), "{}", params["color"]);+ }+ }+ #[test] fn font_dialog_with_no_fields_asks_the_ui_to_open() { let mut s = shape();@@ -372,7 +372,8 @@ pub(crate) fn ok() -> Result<Value> { /// Parse a colour param: `"#RRGGBB"`, `"accent1"`…, or `{"scheme":"accent2","lumMod":75000}`. /// A preset or system colour keeps its base: `{"preset":"red"}`,-/// `{"system":"windowText","last":"#000000"}`.+/// `{"system":"windowText","last":"#000000"}`. `"mods"` lists transforms in order:+/// `{"preset":"red","mods":[{"op":"inv"},{"op":"inv"}]}`. pub(crate) fn color_param(p: &Value, key: &str) -> Option<deckcraft_model::ColorRef> { let v = p.get(key)?; color_value(v)@@ -412,6 +413,10 @@ pub(crate) fn color_value(v: &Value) -> Option<deckcraft_model::ColorRef> { c.mods.push(t); } }+ // `mods` lists transforms in order, repeats included: `[{"op":"inv"},{"op":"lumMod","val":50000}]`.+ if let Some(list) = o.get("mods") {+ c.mods.extend(serde_json::from_value::<Vec<deckcraft_color::ColorTransform>>(list.clone()).ok()?);+ } Some(c) } _ => None,@@ -669,5 +674,9 @@ mod tests { assert!(color_value(&json!({"preset": name})).is_some(), "{name}"); } assert!(color_value(&json!({"system": "3dDkShadow", "last": "#000000"})).is_some());+ let listed = color_value(&json!({"preset": "red", "mods": [{"op": "inv"}, {"op": "lumOff", "val": 20_000}, {"op": "inv"}]}));+ let mods = vec![ColorTransform::Inv, ColorTransform::LumOff(20_000), ColorTransform::Inv];+ assert_eq!(listed, Some(ColorRef { base: ColorBase::Preset { name: "red".into() }, mods }));+ assert_eq!(color_value(&json!({"preset": "red", "mods": [{"op": "notATransform"}]})), None); } }@@ -2896,7 +2896,7 @@ fn json_text(v: Option<&Value>, default: &str) -> String { fn color_label(v: &Value) -> String { match v { Value::String(s) => s.clone(),- Value::Object(o) => o.get("scheme").or_else(|| o.get("rgb")).and_then(Value::as_str).unwrap_or("").to_string(),+ Value::Object(o) => ["scheme", "rgb", "preset", "last"].iter().find_map(|k| o.get(*k)).and_then(Value::as_str).unwrap_or("").to_string(), _ => String::new(), } }@@ -63,3 +63,36 @@ fn command_t_opens_the_font_dialog_and_cancel_keeps_the_text() { assert!(h.state().dialog.is_none()); assert_eq!(bold_of(&h), None); }++#[test]+fn preset_and_system_colors_show_in_the_dialog_and_ok_keeps_them() {+ for (color, shown) in [+ (json!({"preset": "red", "lumMod": 50000}), "red"),+ (json!({"system": "windowText", "last": "#123456", "alpha": 60000}), "#123456"),+ (json!({"preset": "red", "mods": [{"op": "inv"}, {"op": "inv"}]}), "red"),+ (json!({"scheme": "accent2", "mods": [{"op": "lumOff", "val": 20000}, {"op": "lumMod", "val": 50000}]}), "accent2"),+ ] {+ let mut h = harness();+ h.state_mut().session.execute("format.fontDialog", &json!({ "color": color })).unwrap();+ let before = fill_of(&h);+ assert!(before.is_some());+ h.get_by_label("Font…").click();+ h.run_steps(2);+ let d = h.state().dialog.as_ref().expect("font dialog");+ assert_eq!(d.fields.get("color").map(String::as_str), Some(shown));+ h.get_by_label("OK").click_accesskit();+ h.run_steps(1);+ assert!(h.state().dialog.is_none(), "status {:?}", h.state().status);+ assert_eq!(fill_of(&h), before, "{shown}");+ }+}++fn fill_of(h: &Harness<'_, SlideApp>) -> Option<deckcraft_model::Fill> {+ let st = h.state().session.active().unwrap();+ st.selected_shapes()+ .first()+ .and_then(|sh| sh.text.as_ref())+ .and_then(|t| t.paragraphs.first())+ .and_then(|p| p.runs.first())+ .and_then(|r| r.props.fill.clone())+}