From 43e6cba2ef87a7df3473826a48d2a38613ef0d87 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:26:22 +0200 Subject: [PATCH] Keep committed line height and letter spacing in the text style editor Fixes #438 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/style_picker.rs | 5 ++-- crates/ui-egui/src/style_picker_tests.rs | 37 ++++++++++++++++++++++++ crates/ui-egui/src/typography.rs | 17 +++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/style_picker.rs b/crates/ui-egui/src/style_picker.rs index 3a4e7b11..3b405f1b 100644 --- a/crates/ui-egui/src/style_picker.rs +++ b/crates/ui-egui/src/style_picker.rs @@ -733,12 +733,13 @@ fn text_properties(ui: &mut Ui, id: &str, def: &TextStyle, out: &mut Frame) { }); ui.horizontal(|ui| { if let Some(v) = widgets::text_field(ui, Id::new(("ts_lh", id)), &def.line_height.label(), half, tr("Line height")) - && let Some(lh) = crate::typography::parse_line_height(&v, Some(def.line_height)).and_then(|v| serde_json::from_value(v).ok()) + && let Some(lh) = crate::typography::parse_line_height(&v, Some(def.line_height)).and_then(|v| crate::typography::line_height_of(&v)) { set(out, &|d| d.line_height = lh); } if let Some(v) = widgets::text_field(ui, Id::new(("ts_ls", id)), &def.letter_spacing.label(), half, tr("Letter spacing")) - && let Some(ls) = crate::typography::parse_letter_spacing(&v, Some(def.letter_spacing)).and_then(|v| serde_json::from_value(v).ok()) + && let Some(ls) = + crate::typography::parse_letter_spacing(&v, Some(def.letter_spacing)).and_then(|v| crate::typography::letter_spacing_of(&v)) { set(out, &|d| d.letter_spacing = ls); } diff --git a/crates/ui-egui/src/style_picker_tests.rs b/crates/ui-egui/src/style_picker_tests.rs index 6567e426..becc42ce 100644 --- a/crates/ui-egui/src/style_picker_tests.rs +++ b/crates/ui-egui/src/style_picker_tests.rs @@ -211,3 +211,40 @@ fn edit_local_for_test(app: &mut FigApp, ctx: &egui::Context) { let mut out = ctx.run_ui(RawInput::default(), |ui| edit_local(app, ui, StyleSlot::Fill, "Danger")); out.textures_delta.clear(); } + +/// Commits `typed` in the text style editor's field `field` ("ts_lh" / "ts_ls") with Enter and +/// returns the redefined style, if any. +fn commit_text_field(def: &TextStyle, field: &str, typed: &str) -> Option { + let ctx = egui::Context::default(); + ctx.data_mut(|d| d.insert_temp(Id::new((field, "S")).with("editing"), typed.to_string())); + let enter = egui::Event::Key { key: egui::Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }; + let input = RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(400.0, 400.0))), events: vec![enter], ..Default::default() }; + let mut f = Frame { act: None, hover: None, budget: 0 }; + let mut out = ctx.run_ui(input, |ui| text_properties(ui, "S", def, &mut f)); + out.textures_delta.clear(); + match f.act { + Some(Act::SetText(id, d)) if id == "S" => Some(d), + _ => None, + } +} + +#[test] +fn text_style_editor_commits_line_height_and_letter_spacing() { + use figcraft_doc::{LetterSpacing, LineHeight}; + let def = TextStyle { line_height: LineHeight::Auto, letter_spacing: LetterSpacing::Percent(0.0), ..Default::default() }; + let lh = |def: &TextStyle, s: &str| commit_text_field(def, "ts_lh", s).map(|d| d.line_height); + let ls = |def: &TextStyle, s: &str| commit_text_field(def, "ts_ls", s).map(|d| d.letter_spacing); + assert_eq!(lh(&def, "150%"), Some(LineHeight::Percent(150.0))); + assert_eq!(lh(&def, "24px"), Some(LineHeight::Pixels(24.0))); + assert_eq!(lh(&def, "24"), Some(LineHeight::Pixels(24.0))); + let pct = TextStyle { line_height: LineHeight::Percent(120.0), ..def.clone() }; + assert_eq!(lh(&pct, "130"), Some(LineHeight::Percent(130.0)), "a bare number keeps percent"); + assert_eq!(lh(&pct, "Auto"), Some(LineHeight::Auto)); + assert_eq!(ls(&def, "2%"), Some(LetterSpacing::Percent(2.0))); + assert_eq!(ls(&def, "1.5px"), Some(LetterSpacing::Pixels(1.5))); + assert_eq!(ls(&def, "3"), Some(LetterSpacing::Percent(3.0))); + // Only the edited property changes. + let d = commit_text_field(&def, "ts_lh", "150%").expect("redefined"); + assert_eq!(TextStyle { line_height: def.line_height, ..d }, def); + assert_eq!(lh(&def, "abc"), None); +} diff --git a/crates/ui-egui/src/typography.rs b/crates/ui-egui/src/typography.rs index 873ac0f0..d1b0f03c 100644 --- a/crates/ui-egui/src/typography.rs +++ b/crates/ui-egui/src/typography.rs @@ -1026,6 +1026,23 @@ pub(crate) fn parse_letter_spacing(s: &str, cur: Option) -> Optio } } +/// A [`parse_line_height`] value as a [`LineHeight`]: "auto", "150%" or a number of pixels. +pub(crate) fn line_height_of(v: &Value) -> Option { + match v { + Value::String(s) if s.eq_ignore_ascii_case("auto") => Some(LineHeight::Auto), + Value::String(s) => s.strip_suffix('%')?.trim().parse().ok().filter(|p: &f64| p.is_finite()).map(LineHeight::Percent), + other => other.as_f64().filter(|v| v.is_finite()).map(LineHeight::Pixels), + } +} + +/// A [`parse_letter_spacing`] value as a [`LetterSpacing`]: "2%" or a number of pixels. +pub(crate) fn letter_spacing_of(v: &Value) -> Option { + match v { + Value::String(s) => s.strip_suffix('%')?.trim().parse().ok().filter(|p: &f64| p.is_finite()).map(LetterSpacing::Percent), + other => other.as_f64().filter(|v| v.is_finite()).map(LetterSpacing::Pixels), + } +} + // ---- Font picker ---- const RECENT_KEY: &str = "figcraft_recent_fonts";