diff --git a/ROADMAP.md b/ROADMAP.md index e6d20d1e..f3240521 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~26 points and ~162–292 h away (one Opus 5.5 agent; ~33–67 h wall-clock with 5 agents) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (system theme on first run; recent files as cards with thumbnails on the home screen; community fixes: lost images reported on loading (#207), SVG import budgets (#190), Leptos code (#223), full screen (#227), server snapshots readable again (#188), owner-only preferences (#200), ruzstd 0.9.1 (#182), layer cycle repair (#205), batch rollback, Direct3D 12 on Windows, plugin metering (#192); vector edit mode regions: hover hatch, selection and drag; SVG/PDF exports refuse over-depth documents (#208); bounded MCP/control framing, connections and queues (#201); plugin log limits (#191); live-session revert guard (#198); attached MCP no longer replays after a lost reply (#194); repeat command recursion fix; the Layout panel's constrain-proportions link scales the other side, #220; core editor leftovers: component playground, auto gap handles, variables inside styles, inner shadows and background blur on groups, command-made text readable on dark pages, a one-step frame/group convert, delays on mouse triggers and manual overlay offsets, wrapped-line alignment; `.fig` export: instance sizes, variables with modes, shared styles, variable-width strokes; starter templates, a Templates tab and Getting started tips on the home screen; your online files; everyday editing partials: number fields, outline options, Hide when publishing, cursors, panel widths, dark pages; sharing with a join code; text box sizes measured against documents re-saved by other software; live sessions per document; .fig export checked by opening written files in other software; a public figcraft-server: open hosting, owner tokens, per-address limits, idle expiry, moderation, install.sh; experimental .fig export; keyboard shortcut parity suite and bindings; Simplified Chinese draft catalog; Latin American Spanish overlay; prototyping tail: gamepad triggers, widgets in prototypes, mode lookups; plugin directory on figcraft-server; Japanese and Korean draft catalogs and CJK UI fonts; plugin widgets and the plugin panel tail; French, German and Brazilian Portuguese draft catalogs; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and edit tail; dev resources links; extended collections tail; branch version history and finer list merges; headline percentages recomputed from the area table: feature breadth ~84%, core ~94%, mainstream ~50%; text lists, hanging punctuation and decoration styles; comments and version history behaviour suite and fixes; canvas navigation, guides and grids behaviour suite and fixes; export and inspect behaviour suite and fixes; .fig import: editable vector networks and more layer kinds; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; styles and variables behaviour suite and fixes; prototyping behaviour suite and fixes; components behaviour suite and fixes; embedded image colour profiles; layers panel keyboard: F2 rename, Shift+F10 context menu, keyboard box selection, 10k-layer QA; per-side stroke weight variables; text behaviour suite: CJK line breaking, metrics, vertical trim; selection and transform behaviour suite and fixes; `.fig` import, best effort; Windows QA pass with eight fixes; auto layout behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; localization layer with a draft Spanish catalog; hosting figcraft-server: wss in the desktop app, server releases, deployment guide; plugin panels and settings; canvas fixes: boolean hit testing on the result, group drags, frame outlines and slices; code connect; stretch and scatter brushes, pen pressure; text on a path and pattern fills; team libraries on the server and swap library; branches: create, update from main, review and merge; animated GIF fills and TIFF images; Open recent, outlines for the selection and mask outlines: menu breadth 100%; extended collections, variables import/export, gradient stop and variant bindings; feature breadth ~80%; ready ~49%) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-15 · **Change:** minor (Typography spacing icons support scrubbing (#373); system theme on first run; recent files as cards with thumbnails on the home screen; community fixes: lost images reported on loading (#207), SVG import budgets (#190), Leptos code (#223), full screen (#227), server snapshots readable again (#188), owner-only preferences (#200), ruzstd 0.9.1 (#182), layer cycle repair (#205), batch rollback, Direct3D 12 on Windows, plugin metering (#192); vector edit mode regions: hover hatch, selection and drag; SVG/PDF exports refuse over-depth documents (#208); bounded MCP/control framing, connections and queues (#201); plugin log limits (#191); live-session revert guard (#198); attached MCP no longer replays after a lost reply (#194); repeat command recursion fix; the Layout panel's constrain-proportions link scales the other side, #220; core editor leftovers: component playground, auto gap handles, variables inside styles, inner shadows and background blur on groups, command-made text readable on dark pages, a one-step frame/group convert, delays on mouse triggers and manual overlay offsets, wrapped-line alignment; `.fig` export: instance sizes, variables with modes, shared styles, variable-width strokes; starter templates, a Templates tab and Getting started tips on the home screen; your online files; everyday editing partials: number fields, outline options, Hide when publishing, cursors, panel widths, dark pages; sharing with a join code; text box sizes measured against documents re-saved by other software; live sessions per document; .fig export checked by opening written files in other software; a public figcraft-server: open hosting, owner tokens, per-address limits, idle expiry, moderation, install.sh; experimental .fig export; keyboard shortcut parity suite and bindings; Simplified Chinese draft catalog; Latin American Spanish overlay; prototyping tail: gamepad triggers, widgets in prototypes, mode lookups; plugin directory on figcraft-server; Japanese and Korean draft catalogs and CJK UI fonts; plugin widgets and the plugin panel tail; French, German and Brazilian Portuguese draft catalogs; cursor chat and spotlight in live sessions; layers and assets panels behaviour suite and fixes; vector multi-edit and edit tail; dev resources links; extended collections tail; branch version history and finer list merges; headline percentages recomputed from the area table: feature breadth ~84%, core ~94%, mainstream ~50%; text lists, hanging punctuation and decoration styles; comments and version history behaviour suite and fixes; canvas navigation, guides and grids behaviour suite and fixes; export and inspect behaviour suite and fixes; .fig import: editable vector networks and more layer kinds; shapes and vector behaviour suite and fixes; paint and effects behaviour suite and fixes; frames, groups, sections and masks behaviour suite and fixes; styles and variables behaviour suite and fixes; prototyping behaviour suite and fixes; components behaviour suite and fixes; embedded image colour profiles; layers panel keyboard: F2 rename, Shift+F10 context menu, keyboard box selection, 10k-layer QA; per-side stroke weight variables; text behaviour suite: CJK line breaking, metrics, vertical trim; selection and transform behaviour suite and fixes; `.fig` import, best effort; Windows QA pass with eight fixes; auto layout behaviour suite and fixes; Display P3: gamut warning and soft-proof (true P3 blocked upstream); several windows; localization layer with a draft Spanish catalog; hosting figcraft-server: wss in the desktop app, server releases, deployment guide; plugin panels and settings; canvas fixes: boolean hit testing on the result, group drags, frame outlines and slices; code connect; stretch and scatter brushes, pen pressure; text on a path and pattern fills; team libraries on the server and swap library; branches: create, update from main, review and merge; animated GIF fills and TIFF images; Open recent, outlines for the selection and mask outlines: menu breadth 100%; extended collections, variables import/export, gradient stop and variant bindings; feature breadth ~80%; ready ~49%) · **Target:** Figma Design (desktop app 126.10.7) FigCraft is a clean-room, open-source, pure-Rust reimplementation of the Figma design workflow, native on macOS, Windows, Linux and FreeBSD and in the browser, fully drivable by agents through @@ -111,7 +111,7 @@ native speakers' reviews of the six language drafts need people. | Korean | ko | 100% | draft (needs native review) | | Vietnamese | vi | 0% | none | -No other languages. Figma ships English, Japanese, Korean, French, German, Spanish and Portuguese +Also shipped: Swedish (`sv`: every UI string, 120 of 588 command labels and menu wordings; native review welcome). Figma ships English, Japanese, Korean, French, German, Spanish and Portuguese (Brazil); FigCraft has the localization layer, English and Spanish (Spain and Latin America), French, German, Brazilian Portuguese, Japanese, Korean and Simplified Chinese drafts (Preferences ▸ Language; Traditional Chinese has no catalog; CJK UI @@ -315,6 +315,7 @@ Ranked; detail and milestones in [`docs/roadmap.md`](docs/roadmap.md). | Date | Change | Summary | |---|---|---| | 2026-10-15 | minor | Dev Mode tail (M6, gap 16): import lines for connected components inside generated layer trees (definitions, slots and swapped-in instances too); `dev.compare {version}` and the compare view's Compare with menu compare with any saved version; behaviour suite (5 tests; 4 failed before the fixes): an inspect-only Dev Mode canvas with menus, shortcuts, nudges, opacity keys and paste refusing design edits, toolbar drawing tools returning to the design, Changed in the ready list (`inspect.readyForDev {status: changed}`), statuses only on sections and top-level frames; read-only plugins' panels from the Inspect tab. Dev Mode 96%/72% → 97%/76%, 0–1 h → 2–4 h (measurements kept on the canvas and a read-only layers panel counted as missing); header totals not recomputed | +| 2026-10-15 | minor | Typography spacing icons scrub with the shared number-field gesture, modifier rates and one Undo step per drag (#373). Units and selected-character targeting are preserved. Numbers unchanged. | | 2026-10-14 | minor | System theme on first run (M7): preferences without a theme default to System; the first frame reads the OS appearance directly, including when returning from a manual theme. Saved Light/Dark choices stay explicit; unavailable OS appearance falls back to Light. Numbers unchanged. | | 2026-10-14 | minor | Home screen recent files as cards (M7): each card shows the picture its file carries on its first page's colour, the file's icon, name and "Edited ... ago"; sort by name, creation or last change either way; a table view; a click selects, a double-click or Enter opens, arrow keys move; Open, Show in folder and Remove from recents on right-click. Numbers unchanged. | | 2026-10-14 | minor | Lost images reported on loading (#207): a `.figcraft` file whose image member is damaged, too large or missing no longer loads in silence; each lost image (up to 20, then a count) is listed in `repairs` on `file.open` with its hash and the reason, and the app shows it as a toast. Saving still writes the file without the lost images. Numbers unchanged | diff --git a/apps/figcraft-cli/src/files.rs b/apps/figcraft-cli/src/files.rs index 9b0f9b55..4fb485e1 100644 --- a/apps/figcraft-cli/src/files.rs +++ b/apps/figcraft-cli/src/files.rs @@ -38,7 +38,7 @@ fn write(path: &str, bytes: &[u8]) -> CliResult<()> { fn page_named(doc: &Document, name: Option<&str>) -> CliResult { match name { - None => doc.first_page().ok_or_else(|| CliError::Failed("the document has no pages".into())), + None => doc.first_open_page().or_else(|| doc.first_page()).ok_or_else(|| CliError::Failed("the document has no pages".into())), Some(n) => doc .pages .iter() @@ -380,6 +380,22 @@ mod tests { assert_eq!(sanitize(""), "layer"); } + #[test] + fn default_page_skips_leading_divider() { + let mut doc = Document::new("divider"); + let page = doc.first_page().unwrap(); + doc.create(page, figcraft_doc::NodeKind::Frame, "First", 0.0, 0.0, 40.0, 30.0).unwrap(); + let divider = doc.add_page("- Divider"); + doc.pages.retain(|p| *p != divider); + doc.pages.insert(0, divider); + assert!(doc.is_divider(divider)); + assert_eq!(page_named(&doc, None).unwrap(), page); + assert_eq!(page_named(&doc, Some("- Divider")).unwrap(), divider); + let pm = render_page(&doc, page_named(&doc, None).unwrap(), 1.0).unwrap(); + assert_eq!((pm.width(), pm.height()), (40, 30)); + assert!(render_page(&doc, divider, 1.0).is_err()); + } + #[test] fn export_depth_errors_preserve_conversion_destinations() { let mut doc = Document::new("export depth"); diff --git a/apps/figcraft-cli/src/perf.rs b/apps/figcraft-cli/src/perf.rs index c08ddc04..03190306 100644 --- a/apps/figcraft-cli/src/perf.rs +++ b/apps/figcraft-cli/src/perf.rs @@ -438,44 +438,66 @@ fn measure_components(table: &mut Table, col: usize, instances: usize) -> CliRes mean(3, |i| exec(&mut s, "node.set", json!({"ids": labels, "props": {"fontSize": 13 + (i % 2) * 2}}))), ); let doc = s.doc().map_err(failed)?.clone(); - let (card, _) = instance_with(&doc, &inst, ds.card, &["Footer button", "Icon", "Glyph"]).ok_or_else(|| failed("no card"))?; - table.put( - col, - "switch a card's variant (nested overrides kept)", - mean(5, |i| { - exec(&mut s, "instance.setProperties", json!({"ids": [card], "properties": {"Size": if i % 2 == 0 { "Large" } else { "Small" }}})) - }), - ); - let (nav, _) = instance_with(&doc, &inst, ds.nav, &["Profile"]).ok_or_else(|| failed("no nav"))?; - table.put(col, "reset all changes on a nav", timed(&mut s, "component.resetInstance", json!({"ids": [nav]}), true)); - table.put(col, "detach a nav", timed(&mut s, "component.detach", json!({"ids": [nav]}), true)); - let (button, bglyph) = instance_with(&doc, &inst, ds.button, &["Icon", "Glyph"]).ok_or_else(|| failed("no button"))?; - table.put(col, "push a button's changes to its main", timed(&mut s, "component.pushChanges", json!({"id": button}), true)); - // A button glyph the drop really lands on: screens can overlap (text sizes differ without - // fonts), so ask the hit test the swap uses. - let now = s.doc().map_err(failed)?.clone(); - let page = s.page().map_err(failed)?; - let b = std::iter::once(bglyph) - .chain(inst.iter().filter_map(|i| { - let NodeKind::Instance { component, .. } = &now.node(*i)?.kind else { return None }; - (figcraft_engine::components::variant_set(&now, *component) == Some(ds.button)).then(|| find_path(&now, *i, &["Icon", "Glyph"]))? - })) - .filter_map(|g| now.world_bounds(g)) - .find(|b| figcraft_engine::drop::swap_target(&now, page, b.center(), None, None).is_some()) - .ok_or_else(|| failed("no button glyph to drop on"))?; - let other = ds.icons.get(5).copied().unwrap_or(icon0); - table.put( - col, - "Alt-drop swap onto an instance", - timed(&mut s, "instance.swapAt", json!({"component": other, "x": b.center().x, "y": b.center().y}), true), - ); - exec(&mut s, "edit.copy", json!({"ids": [nav]})); - table.put(col, "copy + paste a nav", timed(&mut s, "edit.paste", json!({}), true)); + // Small workloads may not contain every instance type; the rows needing a missing one stay empty. + if let Some((card, _)) = instance_with(&doc, &inst, ds.card, &["Footer button", "Icon", "Glyph"]) { + table.put( + col, + "switch a card's variant (nested overrides kept)", + mean(5, |i| { + exec(&mut s, "instance.setProperties", json!({"ids": [card], "properties": {"Size": if i % 2 == 0 { "Large" } else { "Small" }}})) + }), + ); + } else { + table.put(col, "switch a card's variant (nested overrides kept)", f64::NAN); + } + let nav = instance_with(&doc, &inst, ds.nav, &["Profile"]).map(|(nav, _)| nav); + if let Some(nav) = nav { + table.put(col, "reset all changes on a nav", timed(&mut s, "component.resetInstance", json!({"ids": [nav]}), true)); + table.put(col, "detach a nav", timed(&mut s, "component.detach", json!({"ids": [nav]}), true)); + } else { + table.put(col, "reset all changes on a nav", f64::NAN); + table.put(col, "detach a nav", f64::NAN); + } + if let Some((button, bglyph)) = instance_with(&doc, &inst, ds.button, &["Icon", "Glyph"]) { + table.put(col, "push a button's changes to its main", timed(&mut s, "component.pushChanges", json!({"id": button}), true)); + // A button glyph the drop really lands on: screens can overlap (text sizes differ without + // fonts), so ask the hit test the swap uses. + let now = s.doc().map_err(failed)?.clone(); + let page = s.page().map_err(failed)?; + let drop_at = std::iter::once(bglyph) + .chain(inst.iter().filter_map(|i| { + let NodeKind::Instance { component, .. } = &now.node(*i)?.kind else { return None }; + (figcraft_engine::components::variant_set(&now, *component) == Some(ds.button)).then(|| find_path(&now, *i, &["Icon", "Glyph"]))? + })) + .filter_map(|g| now.world_bounds(g)) + .find(|b| figcraft_engine::drop::swap_target(&now, page, b.center(), None, None).is_some()); + let other = ds.icons.get(5).copied().unwrap_or(icon0); + table.put( + col, + "Alt-drop swap onto an instance", + match drop_at { + Some(b) => timed(&mut s, "instance.swapAt", json!({"component": other, "x": b.center().x, "y": b.center().y}), true), + None => f64::NAN, + }, + ); + } else { + table.put(col, "push a button's changes to its main", f64::NAN); + table.put(col, "Alt-drop swap onto an instance", f64::NAN); + } + if let Some(nav) = nav { + exec(&mut s, "edit.copy", json!({"ids": [nav]})); + table.put(col, "copy + paste a nav", timed(&mut s, "edit.paste", json!({}), true)); + } else { + table.put(col, "copy + paste a nav", f64::NAN); + } table.put(col, "undo (deep main edit)", mean(10, |_| _ = s.undo())); table.put(col, "redo", mean(10, |_| _ = s.redo())); // As a library: publish it, use it from another file, publish a change, accept the update. - let dir = std::env::temp_dir().join(format!("figcraft-perf-{}", std::process::id())); + // One directory per run, so concurrent runs in a process don't delete each other's files. + static RUNS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + let run = RUNS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("figcraft-perf-{}-{run}", std::process::id())); std::fs::create_dir_all(&dir).map_err(failed)?; let path = dir.join("design-system.figcraft").to_string_lossy().into_owned(); let (lib_doc, lib_ds) = design_system_document(&spec).map_err(failed)?; @@ -533,6 +555,14 @@ mod tests { assert!(t.rows.iter().all(|r| r.1.len() == 1 && r.1[0].is_finite()), "{:?}", t.rows); } + #[test] + fn measures_components_with_a_single_instance() { + let mut t = super::Table { rows: Vec::new() }; + super::measure_components(&mut t, 0, 1).unwrap(); + assert!(t.rows.iter().all(|r| r.1.len() == 1), "{:?}", t.rows); + assert!(t.rows.iter().any(|r| r.0.starts_with("build design system") && r.1[0].is_finite()), "{:?}", t.rows); + } + #[test] fn measures_a_small_document() { let mut t = super::Table { rows: Vec::new() }; diff --git a/apps/figcraft-server/src/store.rs b/apps/figcraft-server/src/store.rs index 4886bc88..538adeff 100644 --- a/apps/figcraft-server/src/store.rs +++ b/apps/figcraft-server/src/store.rs @@ -347,14 +347,14 @@ impl Store { let key = load_key(root)?; let dir = root.join(FILES); fs::create_dir_all(&dir).map_err(|e| format!("can't create {}: {e}", dir.display()))?; - let (directory, problems) = Directory::open(root, cfg.directory); + let (directory, startup_problems) = Directory::open(root, cfg.directory); let abuse = Abuse::new(cfg.hosting.abuse); let mut store = Store { root: root.to_path_buf(), cfg, key, files: BTreeMap::new(), - problems, + problems: Vec::new(), directory, abuse, served: figcraft_collab_net::net::Bucket::new(LIBRARY_RATE, LIBRARY_BURST), @@ -368,6 +368,9 @@ impl Store { free: None, _lock: lock, }; + for problem in startup_problems { + store.note(problem); + } store.scan(&dir); store.blocked = store.read_list::(BLOCKED).iter().filter_map(|s| s.parse().ok()).map(ip_key).collect(); store.reports = store.read_list(REPORTS); @@ -385,7 +388,7 @@ impl Store { match disk::read_capped(&path, 16 << 20).and_then(|b| serde_json::from_slice(&b).map_err(|e| format!("{name} is damaged ({e})"))) { Ok(list) => list, Err(e) => { - self.problems.push(e); + self.note(e); Vec::new() } } @@ -394,7 +397,7 @@ impl Store { fn scan(&mut self, dir: &Path) { let entries = match fs::read_dir(dir) { Ok(e) => e, - Err(e) => return self.problems.push(format!("can't list {}: {e}", dir.display())), + Err(e) => return self.note(format!("can't list {}: {e}", dir.display())), }; let mut ids: Vec<(String, PathBuf)> = entries.flatten().map(|e| (e.file_name().to_string_lossy().into_owned(), e.path())).collect(); ids.sort(); @@ -403,18 +406,18 @@ impl Store { continue; } if !valid_id(&id) { - self.problems.push(format!("skipped {}: not a file folder of this server", path.display())); + self.note(format!("skipped {}: not a file folder of this server", path.display())); continue; } if self.files.len() >= self.cfg.max_files { - self.problems.push(format!("skipped file {id}: over the {}-file cap", self.cfg.max_files)); + self.note(format!("skipped file {id}: over the {}-file cap", self.cfg.max_files)); continue; } match self.check(&path) { Ok(entry) => { self.files.insert(id, entry); } - Err(why) => self.problems.push(format!("skipped file {id}: {why}")), + Err(why) => self.note(format!("skipped file {id}: {why}")), } } } diff --git a/apps/figcraft-server/tests/problem_retention.rs b/apps/figcraft-server/tests/problem_retention.rs new file mode 100644 index 00000000..221eaa6e --- /dev/null +++ b/apps/figcraft-server/tests/problem_retention.rs @@ -0,0 +1,40 @@ +use figcraft_server::config::Config; +use figcraft_server::store::{FILES, Store}; + +#[test] +fn startup_problem_retention() { + let root = std::env::temp_dir().join(format!("figcraft-problem-cap-{}", std::process::id())); + assert!(!root.exists()); + let result = (|| -> Result<(), String> { + for count in [3u32, 200, 201] { + let data = root.join(count.to_string()); + for i in 0..count { + std::fs::create_dir_all(data.join(FILES).join(format!("{i:016x}"))).map_err(|e| e.to_string())?; + } + let cfg = Config { open_publish: true, data: data.clone(), ..Config::default() }; + cfg.check()?; + let store = Store::open(&data, cfg.store())?; + let expected = (count as usize).min(200); + let problems = store.problems().to_vec(); + let no_loaded_files = store.is_empty(); + drop(store); + if !problems.iter().all(|p| p.contains("meta.json")) || !no_loaded_files { + return Err("unexpected diagnostic or loaded file".into()); + } + if problems.len() != expected { + return Err(format!("expected {expected} retained problems, got {}", problems.len())); + } + // The latest problems stay, in scan order: the first folder's is the one dropped. + let first_kept = count as usize - expected; + for (n, p) in problems.iter().enumerate() { + let id = format!("{:016x}", first_kept + n); + if !p.contains(&id) { + return Err(format!("problem {n} should be about {id}: {p}")); + } + } + } + Ok(()) + })(); + std::fs::remove_dir_all(&root).unwrap(); + assert!(result.is_ok(), "{}", result.unwrap_err()); +} diff --git a/apps/figcraft/src/control_server.rs b/apps/figcraft/src/control_server.rs index 995240f5..1c91840f 100644 --- a/apps/figcraft/src/control_server.rs +++ b/apps/figcraft/src/control_server.rs @@ -111,7 +111,13 @@ fn serve(mut out: TcpStream, queue: Queue, ctx: CtxSlot, timeout: Duration) -> s } } } - Err(e) => json!({"id": null, "ok": false, "error": format!("bad JSON: {e}")}), + Err(e) => { + // A line that isn't JSON ends the connection: a web page can make the browser send + // an HTTP request here, and its header lines must not let the JSON lines of its body + // through as commands. + writeln!(out, "{}", json!({"id": null, "ok": false, "error": format!("bad JSON: {e}")}))?; + break; + } }; writeln!(out, "{reply}")?; } diff --git a/apps/figcraft/src/control_server_tests.rs b/apps/figcraft/src/control_server_tests.rs index 2e0a7b48..d243fb91 100644 --- a/apps/figcraft/src/control_server_tests.rs +++ b/apps/figcraft/src/control_server_tests.rs @@ -53,9 +53,8 @@ fn valid_frames_keep_their_order_ids_and_parameters() { let pending = queue(); let q = pending.clone(); let worker = std::thread::spawn(move || serve(server, q, context(), Duration::from_secs(5))); - client.write_all(b"\r\n{bad JSON}\n{\"id\":\"test\",\"method\":\"engine.execute\",\"params\":{\"command\":\"shape.rectangle\"}}\r\n").unwrap(); + client.write_all(b"\r\n{\"id\":\"test\",\"method\":\"engine.execute\",\"params\":{\"command\":\"shape.rectangle\"}}\r\n").unwrap(); let mut reader = BufReader::new(client); - assert_eq!(reply(&mut reader)["ok"], false); let start = Instant::now(); let r = loop { if let Some(r) = pending.lock().unwrap().pop() { @@ -73,6 +72,23 @@ fn valid_frames_keep_their_order_ids_and_parameters() { worker.join().unwrap().unwrap(); } +#[test] +fn a_line_that_is_not_json_ends_the_connection_before_anything_after_it_runs() { + // What a browser sends when a web page posts to the port: the JSON line in its body never runs. + let (mut client, server) = pair(); + let pending = queue(); + let q = pending.clone(); + let worker = std::thread::spawn(move || serve(server, q, context(), Duration::from_secs(5))); + let body = "{\"id\":1,\"method\":\"engine.execute\",\"params\":{\"command\":\"file.save\"}}\n"; + let http = format!("POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Type: text/plain\r\nContent-Length: {}\r\n\r\n{body}", body.len()); + client.write_all(http.as_bytes()).unwrap(); + let mut reader = BufReader::new(client); + assert_eq!(reply(&mut reader)["ok"], false); + assert_eq!(reader.read(&mut [0]).unwrap(), 0, "the connection is closed"); + worker.join().unwrap().unwrap(); + assert!(pending.lock().unwrap().is_empty(), "nothing was dispatched"); +} + #[test] fn full_queue_rejects_before_dispatch_and_ui_queue_stays_bounded() { let (mut client, server) = pair(); diff --git a/contributors/people.toml b/contributors/people.toml index e744e31c..bc8cf71b 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -38,4 +38,9 @@ real_name = "Justin John" [people.Losminek] [people.RevDonz] + +[people.Nael-Nathanael] sync_github_name = true + +[people.volkovskey] +real_name = "Andrii Volkov" diff --git a/crates/color/src/lib.rs b/crates/color/src/lib.rs index 7d0bae9c..d6859a51 100644 --- a/crates/color/src/lib.rs +++ b/crates/color/src/lib.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; mod blend; mod hsx; mod icc; +mod named; mod profile; pub use blend::BlendMode; pub use hsx::*; @@ -139,7 +140,13 @@ impl Color { Some(Self::from_rgba8(r, g, b, a)) } - /// Parse a CSS colour: hex, `rgb()/rgba()`, `hsl()/hsla()` or a few common names. + /// What a colour field accepts: hex as [`Color::from_hex`] reads it, or a CSS colour name + /// (`red` → `FF0000`, `Cornflower Blue` → `6495ED`; opaque). + pub fn from_hex_or_name(s: &str) -> Option { + Self::from_hex(s).or_else(|| named::lookup(s).map(|[r, g, b]| Self::from_rgba8(r, g, b, 255))) + } + + /// Parse a CSS colour: hex, `rgb()/rgba()`, `hsl()/hsla()`, a CSS colour name or `transparent`. pub fn parse_css(s: &str) -> Option { let t = s.trim().to_ascii_lowercase(); if t.starts_with('#') { @@ -166,21 +173,10 @@ impl Color { let alpha = parts.get(3).and_then(|a| parse_alpha(a)).unwrap_or(1.0); return Some(Hsl { h, s, l }.to_color().with_alpha(alpha)); } - let named = match t.as_str() { - "black" => Color::BLACK, - "white" => Color::WHITE, - "transparent" => Color::TRANSPARENT, - "red" => Color::from_rgb8_const(255, 0, 0), - "green" => Color::from_rgb8_const(0, 128, 0), - "lime" => Color::from_rgb8_const(0, 255, 0), - "blue" => Color::from_rgb8_const(0, 0, 255), - "yellow" => Color::from_rgb8_const(255, 255, 0), - "orange" => Color::from_rgb8_const(255, 165, 0), - "purple" => Color::from_rgb8_const(128, 0, 128), - "gray" | "grey" => Color::from_rgb8_const(128, 128, 128), - _ => return Self::from_hex(&t), - }; - Some(named) + if t == "transparent" { + return Some(Color::TRANSPARENT); + } + Self::from_hex_or_name(&t) } /// CSS `rgba(...)` / hex text for copy-as-CSS. diff --git a/crates/color/src/named.rs b/crates/color/src/named.rs new file mode 100644 index 00000000..882472cc --- /dev/null +++ b/crates/color/src/named.rs @@ -0,0 +1,197 @@ +//! The CSS named colours (CSS Color Module Level 4 §6.1), so a colour field takes `red` as well as +//! `FF0000`. + +/// Every CSS named colour except `transparent`, sorted by name for binary search. +const NAMED: [(&str, [u8; 3]); 148] = [ + ("aliceblue", [240, 248, 255]), + ("antiquewhite", [250, 235, 215]), + ("aqua", [0, 255, 255]), + ("aquamarine", [127, 255, 212]), + ("azure", [240, 255, 255]), + ("beige", [245, 245, 220]), + ("bisque", [255, 228, 196]), + ("black", [0, 0, 0]), + ("blanchedalmond", [255, 235, 205]), + ("blue", [0, 0, 255]), + ("blueviolet", [138, 43, 226]), + ("brown", [165, 42, 42]), + ("burlywood", [222, 184, 135]), + ("cadetblue", [95, 158, 160]), + ("chartreuse", [127, 255, 0]), + ("chocolate", [210, 105, 30]), + ("coral", [255, 127, 80]), + ("cornflowerblue", [100, 149, 237]), + ("cornsilk", [255, 248, 220]), + ("crimson", [220, 20, 60]), + ("cyan", [0, 255, 255]), + ("darkblue", [0, 0, 139]), + ("darkcyan", [0, 139, 139]), + ("darkgoldenrod", [184, 134, 11]), + ("darkgray", [169, 169, 169]), + ("darkgreen", [0, 100, 0]), + ("darkgrey", [169, 169, 169]), + ("darkkhaki", [189, 183, 107]), + ("darkmagenta", [139, 0, 139]), + ("darkolivegreen", [85, 107, 47]), + ("darkorange", [255, 140, 0]), + ("darkorchid", [153, 50, 204]), + ("darkred", [139, 0, 0]), + ("darksalmon", [233, 150, 122]), + ("darkseagreen", [143, 188, 143]), + ("darkslateblue", [72, 61, 139]), + ("darkslategray", [47, 79, 79]), + ("darkslategrey", [47, 79, 79]), + ("darkturquoise", [0, 206, 209]), + ("darkviolet", [148, 0, 211]), + ("deeppink", [255, 20, 147]), + ("deepskyblue", [0, 191, 255]), + ("dimgray", [105, 105, 105]), + ("dimgrey", [105, 105, 105]), + ("dodgerblue", [30, 144, 255]), + ("firebrick", [178, 34, 34]), + ("floralwhite", [255, 250, 240]), + ("forestgreen", [34, 139, 34]), + ("fuchsia", [255, 0, 255]), + ("gainsboro", [220, 220, 220]), + ("ghostwhite", [248, 248, 255]), + ("gold", [255, 215, 0]), + ("goldenrod", [218, 165, 32]), + ("gray", [128, 128, 128]), + ("green", [0, 128, 0]), + ("greenyellow", [173, 255, 47]), + ("grey", [128, 128, 128]), + ("honeydew", [240, 255, 240]), + ("hotpink", [255, 105, 180]), + ("indianred", [205, 92, 92]), + ("indigo", [75, 0, 130]), + ("ivory", [255, 255, 240]), + ("khaki", [240, 230, 140]), + ("lavender", [230, 230, 250]), + ("lavenderblush", [255, 240, 245]), + ("lawngreen", [124, 252, 0]), + ("lemonchiffon", [255, 250, 205]), + ("lightblue", [173, 216, 230]), + ("lightcoral", [240, 128, 128]), + ("lightcyan", [224, 255, 255]), + ("lightgoldenrodyellow", [250, 250, 210]), + ("lightgray", [211, 211, 211]), + ("lightgreen", [144, 238, 144]), + ("lightgrey", [211, 211, 211]), + ("lightpink", [255, 182, 193]), + ("lightsalmon", [255, 160, 122]), + ("lightseagreen", [32, 178, 170]), + ("lightskyblue", [135, 206, 250]), + ("lightslategray", [119, 136, 153]), + ("lightslategrey", [119, 136, 153]), + ("lightsteelblue", [176, 196, 222]), + ("lightyellow", [255, 255, 224]), + ("lime", [0, 255, 0]), + ("limegreen", [50, 205, 50]), + ("linen", [250, 240, 230]), + ("magenta", [255, 0, 255]), + ("maroon", [128, 0, 0]), + ("mediumaquamarine", [102, 205, 170]), + ("mediumblue", [0, 0, 205]), + ("mediumorchid", [186, 85, 211]), + ("mediumpurple", [147, 112, 219]), + ("mediumseagreen", [60, 179, 113]), + ("mediumslateblue", [123, 104, 238]), + ("mediumspringgreen", [0, 250, 154]), + ("mediumturquoise", [72, 209, 204]), + ("mediumvioletred", [199, 21, 133]), + ("midnightblue", [25, 25, 112]), + ("mintcream", [245, 255, 250]), + ("mistyrose", [255, 228, 225]), + ("moccasin", [255, 228, 181]), + ("navajowhite", [255, 222, 173]), + ("navy", [0, 0, 128]), + ("oldlace", [253, 245, 230]), + ("olive", [128, 128, 0]), + ("olivedrab", [107, 142, 35]), + ("orange", [255, 165, 0]), + ("orangered", [255, 69, 0]), + ("orchid", [218, 112, 214]), + ("palegoldenrod", [238, 232, 170]), + ("palegreen", [152, 251, 152]), + ("paleturquoise", [175, 238, 238]), + ("palevioletred", [219, 112, 147]), + ("papayawhip", [255, 239, 213]), + ("peachpuff", [255, 218, 185]), + ("peru", [205, 133, 63]), + ("pink", [255, 192, 203]), + ("plum", [221, 160, 221]), + ("powderblue", [176, 224, 230]), + ("purple", [128, 0, 128]), + ("rebeccapurple", [102, 51, 153]), + ("red", [255, 0, 0]), + ("rosybrown", [188, 143, 143]), + ("royalblue", [65, 105, 225]), + ("saddlebrown", [139, 69, 19]), + ("salmon", [250, 128, 114]), + ("sandybrown", [244, 164, 96]), + ("seagreen", [46, 139, 87]), + ("seashell", [255, 245, 238]), + ("sienna", [160, 82, 45]), + ("silver", [192, 192, 192]), + ("skyblue", [135, 206, 235]), + ("slateblue", [106, 90, 205]), + ("slategray", [112, 128, 144]), + ("slategrey", [112, 128, 144]), + ("snow", [255, 250, 250]), + ("springgreen", [0, 255, 127]), + ("steelblue", [70, 130, 180]), + ("tan", [210, 180, 140]), + ("teal", [0, 128, 128]), + ("thistle", [216, 191, 216]), + ("tomato", [255, 99, 71]), + ("turquoise", [64, 224, 208]), + ("violet", [238, 130, 238]), + ("wheat", [245, 222, 179]), + ("white", [255, 255, 255]), + ("whitesmoke", [245, 245, 245]), + ("yellow", [255, 255, 0]), + ("yellowgreen", [154, 205, 50]), +]; + +/// Longest name in [`NAMED`]; anything longer can't match. +const LONGEST: usize = 20; + +/// The opaque sRGB bytes of a CSS colour name. Case, spaces, `-` and `_` are ignored, so +/// `Light Blue`, `light-blue` and `lightblue` are the same colour. +pub fn lookup(s: &str) -> Option<[u8; 3]> { + let key: String = s.chars().filter(|c| !c.is_whitespace() && *c != '-' && *c != '_').take(LONGEST + 1).collect::().to_ascii_lowercase(); + if key.is_empty() || key.len() > LONGEST { + return None; + } + NAMED.binary_search_by(|(name, _)| name.cmp(&key.as_str())).ok().and_then(|i| NAMED.get(i)).map(|(_, rgb)| *rgb) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn table_is_sorted_and_unique() { + assert!(NAMED.windows(2).all(|w| w[0].0 < w[1].0)); + assert_eq!(NAMED.iter().map(|(n, _)| n.len()).max(), Some(LONGEST)); + } + + #[test] + fn every_name_is_found() { + for (name, rgb) in NAMED { + assert_eq!(lookup(name), Some(rgb), "{name}"); + } + } + + #[test] + fn lookup_normalises() { + assert_eq!(lookup("Red"), Some([255, 0, 0])); + assert_eq!(lookup(" CORNFLOWER blue "), Some([100, 149, 237])); + assert_eq!(lookup("light-goldenrod_yellow"), Some([250, 250, 210])); + assert_eq!(lookup(""), None); + assert_eq!(lookup("transparent"), None); + assert_eq!(lookup("reddish"), None); + assert_eq!(lookup("rédé"), None); + assert_eq!(lookup(&"a".repeat(10_000)), None); + } +} diff --git a/crates/color/src/tests.rs b/crates/color/src/tests.rs index facb3415..68a838e0 100644 --- a/crates/color/src/tests.rs +++ b/crates/color/src/tests.rs @@ -17,6 +17,23 @@ fn css_parsing() { assert_eq!(Color::parse_css("rgba(0 0 255 / 50%)").unwrap().to_rgba8(), [0, 0, 255, 128]); assert_eq!(Color::parse_css("hsl(120, 100%, 50%)").unwrap().to_hex(), "00FF00"); assert_eq!(Color::parse_css("white").unwrap(), Color::WHITE); + assert_eq!(Color::parse_css("RebeccaPurple").unwrap().to_hex(), "663399"); + assert_eq!(Color::parse_css("transparent").unwrap(), Color::TRANSPARENT); +} + +#[test] +fn colour_fields_take_names() { + assert_eq!(Color::from_hex_or_name("red").unwrap().to_css_hex(), "#FF0000"); + assert_eq!(Color::from_hex_or_name(" Cornflower Blue ").unwrap().to_hex(), "6495ED"); + assert_eq!(Color::from_hex_or_name("darkblue").unwrap().to_rgba8(), [0, 0, 139, 255]); + // Hex wins: every digit string still reads as hex. + assert_eq!(Color::from_hex_or_name("abc").unwrap().to_hex(), "AABBCC"); + assert_eq!(Color::from_hex_or_name("#0D99FF").unwrap().to_hex(), "0D99FF"); + // Names are opaque; `transparent` is not a field colour. + assert!(Color::from_hex_or_name("transparent").is_none()); + for junk in ["", "#", "redd", "re d d", "12345", "\u{0}", "🟥", &"x".repeat(100_000)] { + assert!(Color::from_hex_or_name(junk).is_none(), "{junk:?}"); + } } #[test] diff --git a/crates/doc/src/proto_expr.rs b/crates/doc/src/proto_expr.rs index 2ec8e91f..f4404dc7 100644 --- a/crates/doc/src/proto_expr.rs +++ b/crates/doc/src/proto_expr.rs @@ -143,7 +143,7 @@ impl Expr { return "…".into(); } match self { - Expr::Value(VariableValue::Alias(id)) => vars.get(id).map_or_else(|| "(missing variable)".into(), |v| v.name.clone()), + Expr::Value(VariableValue::Alias(id)) => vars.get(id).map_or_else(|| "(missing variable)".into(), |v| var_ref(vars, v)), Expr::Value(VariableValue::String(s)) => quote(s), // Exact numbers and `#` colours, so the text reads back as the same expression. Expr::Value(VariableValue::Number(n)) => format!("{n}"), @@ -178,6 +178,18 @@ impl Expr { } } +/// A variable as written in an expression: its bare name when that reads back as this variable, +/// otherwise `{name}` (or `{id}` when even the braced name wouldn't), so names like `1`, `2x` or +/// `"q"` don't read back as literals or errors. +fn var_ref(vars: &Variables, v: &crate::Variable) -> String { + let this = Expr::var(v.id.clone()); + if Expr::parse(&v.name, vars).is_ok_and(|e| e == this) { + return v.name.clone(); + } + let braced = !v.name.contains('}') && v.name.trim() == v.name && vars.get(&v.name).is_some_and(|x| x.id == v.id); + if braced { format!("{{{}}}", v.name) } else { format!("{{{}}}", v.id) } +} + /// A collection or mode name as written inside `mode(…)` or `[…]`: as it is, or quoted when it has /// spaces at its ends, quotes or brackets. fn name_arg(name: &str) -> String { diff --git a/crates/doc/src/proto_expr_parse_tests.rs b/crates/doc/src/proto_expr_parse_tests.rs index 52f909fd..e106121d 100644 --- a/crates/doc/src/proto_expr_parse_tests.rs +++ b/crates/doc/src/proto_expr_parse_tests.rs @@ -176,3 +176,22 @@ fn hostile_text_is_bounded() { } } } + +#[test] +fn names_that_look_like_literals_read_back_as_variables() { + for name in ["count", "count total", "2x", "1", "-3", ".5", "(x)", "#ff", "\"q\"", "not", "true"] { + let mut v = vars(); + if let Some(var) = v.variables.iter_mut().find(|x| x.id == "n") { + var.name = name.into(); + } + for e in [Expr::var("n"), Expr::op(ExprOp::Add, vec![Expr::var("n"), n(1.0)]), Expr::InMode { variable: "n".into(), mode: "dark".into() }] { + let shown = e.display(&v); + assert_eq!(Expr::parse(&shown, &v), Ok(e.clone()), "{name:?} shown as {shown:?}"); + } + } + // Plain names stay bare, and literals still read as literals. + let v = vars(); + assert_eq!(Expr::var("n").display(&v), "n-name"); + assert_eq!(parse("1"), n(1.0)); + assert_eq!(parse("\"q\""), Expr::Value(VariableValue::String("q".into()))); +} diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index d54bb29b..43031438 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -381,3 +381,25 @@ fn custom_easings() { assert_eq!(back, Easing::CustomSpring { mass: 2.0, stiffness: 50.0, damping: 5.0 }); assert_eq!(serde_json::from_value::(serde_json::json!("easeInOut")).unwrap(), Easing::EaseInOut); } + +#[test] +fn text_style_empty_range_links_and_unlinks_all() { + let def = TextStyle { font_style: "Bold".into(), ..Default::default() }; + let mut t = TextData::new("abcd"); + t.link_text_style(2, 2, &def, "S"); + assert!((0..4).all(|i| t.style_at(i).style_id.as_deref() == Some("S") && t.style_at(i).font_style == "Bold")); + t.unlink_text_style(2, 2); + assert!((0..4).all(|i| t.style_at(i).style_id.is_none() && t.style_at(i).font_style == "Bold")); + // Partial ranges stay partial. + t.link_text_style(0, 4, &def, "S"); + t.unlink_text_style(1, 3); + assert_eq!(t.style_at(0).style_id.as_deref(), Some("S")); + assert!(t.style_at(1).style_id.is_none() && t.style_at(2).style_id.is_none()); + assert_eq!(t.style_at(3).style_id.as_deref(), Some("S")); + // Empty text changes the base style. + let mut e = TextData::new(""); + e.link_text_style(0, 0, &def, "S"); + assert_eq!(e.style.style_id.as_deref(), Some("S")); + e.unlink_text_style(0, 0); + assert!(e.style.style_id.is_none()); +} diff --git a/crates/doc/src/text.rs b/crates/doc/src/text.rs index 8018609d..3ddf6559 100644 --- a/crates/doc/src/text.rs +++ b/crates/doc/src/text.rs @@ -675,11 +675,13 @@ impl TextData { /// Link chars `start..end` (all of them when the range is empty or the whole text) to text /// style `id`, defined as `def`. pub fn link_text_style(&mut self, start: usize, end: usize, def: &TextStyle, id: &str) { + let (start, end) = if start == end { (0, self.char_count()) } else { (start, end) }; self.restyle(start, end, |s| s.take_text_style(def, id)); } /// Unlink chars `start..end` (all when empty or whole) from their text styles; values stay. pub fn unlink_text_style(&mut self, start: usize, end: usize) { + let (start, end) = if start == end { (0, self.char_count()) } else { (start, end) }; self.restyle(start, end, |s| s.style_id = None); } diff --git a/crates/doc/src/vector_faces.rs b/crates/doc/src/vector_faces.rs index a34c5036..050ab821 100644 --- a/crates/doc/src/vector_faces.rs +++ b/crates/doc/src/vector_faces.rs @@ -136,8 +136,8 @@ impl VectorNetwork { } let k = key(&face.loops); match (self.regions.iter().position(|r| key(&r.loops) == k), want) { - (Some(i), None | Some(false)) => { - self.regions.remove(i); + (Some(_), None | Some(false)) => { + self.regions.retain(|r| key(&r.loops) != k); Some(false) } (Some(_), Some(true)) => Some(true), @@ -525,4 +525,21 @@ mod tests { let _ = j.paint(Point::new(f64::NAN, 5.0)); assert!(interior_point(&BezPath::new()).is_none()); } + + #[test] + fn the_bucket_unfills_a_face_with_duplicate_regions_in_one_click() { + let c = Point::new(50.0, 50.0); + let mut n = square(0.0, 0.0, 100.0); + n.regions.push(n.regions[0].clone()); + assert_eq!(n.paint(c), Some(false)); + assert!(!n.fill_contains(c)); + assert_eq!(n.region_at(c), None); + assert_eq!(n.paint(c), Some(true)); + assert!(n.fill_contains(c)); + + let mut e = square(0.0, 0.0, 100.0); + e.regions.push(e.regions[0].clone()); + assert_eq!(e.paint_to(c, Some(false)), Some(false)); + assert!(!e.fill_contains(c)); + } } diff --git a/crates/doc/src/vector_regions.rs b/crates/doc/src/vector_regions.rs index 1d1cd794..cc456bef 100644 --- a/crates/doc/src/vector_regions.rs +++ b/crates/doc/src/vector_regions.rs @@ -18,8 +18,9 @@ impl VectorNetwork { /// second; a segment that doesn't touch the point reached so far (a malformed loop) is taken /// as stored. Missing segments are skipped. pub fn loop_walk(&self, lp: &[usize]) -> Vec<(usize, bool)> { - let Some(first) = lp.first().and_then(|i| self.segments.get(*i)) else { return Vec::new() }; - let forward_first = match lp.get(1).and_then(|i| self.segments.get(*i)) { + let mut existing = lp.iter().filter_map(|i| self.segments.get(*i)); + let Some(first) = existing.next() else { return Vec::new() }; + let forward_first = match existing.next() { Some(second) => first.b == second.a || first.b == second.b, None => true, }; @@ -102,10 +103,18 @@ impl VectorNetwork { (0..self.regions.len()).rev().find(|r| self.region_contains(*r, p)) } - /// Every point on region `r`'s loops (holes' too), each once, sorted. + /// Every point on region `r`'s loops (holes' too), each once, sorted (segments or points that + /// don't exist are left out). pub fn region_points(&self, r: usize) -> Vec { let Some(region) = self.regions.get(r) else { return Vec::new() }; - let mut out: Vec = region.loops.iter().flatten().filter_map(|i| self.segments.get(*i)).flat_map(|s| [s.a, s.b]).collect(); + let mut out: Vec = region + .loops + .iter() + .flatten() + .filter_map(|i| self.segments.get(*i)) + .flat_map(|s| [s.a, s.b]) + .filter(|v| *v < self.vertices.len()) + .collect(); out.sort_unstable(); out.dedup(); out diff --git a/crates/doc/src/vector_regions_tests.rs b/crates/doc/src/vector_regions_tests.rs index d1e3d0d8..c09664b4 100644 --- a/crates/doc/src/vector_regions_tests.rs +++ b/crates/doc/src/vector_regions_tests.rs @@ -148,6 +148,28 @@ fn a_loop_that_does_not_close_is_closed_implicitly() { assert_eq!(n.region_at(p(300.0, 100.0)), None); } +#[test] +fn missing_segments_are_skipped_wherever_they_sit_in_a_loop() { + use figcraft_geom::Shape; + let mut n = VectorNetwork::default(); + square(&mut n, 0.0, 0.0, 200.0); + n.regions = vec![region(vec![vec![0, 1, 2]], WindingRule::NonZero)]; + let control = n.region_path(0).unwrap(); + let control_walk = n.loop_walk(&[0, 1, 2]); + for lp in [vec![99, 0, 1, 2], vec![0, 99, 1, 2], vec![99, 98, 0, 1, 2]] { + n.regions = vec![region(vec![lp.clone()], WindingRule::NonZero)]; + assert_eq!(n.loop_walk(&lp), control_walk, "{lp:?}"); + assert_eq!(n.region_at(p(100.0, 100.0)), Some(0), "{lp:?}"); + assert!(n.region_contains(0, p(100.0, 100.0)), "{lp:?}"); + let path = n.region_path(0).unwrap(); + assert_eq!(path.elements().len(), control.elements().len(), "{lp:?}"); + assert!((path.area().abs() - control.area().abs()).abs() < 1e-6, "{lp:?}"); + } + n.regions = vec![region(vec![vec![99]], WindingRule::NonZero)]; + assert!(n.region_path(0).is_none()); + assert!(!n.region_contains(0, p(100.0, 100.0))); +} + #[test] fn a_regions_points_are_listed_once() { let mut n = nested(); @@ -186,6 +208,19 @@ fn empty_and_damaged_networks_have_no_regions_and_never_panic() { } } +#[test] +fn region_points_leave_out_segment_ends_that_do_not_exist() { + let mut n = VectorNetwork::default(); + let ring = square(&mut n, 0.0, 0.0, 100.0); + n.regions = vec![region(vec![ring], WindingRule::NonZero)]; + assert_eq!(n.region_points(0), vec![0, 1, 2, 3]); + n.segments[3] = Segment::line(3, 9); + assert_eq!(n.region_points(0), vec![0, 1, 2, 3]); + assert!(n.region_path(0).is_some()); + n.regions = vec![region(vec![vec![99]], WindingRule::NonZero)]; + assert!(n.region_points(0).is_empty()); +} + #[test] fn the_topology_key_ignores_moved_points_but_not_new_segments() { let mut n = nested(); diff --git a/crates/engine/src/al_keys_tests.rs b/crates/engine/src/al_keys_tests.rs index 1053997e..76256466 100644 --- a/crates/engine/src/al_keys_tests.rs +++ b/crates/engine/src/al_keys_tests.rs @@ -74,6 +74,27 @@ fn along_the_flow_skips_hidden_and_absolute_siblings() { assert_eq!(kids(&s, col), vec![c1, c0], "Right does nothing in a lone column"); } +#[test] +fn single_item_wrapped_lines_move_and_drop_by_line() { + let mut s = Session::new(); + // Full-width items: one per row. + let row = al(&mut s, None, "horizontal", 160.0, 200.0); + set(&mut s, row, json!({"wrap": true})); + let k: Vec = (0..2).map(|_| rect(&mut s, row, 160.0, 20.0)).collect(); + assert_eq!(s.doc().unwrap().world_bounds(k[1]).unwrap().y0, 30.0, "second row"); + let slot = crate::drop::auto_layout_slot(s.doc().unwrap(), row, figcraft_geom::Point::new(10.0, 35.0), &[]); + assert_eq!(slot, Some((1, Some([-2.0, 30.0, -2.0, 50.0])))); + arrow(&mut s, &[k[1]], 0, -1); + assert_eq!(kids(&s, row), vec![k[1], k[0]]); + + // Full-height items in a vertical wrap: one per column. + let col = al(&mut s, None, "vertical", 200.0, 160.0); + set(&mut s, col, json!({"wrap": true})); + let c: Vec = (0..3).map(|_| rect(&mut s, col, 20.0, 160.0)).collect(); + arrow(&mut s, &[c[2]], -1, 0); + assert_eq!(kids(&s, col), vec![c[0], c[2], c[1]]); +} + #[test] fn wrapped_rows_move_up_and_down_a_row() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 62db0835..8f91826a 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -824,12 +824,21 @@ pub fn export_bytes(doc: &Document, ids: &[NodeId], format: ExportFormat, scale: } /// The page area exporting `ids` with `setting` covers: what they paint (a clipping frame's own -/// area), or their boxes with "include bounding box" and for slices. +/// area), or their boxes with "include bounding box" and for slices. Hidden layers paint nothing, +/// so they take no room next to visible ones (a hidden-only export keeps its own area). pub fn export_area(doc: &Document, ids: &[NodeId], setting: &ExportSetting) -> Option { + let any_visible = ids.iter().any(|i| doc.node(*i).is_some_and(|n| n.visible)); ids.iter() .filter_map(|i| { - let slice = doc.node(*i).is_some_and(|n| matches!(n.kind, NodeKind::Slice)); - if setting.include_bounding_box || slice { doc.world_bounds(*i) } else { figcraft_render::export_bounds(doc, *i) } + let node = doc.node(*i); + let slice = node.is_some_and(|n| matches!(n.kind, NodeKind::Slice)); + if setting.include_bounding_box || slice { + doc.world_bounds(*i) + } else if any_visible && node.is_some_and(|n| !n.visible) { + None + } else { + figcraft_render::export_bounds(doc, *i) + } }) .reduce(|a, b| a.union(b)) } diff --git a/crates/engine/src/cmd/page.rs b/crates/engine/src/cmd/page.rs index 0064ffe0..52531aa3 100644 --- a/crates/engine/src/cmd/page.rs +++ b/crates/engine/src/cmd/page.rs @@ -1,5 +1,6 @@ //! Pages. +use figcraft_doc::proto::Flow; use figcraft_doc::*; use serde_json::{Value, json}; @@ -23,6 +24,15 @@ fn settle(s: &mut Session, cmd: &str) -> crate::Result<()> { Ok(()) } +/// The page a command targets: the explicit `id` (which must be a page) or the open page. +fn page_target(s: &Session, cmd: &str, p: &Value) -> crate::Result { + let Some(target) = id(p, "id") else { return s.page() }; + if !s.doc()?.pages.contains(&target) { + return Err(bad(cmd, format!("{target} is not a page"))); + } + Ok(target) +} + pub fn specs() -> Vec { vec![ spec!("page.add", "Add page", "{name?} (a name starting with a hyphen adds a divider)", true, |s, p| { @@ -71,13 +81,13 @@ pub fn specs() -> Vec { }), spec!("page.rename", "Rename page", "{id?, name} (an empty page named with a leading hyphen becomes a divider)", true, |s, p| { let name = req_string("page.rename", p, "name")?; - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.rename", p)?; s.doc_mut()?.get_mut(id)?.name = name; settle(s, "page.rename")?; ok() }), spec!("page.delete", "Delete page", "{id?}", true, |s, p| { - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.delete", p)?; s.doc_mut()?.remove(id)?; settle(s, "page.delete")?; ok() @@ -87,13 +97,23 @@ pub fn specs() -> Vec { let doc = s.doc_mut()?; let name = format!("Copy of {}", doc.get(src)?.name); let np = doc.add_page(&name); - if let NodeKind::Page { background, .. } = doc.get(src)?.kind.clone() - && let NodeKind::Page { background: b, .. } = &mut doc.get_mut(np)?.kind + // Each layer and its copy (also those in sections), so flows can follow their start frames. + let mut copies = Vec::new(); + for c in doc.get(src)?.children.clone() { + let copy = doc.duplicate_into(c, np, None)?; + copies.extend(doc.subtree(c).into_iter().zip(doc.subtree(copy))); + } + // The copy keeps the page's colour, flows (pointing at the copied frames), prototype settings and guides. + if let NodeKind::Page { background, flows, prototype, guides } = doc.get(src)?.kind.clone() + && let NodeKind::Page { background: b, flows: f, prototype: pr, guides: g } = &mut doc.get_mut(np)?.kind { *b = background; - } - for c in doc.get(src)?.children.clone() { - doc.duplicate_into(c, np, None)?; + *f = flows + .into_iter() + .filter_map(|fl| copies.iter().find(|(old, _)| *old == fl.start).map(|(_, new)| Flow { start: *new, ..fl })) + .collect(); + *pr = prototype; + *g = guides; } // Keep the new page right after the source. if let Some(si) = doc.pages.iter().position(|x| *x == src) { @@ -117,7 +137,7 @@ pub fn specs() -> Vec { }), spec!("page.setBackground", "Page background", "{id?, color}", true, |s, p| { let c = color(p, "color").ok_or_else(|| bad("page.setBackground", "`color` required"))?; - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.setBackground", p)?; if let NodeKind::Page { background, .. } = &mut s.doc_mut()?.get_mut(id)?.kind { *background = c; } @@ -163,8 +183,11 @@ pub fn specs() -> Vec { } guides.remove(k); let ds = s.ds_mut()?; - if ds.guide == Some((owner, k)) { - ds.guide = None; + match ds.guide { + Some((o, i)) if o == owner && i == k => ds.guide = None, + // A later guide of the same owner moves up one place; keep the selection on it. + Some((o, i)) if o == owner && i > k => ds.guide = Some((o, i - 1)), + _ => {} } ok() }), diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index a5f2b78f..552a44cf 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -112,6 +112,9 @@ pub fn specs() -> Vec { spec!("variable.createCollection", "Create variable collection", "{name, modes? [names]}", true, |s, p| { let name = req_string("variable.createCollection", p, "name")?; let doc = s.doc_mut()?; + if doc.variables.collections.iter().any(|c| c.name == name) { + return Err(bad("variable.createCollection", format!("a collection named `{name}` already exists"))); + } let id = doc.new_collection_id(); let modes: Vec = match get(p, "modes") { Some(Value::Array(a)) => a.iter().filter_map(|m| m.as_str().map(str::to_string)).collect(), diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index ae627abe..3e6dc69d 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -87,7 +87,7 @@ fn update(s: &mut Session, p: &Value) -> Result { } None => None, }; - let scopes = match get(p, "scopes") { + let scopes = match p.get("scopes") { None => None, Some(Value::Null) => Some(None), Some(Value::Array(a)) => { diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index 0f09a6db..de69e690 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -138,8 +138,16 @@ fn page_step(s: &mut Session, dir: i64) -> Result { let pages = &ds.doc.pages; let i = pages.iter().position(|p| *p == ds.page).unwrap_or(0) as i64; let n = pages.len() as i64; - let next = (i + dir).clamp(0, n - 1) as usize; - let Some(pg) = pages.get(next).copied() else { return ok() }; + // Dividers never open: step over them, and stay put when only dividers lie ahead. + let mut next = i + dir; + let pg = loop { + let page = usize::try_from(next).ok().and_then(|k| pages.get(k).copied()); + match page { + Some(p) if !ds.doc.is_divider(p) => break p, + Some(_) if dir != 0 && next >= 0 && next < n => next += dir, + _ => break ds.page, + } + }; let ds = s.ds_mut()?; ds.page = pg; ds.selection.clear(); diff --git a/crates/engine/src/codegen.rs b/crates/engine/src/codegen.rs index cf96f70d..55343c40 100644 --- a/crates/engine/src/codegen.rs +++ b/crates/engine/src/codegen.rs @@ -113,8 +113,58 @@ pub(crate) fn num(v: f64) -> String { } /// `code` with every pixel length (`12px`) in rem of `root` pixels (`0.75rem`); names that end -/// in "px" are left alone. +/// in "px", markup lines (layer text), string literals and comments are left alone. pub fn px_to_rem(code: &str, root: f64) -> String { + let mut out = String::with_capacity(code.len()); + let mut in_comment = false; + for line in code.split_inclusive('\n') { + if !in_comment && line.trim_start().starts_with('<') { + out.push_str(line); + continue; + } + // Lengths outside strings and comments convert; `plain` is where that stretch starts. + let mut plain = 0; + let mut chars = line.char_indices().peekable(); + while let Some((i, c)) = chars.next() { + if in_comment { + if c == '*' && chars.peek().is_some_and(|(_, n)| *n == '/') { + chars.next(); + out.push_str(line.get(plain..i + 2).unwrap_or("")); + plain = i + 2; + in_comment = false; + } + } else if c == '/' && chars.peek().is_some_and(|(_, n)| *n == '*') { + chars.next(); + out.push_str(&lengths_to_rem(line.get(plain..i).unwrap_or(""), root)); + plain = i; + in_comment = true; + } else if c == '"' { + out.push_str(&lengths_to_rem(line.get(plain..i).unwrap_or(""), root)); + let mut end = line.len(); + while let Some((j, s)) = chars.next() { + if s == '\\' { + chars.next(); + } else if s == '"' { + end = j + 1; + break; + } + } + out.push_str(line.get(i..end).unwrap_or("")); + plain = end; + } + } + let rest = line.get(plain..).unwrap_or(""); + if in_comment { + out.push_str(rest); + } else { + out.push_str(&lengths_to_rem(rest, root)); + } + } + out +} + +/// `code` with every pixel length in rem of `root` pixels; names that end in "px" are left alone. +fn lengths_to_rem(code: &str, root: f64) -> String { let ident = |c: char| c.is_alphanumeric() || c == '_'; let mut out = String::with_capacity(code.len()); let mut rest = code; diff --git a/crates/engine/src/component_codegen.rs b/crates/engine/src/component_codegen.rs index 2b776ca3..a8217be1 100644 --- a/crates/engine/src/component_codegen.rs +++ b/crates/engine/src/component_codegen.rs @@ -615,10 +615,10 @@ pub fn definition(doc: &Document, c: &Component, lang: CodeLang) -> String { } } -/// `data-*` attribute names for variant parameters. -fn data_attr(p: &Param) -> String { +/// `data-*` attribute names for variant parameters, unique among them (in order). +fn data_attrs(variants: &[&Param]) -> Vec { let mut used = Vec::new(); - format!("data-{}", class_name(&p.prop, &mut used)) + variants.iter().map(|p| format!("data-{}", class_name(&p.prop, &mut used))).collect() } /// `c`'s markup in React or Leptos (two levels in) and its stylesheet (variables as custom @@ -628,6 +628,7 @@ fn web(doc: &Document, c: &Component, lang: CodeLang) -> (String, String, String let root_name = c.set.and_then(|s| doc.node(s)).or_else(|| doc.node(c.comp)).map(|n| n.name.clone()).unwrap_or_default(); let root_class = class_name(&root_name, &mut Vec::new()); let variants: Vec<&Param> = c.params.iter().filter(|p| matches!(p.kind, ParamKind::Variant(_))).collect(); + let attr_names = data_attrs(&variants); let pass = |root: NodeId| { let vctx = Ctx { doc, root, comp: Some(c), base: 2 }; let mut m = Markup::new(&vctx, lang); @@ -639,9 +640,10 @@ fn web(doc: &Document, c: &Component, lang: CodeLang) -> (String, String, String m.root_class = Some(root_class.clone()); m.root_attrs = variants .iter() - .map(|p| match lang { - CodeLang::Leptos => format!(" {}={{{}.as_str()}}", data_attr(p), rust_ident(&p.ident)), - _ => format!(" {}={{{}}}", data_attr(p), p.ident), + .zip(&attr_names) + .map(|(p, attr)| match lang { + CodeLang::Leptos => format!(" {attr}={{{}.as_str()}}", rust_ident(&p.ident)), + _ => format!(" {attr}={{{}}}", p.ident), }) .collect(); m.node(c.comp, None, 2, ""); @@ -653,7 +655,8 @@ fn web(doc: &Document, c: &Component, lang: CodeLang) -> (String, String, String let pairs = variant_props(&vn.name); let attrs: String = variants .iter() - .filter_map(|p| pairs.get(&p.prop).map(|val| format!("[{}=\"{}\"]", data_attr(p), val.replace(['"', '\\'], "")))) + .zip(&attr_names) + .filter_map(|(p, attr)| pairs.get(&p.prop).map(|val| format!("[{attr}=\"{}\"]", val.replace(['"', '\\'], "")))) .collect(); for (path, _, decls) in pass(v) { let Some((_, class, base)) = m.rules.iter().find(|(p, _, _)| *p == path) else { continue }; diff --git a/crates/engine/src/component_codegen_tests.rs b/crates/engine/src/component_codegen_tests.rs index c92815a7..4cd3d776 100644 --- a/crates/engine/src/component_codegen_tests.rs +++ b/crates/engine/src/component_codegen_tests.rs @@ -314,3 +314,32 @@ fn bound_variables_become_custom_properties() { let react = code(&mut s, c, "react"); assert!(react.contains(":root {\n --colors-brand-500: #3366FF;\n}"), "{react}"); } + +#[test] +fn case_colliding_variant_properties_get_distinct_data_attributes() { + let mut s = Session::new(); + let mut variants = Vec::new(); + for (k, (first, second, fill)) in [("Small", "Big", "#0000FF"), ("Large", "Small", "#FF0000")].iter().enumerate() { + let name = format!("Size={first}, size={second}"); + let f = idv(&ex( + &mut s, + "frame.create", + json!({"x": k as f64 * 150.0, "y": 0, "width": 100, "height": 40, "name": name, "props": {"fill": fill}}), + )); + variants.push(idv(&ex(&mut s, "component.create", json!({"ids": [f]})))); + } + let set = idv(&ex(&mut s, "component.combineVariants", json!({"ids": variants}))); + ex(&mut s, "node.set", json!({"id": set, "props": {"name": "Chip"}})); + for lang in ["react", "leptos"] { + let v = component_code(&mut s, set, lang); + let def = v["definition"].as_str().unwrap_or_default(); + let root = def.lines().find(|l| l.trim_start().starts_with("
= + root.split_whitespace().filter_map(|w| w.strip_prefix("data-")).map(|w| w.split('=').next().unwrap_or_default()).collect(); + assert_eq!(attrs.len(), 2, "{lang}: {root}"); + assert_ne!(attrs[0], attrs[1], "{lang}: distinct attributes in {root}"); + let sel = format!(".chip[data-{}=\"Large\"][data-{}=\"Small\"]", attrs[0], attrs[1]); + assert!(def.contains(&sel), "{lang}: {sel} in:\n{def}"); + assert!(def.contains("#FF0000"), "{lang}:\n{def}"); + } +} diff --git a/crates/engine/src/cursor_tests.rs b/crates/engine/src/cursor_tests.rs index 0975f3fa..05a95f66 100644 --- a/crates/engine/src/cursor_tests.rs +++ b/crates/engine/src/cursor_tests.rs @@ -219,6 +219,24 @@ fn an_edit_an_instance_refuses_shows_not_allowed_and_is_undone() { assert_eq!(s.doc().unwrap().world_bounds(inner).unwrap(), before, "the move was undone"); } +#[test] +fn a_pen_click_that_wobbles_a_pixel_is_still_a_click() { + let mut s = Session::new(); + s.set_tool(ToolId::Pen); + // A real click: the mouse moves a pixel or two between press and release. + s.pointer(&ev(PointerKind::Down, (100.0, 100.0), NONE)).unwrap(); + s.pointer(&ev(PointerKind::Move, (102.0, 101.0), NONE)).unwrap(); + s.pointer(&ev(PointerKind::Up, (102.0, 101.0), NONE)).unwrap(); + let out = s.tool.pen.as_ref().and_then(|p| p.points.first()).map(|(_, _, o)| *o); + assert_eq!(out.map(|o| o.hypot()), Some(0.0), "no handle pulled out"); + assert!(s.tool.overlay.pen_handles.is_empty()); + // Past the slop it is a drag: the handle follows the pointer from where it was pressed. + s.pointer(&ev(PointerKind::Down, (200.0, 100.0), NONE)).unwrap(); + s.pointer(&ev(PointerKind::Move, (204.0, 100.0), NONE)).unwrap(); + let out = s.tool.pen.as_ref().and_then(|p| p.points.last()).map(|(_, _, o)| (o.x, o.y)); + assert_eq!(out, Some((4.0, 0.0))); +} + #[test] fn the_pen_shows_what_a_click_would_do() { // The Pen tool drawing a new path: three points, then over each of them. diff --git a/crates/engine/src/devfocus_tests.rs b/crates/engine/src/devfocus_tests.rs index 5583efe9..1607ce56 100644 --- a/crates/engine/src/devfocus_tests.rs +++ b/crates/engine/src/devfocus_tests.rs @@ -93,3 +93,29 @@ fn only_the_focused_frame_is_hit_and_escape_leaves() { click(&mut s, 350.0, 100.0); assert_eq!(s.selection(), vec![b], "everything is hit again"); } + +#[test] +fn nested_frames_stay_out_of_the_ready_list_and_its_navigation() { + let mut s = Session::new(); + let screen = frame(&mut s, 0.0, "Screen"); + let card = idv(&ex(&mut s, "frame.create", json!({"parent": screen, "width": 100, "height": 50, "name": "Card"}))); + // A status a nested frame already stores (an older file, or set through node.set) is ignored. + ex(&mut s, "node.set", json!({"id": card, "props": {"devStatus": "readyForDev"}})); + assert_eq!(ex(&mut s, "inspect.readyForDev", json!({"status": "any"}))["items"], json!([])); + assert!(s.execute("dev.focus", &json!({"direction": "next"})).is_err(), "nothing is ready"); + ex(&mut s, "node.set", json!({"id": screen, "props": {"devStatus": "readyForDev"}})); + let ids = |v: serde_json::Value| -> Vec { + v["items"].as_array().map(|a| a.iter().map(|i| i["id"].clone()).collect()).unwrap_or_default() + }; + assert_eq!(ids(ex(&mut s, "inspect.readyForDev", json!({}))), vec![json!(screen)]); + ex(&mut s, "dev.focus", json!({"id": screen})); + for direction in ["next", "previous"] { + let v = ex(&mut s, "dev.focus", json!({"direction": direction})); + assert_eq!((v["id"].clone(), v["count"].clone()), (json!(screen), json!(1)), "{direction} wraps to the only ready frame"); + } + // A frame directly inside a section still counts. + let sec = idv(&ex(&mut s, "node.create", json!({"type": "section", "x": 1000, "y": 0, "width": 400, "height": 300}))); + let in_sec = idv(&ex(&mut s, "frame.create", json!({"parent": sec, "width": 100, "height": 100, "name": "In section"}))); + ex(&mut s, "node.set", json!({"ids": [sec, in_sec], "props": {"devStatus": "readyForDev"}})); + assert_eq!(ids(ex(&mut s, "inspect.readyForDev", json!({}))), vec![json!(screen), json!(sec), json!(in_sec)]); +} diff --git a/crates/engine/src/devinspect.rs b/crates/engine/src/devinspect.rs index 95895f64..7d93e974 100644 --- a/crates/engine/src/devinspect.rs +++ b/crates/engine/src/devinspect.rs @@ -244,7 +244,7 @@ pub fn has_status(doc: &Document, id: NodeId) -> bool { } /// The layers with handoff status `status` (`None`: any status) on `page` (`None`: every page), -/// in page and layer order (bounded). +/// in page and layer order (bounded). Only layers that can carry a status are listed. pub fn dev_items(doc: &Document, status: Option, page: Option) -> Vec { let mut out = Vec::new(); let mut visited = 0usize; @@ -256,7 +256,7 @@ pub fn dev_items(doc: &Document, status: Option, page: Option return out; } let Some(n) = doc.node(id) else { continue }; - if !n.dev_status.is_none() && status.is_none_or(|s| s == n.dev_status) { + if !n.dev_status.is_none() && status.is_none_or(|s| s == n.dev_status) && has_status(doc, id) { let changed = n.dev_status == DevStatus::ReadyForDev && n.dev_changed; out.push(DevItem { id, name: n.name.clone(), page: pg, status: n.dev_status, changed }); } diff --git a/crates/engine/src/drop.rs b/crates/engine/src/drop.rs index f478735e..d5e8b3cf 100644 --- a/crates/engine/src/drop.rs +++ b/crates/engine/src/drop.rs @@ -61,12 +61,15 @@ pub fn flip(r: figcraft_geom::Rect, vertical: bool) -> figcraft_geom::Rect { /// The visual rows (columns when `vertical`) of a wrapping layout's placed children `kids`, each /// child as (its index in `kids`, its page bounds [`flip`]ped so the flow runs along x). Children -/// wrap in flow order, so a new row starts where a child sits before the one before. +/// wrap in flow order, so a new row starts where a child sits before the one before, or where it +/// does not follow the one before along the flow but sits past it across (one child per line). pub fn wrap_rows(doc: &Document, kids: &[NodeId], vertical: bool) -> Vec> { let mut rows: Vec> = Vec::new(); for (i, b) in kids.iter().enumerate().filter_map(|(i, c)| doc.world_bounds(*c).map(|b| (i, flip(b, vertical)))) { match rows.last_mut() { - Some(row) if row.last().is_some_and(|(_, prev)| b.x0 >= prev.x0 - 0.5) => row.push((i, b)), + Some(row) if row.last().is_some_and(|(_, prev)| b.x0 >= prev.x0 - 0.5 && !(b.x0 < prev.x1 - 0.5 && b.y0 >= prev.y1 - 0.5)) => { + row.push((i, b)) + } _ => rows.push(vec![(i, b)]), } } diff --git a/crates/engine/src/html_codegen_tests.rs b/crates/engine/src/html_codegen_tests.rs index 95b8f40d..b7fbb9ac 100644 --- a/crates/engine/src/html_codegen_tests.rs +++ b/crates/engine/src/html_codegen_tests.rs @@ -77,3 +77,43 @@ fn leptos_wraps_the_markup_in_a_component_with_text_as_string_literals() { let out = s.execute("inspect.code", &json!({"id": card, "language": "leptos", "unit": "rem"})).unwrap(); assert!(out["code"].as_str().unwrap().contains("width: 12.5rem;"), "{}", out["code"]); } + +fn web_code(s: &mut Session, id: figcraft_doc::NodeId, language: &str, unit: &str) -> String { + let out = s.execute("inspect.code", &json!({"id": id, "language": language, "unit": unit})).unwrap(); + out["code"].as_str().unwrap().to_string() +} + +#[test] +fn rem_preserves_text_in_web_markup() { + let mut s = Session::new(); + let frame = id(s.execute("frame.create", &json!({"width": 200, "height": 100, "name": "Card"})).unwrap()); + s.execute("text.create", &json!({"text": "Tile 24px", "parent": frame, "name": "Label"})).unwrap(); + let mut changed = Vec::new(); + for language in ["html", "react", "leptos"] { + let px = web_code(&mut s, frame, language, "px"); + let rem = web_code(&mut s, frame, language, "rem"); + assert!(px.contains("Tile 24px"), "{language}: px control: {px}"); + assert!(rem.contains("width: 12.5rem;"), "{language}: width: {rem}"); + assert!(rem.contains("font-size: 0.75rem;"), "{language}: font: {rem}"); + if !rem.contains("Tile 24px") { + changed.push(language); + } + } + assert!(changed.is_empty(), "text rewritten in {changed:?}"); +} + +#[test] +fn rem_preserves_text_property_default() { + let mut s = Session::new(); + let frame = id(s.execute("frame.create", &json!({"width": 100, "height": 40, "name": "Chip"})).unwrap()); + let label = id(s.execute("text.create", &json!({"text": "24px", "parent": frame, "name": "Label"})).unwrap()); + let component = id(s.execute("component.create", &json!({"ids": [frame]})).unwrap()); + s.execute("component.addProperty", &json!({"id": component, "name": "Label", "type": "text", "default": "24px", "layer": label})).unwrap(); + for unit in ["px", "rem"] { + let code = web_code(&mut s, component, "react", unit); + assert!(code.contains("label = \"24px\""), "{unit}: changed default: {code}"); + if unit == "rem" { + assert!(code.contains("width: 6.25rem;"), "rem still converts lengths: {code}"); + } + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 46571696..4b155897 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1236,8 +1236,8 @@ pub fn changed_nodes(doc: &Document, before: &Document) -> Vec { pub fn normalize(doc: &mut Document, before: &Document) { // Only what was touched since the last normalisation (edited, added or removed). let mut touched: BTreeSet = doc.take_dirty(); - // A variable edit changes no layer directly, but bound layers must follow it. - if touched.is_empty() && doc.variables == before.variables { + // A variable or style edit changes no layer directly, but bound and linked layers must follow it. + if touched.is_empty() && doc.variables == before.variables && doc.styles == before.styles { return; } let m = measure::TextMeasure; @@ -1562,6 +1562,8 @@ mod invite_links_tests; #[cfg(test)] mod keymap_tests; #[cfg(test)] +mod library_range_styles_tests; +#[cfg(test)] mod library_variables_tests; #[cfg(test)] mod live_access_tests; @@ -1590,6 +1592,8 @@ mod open_issues_tests; #[cfg(test)] mod outline_options_tests; #[cfg(test)] +mod page_target_tests; +#[cfg(test)] mod paint_reorder_tests; #[cfg(test)] mod paint_tests; diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index 936d798a..73c599a4 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -134,9 +134,12 @@ pub fn closure(doc: &Document, roots: &[NodeId]) -> Vec { out } -/// The style ids a layer links to (fill, stroke, effect, grid, text). +/// The style ids a layer links to (fill, stroke, effect, grid, text, and text styles on +/// character ranges). pub(crate) fn style_links(n: &Node) -> impl Iterator { - [&n.styles.fill, &n.styles.stroke, &n.styles.effect, &n.styles.grid, &n.styles.text].into_iter().flatten() + let chars = + n.kind.text().into_iter().flat_map(|t| std::iter::once(&t.style).chain(t.runs.iter().map(|r| &r.style))).filter_map(|s| s.style_id.as_ref()); + [&n.styles.fill, &n.styles.stroke, &n.styles.effect, &n.styles.grid, &n.styles.text].into_iter().flatten().chain(chars) } /// Style ids the layers under `roots` link to. @@ -582,11 +585,15 @@ fn local_key(lid: &str, key: &str, taken: impl Fn(&str) -> bool) -> String { } /// Copy library styles `keys` into `doc` as linked remote styles (with `update`, copies older than -/// the library's are refreshed). Returns library style id → local id. +/// the library's are refreshed). The variables a written copy binds are imported too and its +/// bindings point at their copies (never at this file's variables of the same ids). Returns library +/// style id → local id. pub fn import_styles(doc: &mut Document, lib: &PublishedLibrary, keys: &BTreeSet, update: bool) -> BTreeMap { let lid = lib.manifest.id.clone(); let mut out = BTreeMap::new(); let mut hashes = BTreeSet::new(); + // Local ids of the copies written now, with the library style they copy. + let mut written: Vec<(String, &Style)> = Vec::new(); for key in keys { let Some(style) = lib.doc.styles.iter().find(|s| s.id == *key) else { continue }; let version = lib.manifest.style(key).map_or(lib.manifest.version, |s| s.version); @@ -598,6 +605,7 @@ pub fn import_styles(doc: &mut Document, lib: &PublishedLibrary, keys: &BTreeSet { *slot = Style { id: r.local.clone(), ..style.clone() }; set_record(&mut doc.libraries_mut().styles, ImportedStyle { version, ..r.clone() }); + written.push((r.local.clone(), style)); } r.local } @@ -605,6 +613,7 @@ pub fn import_styles(doc: &mut Document, lib: &PublishedLibrary, keys: &BTreeSet let local = local_key(&lid, key, |l| doc.styles.iter().any(|s| s.id == l)); doc.styles.push(Style { id: local.clone(), ..style.clone() }); set_record(&mut doc.libraries_mut().styles, ImportedStyle { library: lid.clone(), key: key.clone(), local: local.clone(), version }); + written.push((local.clone(), style)); local } }; @@ -613,6 +622,17 @@ pub fn import_styles(doc: &mut Document, lib: &PublishedLibrary, keys: &BTreeSet } out.insert(key.clone(), local); } + let bound: BTreeSet = written.iter().flat_map(|(_, s)| s.bound.values().cloned()).collect(); + if !bound.is_empty() { + let vars = import_variables(doc, lib, &bound, &BTreeSet::new(), update); + for (local, _) in &written { + if let Some(slot) = doc.styles.iter_mut().find(|s| s.id == *local) { + // Bindings to variables the library lacks are dropped (the published value stays). + slot.bound = + std::mem::take(&mut slot.bound).into_iter().filter_map(|(f, v)| vars.variables.get(&v).map(|l| (f, l.clone()))).collect(); + } + } + } copy_images(doc, &lib.doc, &hashes); out } diff --git a/crates/engine/src/library_range_styles_tests.rs b/crates/engine/src/library_range_styles_tests.rs new file mode 100644 index 00000000..077470e7 --- /dev/null +++ b/crates/engine/src/library_range_styles_tests.rs @@ -0,0 +1,66 @@ +//! Text styles linked on character ranges travel with components (publish and place). + +use crate::Session; +use figcraft_doc::NodeId; +use serde_json::{Value, json}; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap_or_else(|e| panic!("{command}: {e}")) +} + +fn node(value: Value) -> NodeId { + NodeId(value["id"].as_u64().unwrap()) +} + +/// A component with "Hello" (chars 0..2 linked to a range style) and "World" (whole-layer style). +fn fixture() -> (Session, NodeId, String, String) { + let mut s = Session::new(); + let frame = node(run(&mut s, "frame.create", json!({"name":"Card", "width":100, "height":40}))); + let text = node(run(&mut s, "text.create", json!({"parent":frame, "text":"Hello"}))); + let range = run(&mut s, "style.create", json!({"type":"text", "name":"Emphasis", "value":{"fontSize":32}}))["id"].as_str().unwrap().to_owned(); + run(&mut s, "style.apply", json!({"id":text, "style":range, "start":0, "end":2})); + let whole_text = node(run(&mut s, "text.create", json!({"parent":frame, "text":"World"}))); + let whole = run(&mut s, "style.create", json!({"type":"text", "name":"Whole", "value":{"fontSize":24}}))["id"].as_str().unwrap().to_owned(); + run(&mut s, "style.apply", json!({"id":whole_text, "style":whole})); + let component = node(run(&mut s, "component.create", json!({"ids":[frame]}))); + let source = s.doc().unwrap().node(text).unwrap(); + let chars = source.kind.text().unwrap(); + assert!(source.styles.text.is_none()); + assert_eq!(chars.style_at(0).style_id.as_deref(), Some(range.as_str())); + assert_eq!(chars.style_at(2).style_id, None); + (s, component, range, whole) +} + +#[test] +fn component_publish_keeps_range_text_style() { + let (mut s, component, range, whole) = fixture(); + run(&mut s, "library.publish", json!({"include":[component]})); + let snapshot = &s.doc().unwrap().published.as_ref().unwrap().doc; + assert!(snapshot.style(&whole).is_some()); + assert!(snapshot.style(&range).is_some(), "character-range style missing from snapshot"); +} + +#[test] +fn component_place_imports_range_text_style() { + let (mut source, component, range, whole) = fixture(); + let publication = run(&mut source, "library.publish", json!({})); + let library = source.doc().unwrap().published.as_ref().unwrap().clone(); + assert!(library.doc.style(&range).is_some()); + let mut target = Session::new(); + let path = "figcraft://library.test/range-styles"; + target.libraries.insert(path, Ok(library)); + run(&mut target, "library.add", json!({"path":path})); + let instance = node(run(&mut target, "library.place", json!({"library":publication["id"], "component":component}))); + let doc = target.doc().unwrap(); + let children = &doc.node(instance).unwrap().children; + let ranged = doc.node(children[0]).unwrap().kind.text().unwrap(); + let full = doc.node(children[1]).unwrap().kind.text().unwrap(); + let whole_record = doc.libraries.styles.iter().find(|r| r.key == whole).unwrap(); + assert_eq!(full.style_at(0).style_id.as_deref(), Some(whole_record.local.as_str())); + assert_eq!(ranged.style_at(0).font_size, 32.0); + assert!(ranged.style_at(2).style_id.is_none()); + let record = doc.libraries.styles.iter().find(|r| r.key == range); + assert!(record.is_some(), "character-range style missing after placement"); + assert_eq!(ranged.style_at(0).style_id.as_deref(), record.map(|r| r.local.as_str())); + assert!(doc.style(ranged.style_at(0).style_id.as_deref().unwrap()).is_some()); +} diff --git a/crates/engine/src/library_variables_tests.rs b/crates/engine/src/library_variables_tests.rs index 4bbaad57..4fc778a7 100644 --- a/crates/engine/src/library_variables_tests.rs +++ b/crates/engine/src/library_variables_tests.rs @@ -314,3 +314,33 @@ fn library_variable_commands_fail_cleanly() { } assert!(s.doc().unwrap().libraries.is_empty()); } + +#[test] +fn library_style_bindings_ignore_unrelated_local_variables() { + let d = dir("style-bound"); + let path = path_str(&d.join("lib.figcraft")); + let mut lib = Session::new(); + ex(&mut lib, "variable.createCollection", json!({"name": "Tokens"})); + let var = ex(&mut lib, "variable.create", json!({"collection": "Tokens", "name": "Brand", "type": "color", "value": "#0000ff"}))["id"].clone(); + let style = ex(&mut lib, "style.create", json!({"type": "paint", "name": "Brand", "value": "#0000ff"}))["id"].clone(); + ex(&mut lib, "style.bind", json!({"style": style, "field": "fill", "variable": var})); + let lid = publish_to(&mut lib, &path); + + let mut s = Session::new(); + ex(&mut s, "variable.createCollection", json!({"name": "Local"})); + let local = ex(&mut s, "variable.create", json!({"collection": "Local", "name": "Unrelated", "type": "color", "value": "#ff0000"}))["id"].clone(); + assert_eq!(local, var, "the ids must collide for this test"); + let r = idv(&ex(&mut s, "shape.rectangle", json!({"width": 40, "height": 40}))); + ex(&mut s, "library.add", json!({"path": path})); + ex(&mut s, "library.applyStyle", json!({"library": lid, "style": "Brand", "id": r})); + assert_eq!(fill(&s, r), "0000FF"); + healthy(&s); + + // An updated style follows the library's variable, not the local one. + ex(&mut lib, "variable.set", json!({"variable": "Brand", "value": "#00ffff"})); + republish(&mut lib); + ex(&mut s, "library.acceptUpdates", json!({})); + assert_eq!(fill(&s, r), "00FFFF"); + healthy(&s); + let _ = std::fs::remove_dir_all(&d); +} diff --git a/crates/engine/src/page_target_tests.rs b/crates/engine/src/page_target_tests.rs new file mode 100644 index 00000000..fd88685d --- /dev/null +++ b/crates/engine/src/page_target_tests.rs @@ -0,0 +1,63 @@ +//! Page commands act on pages: a layer id (a rectangle, a frame) is refused before anything about +//! the layer changes, as `page.select` already does. + +use figcraft_doc::{NodeId, NodeKind}; +use serde_json::{Value, json}; + +use crate::Session; + +fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")) +} + +fn id_of(v: &Value) -> NodeId { + NodeId(v["id"].as_u64().unwrap()) +} + +fn name(s: &Session, id: NodeId) -> String { + s.doc().unwrap().get(id).unwrap().name.clone() +} + +#[test] +fn page_commands_reject_a_rectangle() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let r = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10}))); + let before = name(&s, r); + assert!(s.execute("page.select", &json!({"id": r})).is_err()); + assert!(s.execute("page.rename", &json!({"id": r, "name": "Not a page"})).is_err(), "rename refuses a layer"); + assert_eq!(name(&s, r), before, "the layer keeps its name"); + assert!(s.execute("page.setBackground", &json!({"id": r, "color": "#123456"})).is_err(), "background refuses a layer"); + assert!(s.execute("page.delete", &json!({"id": r})).is_err(), "delete refuses a layer"); + assert!(s.doc().unwrap().contains(r), "the layer is still there"); + assert!(s.doc().unwrap().contains(page)); +} + +#[test] +fn page_delete_keeps_a_frame_and_its_child() { + let mut s = Session::new(); + let f = id_of(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 100, "height": 100}))); + let c = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 5, "y": 5, "width": 10, "height": 10, "parent": f}))); + assert_eq!(s.doc().unwrap().get(c).unwrap().parent, Some(f)); + assert!(s.execute("page.delete", &json!({"id": f})).is_err()); + assert!(s.doc().unwrap().contains(f) && s.doc().unwrap().contains(c), "the frame and its child remain"); + assert!(s.execute("page.rename", &json!({"id": f, "name": "Not a page"})).is_err()); + assert_ne!(name(&s, f), "Not a page"); +} + +#[test] +fn page_commands_still_work_on_pages() { + let mut s = Session::new(); + let first = s.page().unwrap(); + let second = id_of(&ex(&mut s, "page.add", json!({}))); + ex(&mut s, "page.rename", json!({"id": second, "name": "Flows"})); + assert_eq!(name(&s, second), "Flows"); + ex(&mut s, "page.setBackground", json!({"id": second, "color": "#123456"})); + match s.doc().unwrap().get(second).unwrap().kind { + NodeKind::Page { background, .. } => assert_eq!(background.to_css_hex().to_uppercase(), "#123456"), + _ => panic!("not a page"), + } + ex(&mut s, "page.delete", json!({"id": second})); + assert!(!s.doc().unwrap().contains(second)); + assert!(s.execute("page.delete", &json!({"id": first})).is_err(), "the last ordinary page stays"); +} diff --git a/crates/engine/src/panels_behaviour_tests.rs b/crates/engine/src/panels_behaviour_tests.rs index 3866d592..d299b0df 100644 --- a/crates/engine/src/panels_behaviour_tests.rs +++ b/crates/engine/src/panels_behaviour_tests.rs @@ -425,3 +425,25 @@ fn duplicating_a_layer_named_past_the_counter_does_not_overflow() { assert_eq!(name(&s, dup), "Layer 18446744073709551615 2"); assert!(s.doc().unwrap().validate().is_empty(), "the document still checks out"); } + +#[test] +fn page_navigation_skips_dividers() { + // A trailing divider: Next page stays on the ordinary page. + let mut s = Session::new(); + let first = s.page().unwrap(); + let div = id(&ex(&mut s, "page.add", json!({"name": "-"}))); + assert!(s.execute("page.select", &json!({"id": div})).is_err(), "a divider isn't a page to open"); + let r = ex(&mut s, "view.nextPage", json!({})); + assert_eq!(s.page().unwrap(), first, "Next page doesn't open a trailing divider"); + assert_eq!(id_of(&r["page"]), Some(first), "the result names the current page"); + // A middle divider: Previous page crosses it, Next page crosses it back. + let later = id(&ex(&mut s, "page.add", json!({"name": "Later"}))); + assert_eq!(s.page().unwrap(), later); + let r = ex(&mut s, "view.prevPage", json!({})); + assert_eq!(s.page().unwrap(), first, "Previous page crosses a divider"); + assert_eq!(id_of(&r["page"]), Some(first)); + ex(&mut s, "view.nextPage", json!({})); + assert_eq!(s.page().unwrap(), later, "Next page crosses a divider"); + ex(&mut s, "view.nextPage", json!({})); + assert_eq!(s.page().unwrap(), later, "Next page stops on the last page"); +} diff --git a/crates/engine/src/shapebuilder.rs b/crates/engine/src/shapebuilder.rs index 09a6758a..99d59b45 100644 --- a/crates/engine/src/shapebuilder.rs +++ b/crates/engine/src/shapebuilder.rs @@ -322,7 +322,7 @@ fn apply(s: &mut Session, cmd: &str, p: &Value, merge: bool) -> Result { let whole_shape = touched.len() == 1 && a.regions.iter().enumerate().all(|(i, r)| pick.contains(&i) || !r.sources.iter().any(|x| touched.contains(x))); if merge && whole_shape { - return Ok(json!({"id": null, "changed": [], "removed": [], "regions": pick})); + return Ok(json!({"id": null, "changed": [], "removed": [], "outlines": [], "regions": pick})); } let doc = s.doc_mut()?; let created = if merge { diff --git a/crates/engine/src/synthetic.rs b/crates/engine/src/synthetic.rs index 0cb3e148..c2a9f5d6 100644 --- a/crates/engine/src/synthetic.rs +++ b/crates/engine/src/synthetic.rs @@ -66,7 +66,8 @@ pub fn large_document(layers: usize) -> DocResult<(Document, NodeId)> { made += 4; } // A card group (5 layers). - if budget(made) >= 5 { + // The card and the instances leave one layer for the grid, which starts with a rectangle. + if budget(made) > 5 { let card = doc.create(screen, NodeKind::Group, "Card", 12.0, 52.0, 160.0, 64.0)?; // Children are in the group's space; normalising fits the group around them. let cbg = doc.create(card, NodeKind::Rectangle, "Card background", 0.0, 0.0, 160.0, 64.0)?; @@ -78,7 +79,7 @@ pub fn large_document(layers: usize) -> DocResult<(Document, NodeId)> { } // Three button instances (3 layers each). for k in 0..3 { - if budget(made) < 3 { + if budget(made) < 3 + 1 { break; } crate::components::create_instance(&mut doc, button, screen, 12.0 + k as f64 * 108.0, SCREEN_H - 44.0)?; @@ -89,12 +90,14 @@ pub fn large_document(layers: usize) -> DocResult<(Document, NodeId)> { for k in 0..rest { let (gx, gy) = (184.0 + (k % 8) as f64 * 21.0, 52.0 + (k / 8) as f64 * 34.0); let hue = ((s * PER_SCREEN + k) % 360) as f32 / 360.0; - let id = match k % 5 { + // The grid starts with a rectangle so even the smallest workload has one. + let kind = (k + 2) % 5; + let id = match kind { 0 => doc.create(screen, text(&format!("Item {k}")), "Text", gx, gy, 20.0, 14.0)?, 1 | 3 => doc.create(screen, ellipse(), "Ellipse", gx, gy, 18.0, 18.0)?, _ => doc.create(screen, NodeKind::Rectangle, "Rectangle", gx, gy, 18.0, 18.0)?, }; - if k % 5 != 0 { + if kind != 0 { fill(&mut doc, id, Color::new(hue, 0.5, 1.0 - hue, 1.0)); if let Some(n) = doc.node_mut(id) { n.corners.radii = [3.0; 4]; @@ -124,4 +127,16 @@ mod tests { assert!(doc.page_content_bounds(page).is_some()); } } + + #[test] + fn small_documents_hold_the_benchmark_targets() { + for n in [10, 11, 12, 13, 14, 15] { + let (doc, page) = super::large_document(n).unwrap(); + let nodes: Vec<_> = doc.subtree(page).into_iter().filter_map(|id| doc.node(id)).collect(); + let has = |f: &dyn Fn(&figcraft_doc::Node) -> bool| nodes.iter().any(|node| f(node)); + assert!(has(&|x| matches!(x.kind, figcraft_doc::NodeKind::Rectangle) && x.name == "Rectangle"), "{n}: no rectangle"); + assert!(has(&|x| x.name == "Title"), "{n}: no title"); + assert!(has(&|x| matches!(x.kind, figcraft_doc::NodeKind::Component { .. })), "{n}: no component"); + } + } } diff --git a/crates/engine/src/textedit.rs b/crates/engine/src/textedit.rs index 6ba94217..5e5762b5 100644 --- a/crates/engine/src/textedit.rs +++ b/crates/engine/src/textedit.rs @@ -38,16 +38,17 @@ pub fn end(s: &mut Session) -> Result<()> { let _ = doc.remove(te.id); s.ds_mut()?.selection.clear(); } - s.commit_interaction()?; if empty && te.created { - // Nothing to undo for a text box that was never filled. + // Nothing to undo for a text box that was never filled. The gesture snapshot was taken + // after the layer was added, so it still holds it: re-base it on the document without the + // layer, and committing then records no step (and leaves the redo stack alone). let ds = s.ds_mut()?; - if let Some(last) = ds.history.undo.last() - && *last.doc == *ds.doc - { - ds.history.undo.pop(); + let doc = ds.doc.clone(); + if let Some(it) = ds.interaction.as_mut() { + it.doc = doc; } } + s.commit_interaction()?; let ds = s.ds_mut()?; ds.revision += 1; Ok(()) @@ -146,9 +147,15 @@ fn list_key(s: &mut Session, key: &str, shift: bool) -> Result { } /// Cmd/Ctrl+] and Cmd/Ctrl+[ while editing: the selected list items a level deeper or out, like -/// Tab and Shift+Tab. Returns whether anything changed (text that isn't a list stays as it is). +/// Tab and Shift+Tab. Returns whether anything changed (text that isn't a list and items already +/// at the deepest level stay as they are). pub fn list_indent(s: &mut Session, out: bool) -> Result { - list_key(s, "Tab", out) + let Some(id) = s.ds()?.text_edit.as_ref().map(|te| te.id) else { return Ok(false) }; + let before = text_of(s, id)?; + if !list_key(s, "Tab", out)? { + return Ok(false); + } + Ok(text_of(s, id)? != before) } /// The kind of word char `i` belongs to: 0 for spaces and punctuation, else its script, so a word diff --git a/crates/engine/src/theme_pages_tests.rs b/crates/engine/src/theme_pages_tests.rs index feaf4c6d..5172bb4b 100644 --- a/crates/engine/src/theme_pages_tests.rs +++ b/crates/engine/src/theme_pages_tests.rs @@ -182,3 +182,49 @@ fn text_drawn_on_a_dark_page_is_white_and_on_a_light_page_black() { let made = id_of(&ex(&mut s, "text.create", json!({"text": "Hi", "x": 0, "y": 300}))["id"]).unwrap(); assert_eq!(s.doc().unwrap().node(made).unwrap().fills, vec![Paint::solid(Color::WHITE)]); } + +#[test] +fn a_copy_of_a_page_keeps_its_flows_prototype_settings_and_guides() { + let mut s = Session::new(); + ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 100, "height": 100})); + let start = ex(&mut s, "frame.create", json!({"x": 200, "y": 0, "width": 100, "height": 100}))["id"].clone(); + ex(&mut s, "proto.flow.add", json!({"start": start, "name": "Checkout"})); + ex(&mut s, "proto.settings", json!({"device": "iPhone 16", "responsive": true})); + ex(&mut s, "guide.add", json!({"vertical": true, "offset": 25})); + ex(&mut s, "select.none", json!({})); + let source = s.page().unwrap(); + let copy = id_of(&ex(&mut s, "page.duplicate", json!({}))["id"]).unwrap(); + let frames = s.doc().unwrap().node(copy).unwrap().children.clone(); + let state = |s: &Session, id: NodeId| match &s.doc().unwrap().node(id).unwrap().kind { + NodeKind::Page { flows, prototype, guides, .. } => (flows.clone(), prototype.clone(), guides.clone()), + _ => panic!("expected a page"), + }; + let (flows, prototype, guides) = state(&s, copy); + let (src_flows, src_prototype, src_guides) = state(&s, source); + assert_eq!(flows.len(), 1); + assert_eq!(flows[0].name, "Checkout"); + assert_eq!(flows[0].start, frames[1], "the flow starts at the copied frame"); + assert_eq!(src_flows[0].start, id_of(&start).unwrap(), "the source keeps its own"); + assert_eq!(prototype, src_prototype); + assert_eq!(prototype.device, "iPhone 16"); + assert!(prototype.responsive); + assert_eq!(guides, src_guides); + assert_eq!(guides.len(), 1); + assert_eq!(ex(&mut s, "proto.present", json!({}))["start"].as_u64(), Some(frames[1].0)); +} + +#[test] +fn a_copy_of_a_page_keeps_a_flow_that_starts_in_a_section() { + let mut s = Session::new(); + let start = id_of(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 100, "height": 100}))["id"]).unwrap(); + ex(&mut s, "object.wrapInSection", json!({"ids": [start]})); + ex(&mut s, "proto.flow.add", json!({"start": start, "name": "Onboarding"})); + let copy = id_of(&ex(&mut s, "page.duplicate", json!({}))["id"]).unwrap(); + let doc = s.doc().unwrap(); + let section = doc.node(copy).unwrap().children[0]; + let frame = doc.node(section).unwrap().children[0]; + match &doc.node(copy).unwrap().kind { + NodeKind::Page { flows, .. } => assert_eq!(flows.iter().map(|f| (f.name.as_str(), f.start)).collect::>(), vec![("Onboarding", frame)]), + _ => panic!("expected a page"), + } +} diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index 4100d052..f71f347e 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -1468,7 +1468,8 @@ impl Session { } Gesture::Vector(_) | Gesture::ShapeBuild { .. } | Gesture::Origin => Ok(()), Gesture::PenDrag { anchor } => { - let out = ev.p() - *anchor; + // A click's wobble pulls out no handle. + let out = if within_slop(ev, *anchor) { Vec2::ZERO } else { ev.p() - *anchor }; if let Some(pen) = self.tool.pen.as_mut() && let Some(last) = pen.points.last_mut() { @@ -1870,9 +1871,10 @@ impl Session { self.widget_click(ev.p(), ev.tol() * 2.0)?; return Ok(()); } - // The second press began a move or a resize: a double-click isn't one. - if matches!(self.tool.gesture, Gesture::Move { .. } | Gesture::Resize { .. }) { + // The second press began a move, a resize, a shape or a marquee: a double-click isn't one. + if matches!(self.tool.gesture, Gesture::Move { .. } | Gesture::Resize { .. } | Gesture::Draw { .. } | Gesture::Marquee { .. }) { self.tool.gesture = Gesture::None; + self.tool.overlay.marquee = None; self.cancel_interaction()?; } let Some(leaf) = self.hit(&doc, page, ev.p(), ev.tol() * 2.0) else { return Ok(()) }; diff --git a/crates/engine/src/variables_editor_tests.rs b/crates/engine/src/variables_editor_tests.rs index 815fcb2b..9518911e 100644 --- a/crates/engine/src/variables_editor_tests.rs +++ b/crates/engine/src/variables_editor_tests.rs @@ -114,3 +114,14 @@ fn field_scopes_by_property_and_layer() { assert_eq!(scope(f, "paddingHorizontal"), Some("GAP")); assert_eq!(scope(f, "visible"), None); } + +#[test] +fn create_collection_refuses_a_taken_name() { + let mut s = Session::new(); + ex(&mut s, "variable.createCollection", json!({"name": "Theme"})); + let before = s.doc().unwrap().variables.collections.len(); + assert!(s.execute("variable.createCollection", &json!({"name": "Theme"})).is_err()); + assert_eq!(s.doc().unwrap().variables.collections.len(), before); + ex(&mut s, "variable.createCollection", json!({"name": "Theme 2"})); + assert_eq!(s.doc().unwrap().variables.collections.len(), before + 1); +} diff --git a/crates/engine/src/vectoredit/ops.rs b/crates/engine/src/vectoredit/ops.rs index 093d0854..935f033a 100644 --- a/crates/engine/src/vectoredit/ops.rs +++ b/crates/engine/src/vectoredit/ops.rs @@ -3,7 +3,7 @@ //! `vectoredit.rs` re-fit the node afterwards. use figcraft_doc::*; -use figcraft_geom::kurbo::{CubicBez, ParamCurve}; +use figcraft_geom::kurbo::{CubicBez, ParamCurve, ParamCurveNearest}; use figcraft_geom::{Affine, Point, Vec2}; fn tvec(t: (f64, f64)) -> Vec2 { @@ -236,6 +236,12 @@ pub fn bend(net: &mut VectorNetwork, seg: usize, t: f64, p: Point) { pub fn split_segment(net: &mut VectorNetwork, seg: usize, t: f64) -> Option { let s = net.segments.get(seg).copied()?; let c: CubicBez = net.cubic(seg)?; + // Which way each region loop walks `seg`, read before the split changes its endpoints. + let reversed: Vec> = net + .regions + .iter() + .map(|reg| reg.loops.iter().map(|lp| net.loop_walk(lp).iter().find(|(i, _)| *i == seg).is_some_and(|(_, rev)| *rev)).collect()) + .collect(); let t = if t.is_finite() { t.clamp(0.01, 0.99) } else { 0.5 }; let l = c.subsegment(0.0..t); let r = c.subsegment(t..1.0); @@ -263,10 +269,12 @@ pub fn split_segment(net: &mut VectorNetwork, seg: usize, t: f64) -> Option Vec<(f64, if ia { b } else { a } }; let n = 64; + // Upper bound on the curve's speed: no point of a sample interval is farther than the whole + // interval length from either end, so only intervals that come that close need a closer look. + let speed = 3.0 * [(cb.p1 - cb.p0).hypot(), (cb.p2 - cb.p1).hypot(), (cb.p3 - cb.p2).hypot()].into_iter().fold(0.0, f64::max); + let reach = speed / f64::from(n); + // A crossing narrower than an interval whose ends are both outside: the closest point. + let dip = |a: f64, b: f64| -> Option { + if (cb.eval(a) - c).hypot().min((cb.eval(b) - c).hypot()) > r + reach { + return None; + } + let near = cb.subsegment(a..b).nearest(c, 1e-12); + let m = a + (b - a) * near.t; + (m > a && m < b && inside(m)).then_some(m) + }; let mut out = Vec::new(); let mut start: Option = if inside(0.0) { Some(0.0) } else { None }; let mut prev = 0.0; @@ -773,6 +794,11 @@ fn inside_ranges(net: &VectorNetwork, seg: usize, c: Point, r: f64) -> Vec<(f64, let t = f64::from(i) / f64::from(n); match (start, inside(t)) { (None, true) => start = Some(edge(prev, t)), + (None, false) => { + if let Some(m) = dip(prev, t) { + out.push((edge(prev, m), edge(m, t))); + } + } (Some(s0), false) => { out.push((s0, edge(prev, t))); start = None; @@ -856,6 +882,38 @@ mod tests { assert!((a + b).hypot() < 1e-9, "made symmetric: {a:?} {b:?}"); } + fn loop_is_continuous(n: &VectorNetwork, lp: &[usize]) -> bool { + let ends: Vec<(usize, usize)> = n + .loop_walk(lp) + .iter() + .map(|&(i, rev)| { + let s = n.segments[i]; + if rev { (s.b, s.a) } else { (s.a, s.b) } + }) + .collect(); + (0..ends.len()).all(|k| ends[k].1 == ends[(k + 1) % ends.len()].0) + } + + #[test] + fn split_keeps_region_loop_in_ring_order() { + // Triangle closed 0→2: its loop [2, 1, 0] walks segment 1 backwards. + let mut n = VectorNetwork::from_points(&[Point::new(0.0, 0.0), Point::new(100.0, 0.0), Point::new(100.0, 100.0)], false); + n.segments.push(Segment::line(0, 2)); + n.regions.push(Region { loops: vec![vec![2, 1, 0]], winding: WindingRule::NonZero }); + let p = Point::new(25.0, 10.0); + assert!(n.fill_contains(p)); + for (seg, lp) in [(1, vec![2, 1, 0]), (2, vec![2, 1, 0]), (1, vec![0, 1, 2])] { + let mut m = n.clone(); + m.regions[0].loops = vec![lp]; + let mid = split_segment(&mut m, seg, 0.5).unwrap(); + assert!(loop_is_continuous(&m, &m.regions[0].loops[0]), "loop {:?}", m.regions[0].loops); + assert!(m.fill_contains(p), "split of {seg} removed {p:?} from the fill"); + if seg == 1 { + assert_eq!(m.vertices[mid].point(), Point::new(100.0, 50.0)); + } + } + } + #[test] fn inferred_mirroring() { let mut n = open_line(); @@ -999,4 +1057,38 @@ mod tests { assert!(!erase(&mut m, Point::new(f64::NAN, 0.0), 5.0)); assert!(!erase(&mut m, Point::new(0.0, 0.0), -1.0)); } + + #[test] + fn eraser_catches_narrow_crossing_between_samples() { + // A radius-1 brush on a long line: the crossing fits between the sampled parameters. + let mut n = VectorNetwork::from_points(&[Point::new(0.0, 0.0), Point::new(6400.0, 0.0)], false); + assert!(erase(&mut n, Point::new(5050.0, 0.0), 1.0)); + assert_eq!(n.segments.len(), 2); + let ends: Vec = n.vertices.iter().map(|v| v.x).collect(); + assert!(ends.iter().any(|x| (x - 5049.0).abs() < 0.01) && ends.iter().any(|x| (x - 5051.0).abs() < 0.01), "{ends:?}"); + assert!(ends.iter().any(|x| x.abs() < 1e-9) && ends.iter().any(|x| (x - 6400.0).abs() < 1e-9), "{ends:?}"); + // A curved segment that only grazes the brush between samples. + let mut c = VectorNetwork::from_points(&[Point::new(0.0, 0.0), Point::new(6400.0, 0.0)], false); + c.segments[0].ta = (0.0, 3000.0); + c.segments[0].tb = (0.0, 3000.0); + let cb = c.cubic(0).unwrap(); + let t = 0.7039; + let p = cb.eval(t); + let d = figcraft_geom::kurbo::ParamCurveDeriv::deriv(&cb).eval(t).to_vec2().normalize(); + let normal = Vec2::new(-d.y, d.x); + assert!(erase(&mut c, p + normal * 0.5, 1.0)); + assert_eq!(c.segments.len(), 2); + } + + #[test] + fn eraser_catches_crossing_at_full_speed_between_samples() { + // The line moves fastest at t = 0.5; centre a tiny brush halfway between two samples there. + let mut n = VectorNetwork::from_points(&[Point::new(0.0, 0.0), Point::new(6400.0, 0.0)], false); + let centre = n.cubic(0).unwrap().eval(32.5 / 64.0); + assert!(erase(&mut n, centre, 0.5)); + assert_eq!(n.segments.len(), 2); + let ends: Vec = n.vertices.iter().map(|v| v.x).collect(); + assert!(ends.iter().any(|x| (x - (centre.x - 0.5)).abs() < 0.01), "{ends:?}"); + assert!(ends.iter().any(|x| (x - (centre.x + 0.5)).abs() < 0.01), "{ends:?}"); + } } diff --git a/crates/engine/src/view_behaviour_tests.rs b/crates/engine/src/view_behaviour_tests.rs index c22cd1f5..27bc3771 100644 --- a/crates/engine/src/view_behaviour_tests.rs +++ b/crates/engine/src/view_behaviour_tests.rs @@ -456,6 +456,23 @@ fn a_selected_guide_is_deleted_and_nudged_with_the_keyboard() { assert!(s.execute("guide.select", &json!({"index": 7})).is_err(), "only an existing guide can be selected"); } +#[test] +fn removing_an_earlier_guide_keeps_the_selected_guide() { + let mut s = Session::new(); + for offset in [100, 200, 300] { + ex(&mut s, "guide.add", json!({"vertical": true, "offset": offset})); + } + let page = s.page().unwrap(); + ex(&mut s, "guide.select", json!({"index": 1})); + ex(&mut s, "guide.remove", json!({"index": 0})); + assert_eq!(s.ds().unwrap().selected_guide(), Some((page, 0)), "the guide at 200 stays selected at its new index"); + assert!(s.tool_key("ArrowRight", false, false, false).unwrap()); + assert_eq!(guide_list(&s, page), vec![(true, 201.0), (true, 300.0)], "the arrow nudges the selected guide, not its neighbour"); + // A later guide leaves the selection alone. + ex(&mut s, "guide.remove", json!({"index": 1})); + assert_eq!(s.ds().unwrap().selected_guide(), Some((page, 0))); +} + #[test] fn guides_snap_and_measure() { let mut s = Session::new(); diff --git a/crates/engine/tests/empty_text_box_undo.rs b/crates/engine/tests/empty_text_box_undo.rs new file mode 100644 index 00000000..ec73b83a --- /dev/null +++ b/crates/engine/tests/empty_text_box_undo.rs @@ -0,0 +1,60 @@ +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use figcraft_engine::{ + Session, + doc::NodeKind, + tools::{PointerEvent, PointerKind}, +}; +use serde_json::json; + +fn text_count(s: &Session) -> usize { + let doc = s.doc().unwrap(); + doc.node(s.page().unwrap()).unwrap().children.iter().filter(|id| matches!(doc.node(**id).unwrap().kind, NodeKind::Text(_))).count() +} + +fn rect_count(s: &Session) -> usize { + let doc = s.doc().unwrap(); + doc.node(s.page().unwrap()).unwrap().children.len() - text_count(s) +} + +fn click_and_abandon(s: &mut Session) { + s.execute("tool.text", &json!({})).unwrap(); + s.pointer(&PointerEvent::at(PointerKind::Down, 300.0, 300.0, 1.0)).unwrap(); + s.pointer(&PointerEvent::at(PointerKind::Up, 300.0, 300.0, 1.0)).unwrap(); + assert_eq!(text_count(s), 1); + s.execute("text.edit.end", &json!({})).unwrap(); + assert_eq!(text_count(s), 0); +} + +#[test] +fn abandoning_empty_text_does_not_create_an_undo_step() { + let mut s = Session::new(); + assert_eq!(s.undo_depth(), (0, 0)); + click_and_abandon(&mut s); + assert_eq!(s.undo_depth(), (0, 0)); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(text_count(&s), 0, "Undo restored the abandoned empty layer"); +} + +#[test] +fn abandoning_empty_text_keeps_the_previous_undo_step_first() { + let mut s = Session::new(); + s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + click_and_abandon(&mut s); + assert_eq!(s.undo_labels(2), vec!["Rectangle".to_string()]); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(text_count(&s), 0); + assert_eq!(rect_count(&s), 0, "the first Undo must remove the rectangle"); +} + +#[test] +fn abandoning_empty_text_keeps_the_redo_stack() { + let mut s = Session::new(); + s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.undo_depth(), (0, 1)); + click_and_abandon(&mut s); + assert_eq!(s.undo_depth(), (0, 1), "the document is back where it was, so Redo still applies"); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(rect_count(&s), 1); +} diff --git a/crates/engine/tests/library_style_acceptance.rs b/crates/engine/tests/library_style_acceptance.rs new file mode 100644 index 00000000..8398fc72 --- /dev/null +++ b/crates/engine/tests/library_style_acceptance.rs @@ -0,0 +1,72 @@ +//! Accepting a library update that changes only a paint style refreshes the layers linked to it. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use figcraft_doc::{NodeId, StyleKind}; +use figcraft_engine::Session; +use serde_json::{Value, json}; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap() +} + +fn layer_paint(s: &Session, id: NodeId) -> String { + s.doc().unwrap().node(id).unwrap().fills[0].solid_color().unwrap().to_hex() +} + +fn style_paint(s: &Session, id: &str) -> String { + match &s.doc().unwrap().style(id).unwrap().kind { + StyleKind::Paint { paints } => paints[0].solid_color().unwrap().to_hex(), + _ => panic!("expected paint style"), + } +} + +struct Fixture(std::path::PathBuf); + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +#[test] +fn accepted_library_paint_reaches_linked_layer() { + let fixture = Fixture(std::env::temp_dir().join(format!("figcraft-style-accept-{}", std::process::id()))); + let _ = std::fs::remove_dir_all(&fixture.0); + std::fs::create_dir(&fixture.0).unwrap(); + let path = fixture.0.join("library.figcraft"); + let mut publisher = Session::new(); + run(&mut publisher, "style.create", json!({"type":"paint","name":"Brand","value":"#0000ff"})); + let publication = run(&mut publisher, "library.publish", json!({})); + assert_eq!(publication["components"], 0); + assert_eq!(publication["styles"], 1); + run(&mut publisher, "file.saveAs", json!({"path":path})); + let mut consumer = Session::new(); + let rectangle = run(&mut consumer, "shape.rectangle", json!({"width":40,"height":40})); + let layer = NodeId(rectangle["id"].as_u64().unwrap()); + run(&mut consumer, "library.add", json!({"path":path})); + let applied = run(&mut consumer, "library.applyStyle", json!({"library":publication["id"],"style":"Brand","id":layer})); + let style = applied["style"].as_str().unwrap(); + assert_eq!(layer_paint(&consumer, layer), "0000FF"); + run(&mut publisher, "style.update", json!({"style":"Brand","value":"#00ff00"})); + run(&mut publisher, "library.publish", json!({})); + run(&mut publisher, "file.save", json!({})); + assert_eq!(run(&mut consumer, "library.updates", json!({}))["count"], 1); + let accepted = run(&mut consumer, "library.acceptUpdates", json!({})); + assert_eq!(accepted["styles"], 1); + assert_eq!(accepted["components"], 0); + assert_eq!(accepted["variables"], 0); + assert_eq!(style_paint(&consumer, style), "00FF00"); + assert_eq!(consumer.doc().unwrap().node(layer).unwrap().styles.fill.as_deref(), Some(style)); + // Captured before any reapply so a later action can't hide a missed refresh. + let after_accept = layer_paint(&consumer, layer); + assert_eq!(after_accept, "00FF00"); + assert_eq!(run(&mut consumer, "library.updates", json!({}))["count"], 0); + consumer.undo().unwrap(); + assert_eq!(style_paint(&consumer, style), "0000FF"); + assert_eq!(layer_paint(&consumer, layer), "0000FF"); + consumer.redo().unwrap(); + assert_eq!(style_paint(&consumer, style), "00FF00"); + assert_eq!(layer_paint(&consumer, layer), "00FF00"); + run(&mut consumer, "style.apply", json!({"style":style,"id":layer})); + assert_eq!(layer_paint(&consumer, layer), "00FF00"); +} diff --git a/crates/engine/tests/list_indent_response.rs b/crates/engine/tests/list_indent_response.rs new file mode 100644 index 00000000..1dae8754 --- /dev/null +++ b/crates/engine/tests/list_indent_response.rs @@ -0,0 +1,71 @@ +//! `text.listIndent` / `text.listOutdent` report `changed` only when the document changed. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use figcraft_engine::{ + Session, + doc::{ListStyle, NodeId, NodeKind}, +}; +use serde_json::json; + +fn editing_list(indent: u8) -> (Session, NodeId) { + let mut s = Session::new(); + let value = s.execute("node.create", &json!({"type":"text", "props":{"characters":"a"}})).unwrap(); + let id = NodeId(value["id"].as_u64().unwrap()); + s.execute("text.listStyle", &json!({"ids":[id.0], "style":"bulleted"})).unwrap(); + s.execute("node.set", &json!({"id":id.0, "props":{"indentation":indent}})).unwrap(); + s.execute("text.edit.begin", &json!({"id":id.0, "caret":0})).unwrap(); + (s, id) +} + +fn list_state(s: &Session, id: NodeId) -> (ListStyle, u8) { + let NodeKind::Text(t) = &s.doc().unwrap().node(id).unwrap().kind else { panic!("not text") }; + let p = t.para_list(0); + (p.list, p.indent) +} + +#[test] +fn indent_at_cap_reports_false() { + let (mut s, id) = editing_list(8); + let before = s.doc().unwrap().clone(); + let result = s.execute("text.listIndent", &json!({})).unwrap(); + assert_eq!(list_state(&s, id), (ListStyle::Bulleted, 8)); + assert_eq!(before, *s.doc().unwrap()); + assert_eq!(result["changed"], json!(false)); +} + +#[test] +fn indent_from_zero_reports_true() { + let (mut s, id) = editing_list(0); + let result = s.execute("text.listIndent", &json!({})).unwrap(); + assert_eq!(list_state(&s, id), (ListStyle::Bulleted, 1)); + assert_eq!(result["changed"], json!(true)); +} + +#[test] +fn outdent_from_zero_reports_true() { + let (mut s, id) = editing_list(0); + let result = s.execute("text.listOutdent", &json!({})).unwrap(); + assert_eq!(list_state(&s, id).0, ListStyle::None); + assert_eq!(result["changed"], json!(true)); +} + +#[test] +fn indent_plain_text_reports_false() { + let mut s = Session::new(); + let value = s.execute("node.create", &json!({"type":"text", "props":{"characters":"a"}})).unwrap(); + let id = NodeId(value["id"].as_u64().unwrap()); + s.execute("text.edit.begin", &json!({"id":id.0, "caret":0})).unwrap(); + let before = s.doc().unwrap().clone(); + let result = s.execute("text.listIndent", &json!({})).unwrap(); + assert_eq!(before, *s.doc().unwrap()); + assert_eq!(result["changed"], json!(false)); +} + +#[test] +fn tab_keys_still_nest_and_unnest() { + let (mut s, id) = editing_list(0); + s.execute("text.edit.key", &json!({"key":"Tab"})).unwrap(); + assert_eq!(list_state(&s, id), (ListStyle::Bulleted, 1)); + s.execute("text.edit.key", &json!({"key":"Tab", "shift":true})).unwrap(); + assert_eq!(list_state(&s, id), (ListStyle::Bulleted, 0)); +} diff --git a/crates/engine/tests/scope_reset.rs b/crates/engine/tests/scope_reset.rs new file mode 100644 index 00000000..6d875d12 --- /dev/null +++ b/crates/engine/tests/scope_reset.rs @@ -0,0 +1,26 @@ +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use figcraft_engine::Session; +use serde_json::json; + +fn scopes(session: &Session, variable: &str) -> Option> { + session.doc().unwrap().variables.get(variable).unwrap().scopes.clone() +} + +#[test] +fn null_restores_all_supported_properties() { + let mut session = Session::new(); + session.execute("variable.createCollection", &json!({"name": "Theme"})).unwrap(); + let result = session.execute("variable.create", &json!({"collection": "Theme", "name": "Accent", "type": "color"})).unwrap(); + let variable = result["id"].as_str().unwrap(); + assert_eq!(scopes(&session, variable), None); + session.execute("variable.update", &json!({"variable": variable, "scopes": []})).unwrap(); + assert_eq!(scopes(&session, variable), Some(vec![])); + session.execute("variable.update", &json!({"variable": variable, "name": "Accent2"})).unwrap(); + assert_eq!(scopes(&session, variable), Some(vec![])); + assert_eq!(session.execute("variable.update", &json!({"variable": variable, "scopes": null})).unwrap(), json!({"ok": true})); + assert_eq!(scopes(&session, variable), None); + session.execute("variable.update", &json!({"variable": variable, "scopes": []})).unwrap(); + session.execute("variable.update", &json!({"variable": variable, "scopes": ["ALL_SCOPES"]})).unwrap(); + assert_eq!(scopes(&session, variable), None); +} diff --git a/crates/engine/tests/shape_builder_response.rs b/crates/engine/tests/shape_builder_response.rs new file mode 100644 index 00000000..e1426823 --- /dev/null +++ b/crates/engine/tests/shape_builder_response.rs @@ -0,0 +1,36 @@ +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +use figcraft_engine::Session; +use serde_json::{Value, json}; + +fn rectangle(s: &mut Session, x: f64) -> Value { + s.execute("shape.rectangle", &json!({"x": x, "y": 0, "width": 100, "height": 100})).unwrap()["id"].clone() +} + +#[test] +fn merge_response_includes_outlines_for_no_op() { + let mut s = Session::new(); + let id = rectangle(&mut s, 0.0); + let regions = s.execute("shapeBuilder.regions", &json!({"ids": [id]})).unwrap(); + assert_eq!(regions["regions"].as_array().unwrap().len(), 1); + let before = serde_json::to_value(s.doc().unwrap()).unwrap(); + let result = s.execute("shapeBuilder.merge", &json!({"ids": [id], "regions": [0]})).unwrap(); + assert_eq!(serde_json::to_value(s.doc().unwrap()).unwrap(), before); + assert_eq!(result["id"], Value::Null); + assert_eq!(result["changed"], json!([])); + assert_eq!(result["removed"], json!([])); + assert_eq!(result["regions"], json!([0])); + assert_eq!(result.get("outlines"), Some(&json!([]))); +} + +#[test] +fn merge_response_control_includes_outlines() { + let mut s = Session::new(); + let a = rectangle(&mut s, 0.0); + let b = rectangle(&mut s, 50.0); + let regions = s.execute("shapeBuilder.regions", &json!({"ids": [a, b]})).unwrap(); + assert_eq!(regions["regions"].as_array().unwrap().len(), 3); + let result = s.execute("shapeBuilder.merge", &json!({"ids": [a, b], "regions": [1]})).unwrap(); + assert!(result["id"].is_u64()); + assert_eq!(result.get("outlines"), Some(&json!([]))); + assert_eq!(result.as_object().unwrap().len(), 5); +} diff --git a/crates/format/src/fig/assets_tests.rs b/crates/format/src/fig/assets_tests.rs index f3b869ff..4777be18 100644 --- a/crates/format/src/fig/assets_tests.rs +++ b/crates/format/src/fig/assets_tests.rs @@ -450,6 +450,38 @@ fn shared_styles_and_their_links_round_trip() { assert_eq!(named(&nodes, "Title")["styleIdForText"]["guid"], named(&nodes, "Heading")["guid"]); } +#[test] +fn a_styled_layers_fill_variable_survives_a_round_trip() { + let mut doc = Document::new("Overlap"); + let page = doc.first_page().unwrap(); + let mode = |id: &str, name: &str| Mode { id: id.into(), name: name.into() }; + doc.variables.collections = + vec![Collection { id: "c-theme".into(), name: "Theme".into(), modes: vec![mode("m-light", "Light"), mode("m-dark", "Dark")], extends: None }]; + let light = Color::new(0.1, 0.3, 0.7, 1.0); + let dark = VariableValue::Color(Color::new(0.8, 0.9, 1.0, 1.0)); + doc.variables.variables = + vec![var("v-accent", "Accent color", "c-theme", VariableType::Color, &[("m-light", VariableValue::Color(light)), ("m-dark", dark)])]; + doc.styles = vec![Style { + id: "s-accent".into(), + name: "Accent".into(), + description: String::new(), + kind: StyleKind::Paint { paints: vec![Paint::solid(light)] }, + bound: Default::default(), + }]; + add(&mut doc, page, NodeKind::Rectangle, "Swatch", (0.0, 0.0), (120.0, 80.0), &|n| { + n.fills = vec![Paint::solid(light)]; + n.styles.fill = Some("s-accent".into()); + n.bound.insert("fill".into(), "v-accent".into()); + }); + let (back, _) = import(&export(&doc, None).unwrap().0, "x").unwrap(); + let var = back.variables.variables.iter().find(|v| v.name == "Accent color").unwrap().id.clone(); + let swatch = back.nodes().find(|n| n.name == "Swatch").unwrap(); + let on_layer = swatch.bound.get("fill") == Some(&var) || swatch.bound.get("fills/0/color") == Some(&var); + let style = back.styles.iter().find(|s| Some(&s.id) == swatch.styles.fill.as_ref()).unwrap(); + let on_style = style.bound.get("fills/0/color") == Some(&var); + assert!(on_layer || on_style, "the fill binding was lost: layer {:?}, style {:?}", swatch.bound, style.bound); +} + #[test] fn text_styles_on_part_of_a_text_are_written_as_values() { let mut doc = styled(); diff --git a/crates/format/src/fig/export.rs b/crates/format/src/fig/export.rs index 22243355..526ab3d9 100644 --- a/crates/format/src/fig/export.rs +++ b/crates/format/src/fig/export.rs @@ -1429,9 +1429,15 @@ impl<'d> Exporter<'d> { } } - /// How a layer sizes and sits in its auto layout parent. + /// How a layer sizes and sits in its auto layout parent. Min/max sizes are the layer's own + /// and are written whatever its parent is. fn child_layout(&mut self, n: &Node, parent: Option<&Node>, f: &mut Fields) { let cl = &n.child_layout; + for (key, w, h) in [("minSize", cl.min_w, cl.min_h), ("maxSize", cl.max_w, cl.max_h)] { + if w.is_some() || h.is_some() { + f.push((key, o(vec![("value", xy(w.unwrap_or(0.0), h.unwrap_or(0.0)))]))); + } + } let Some(pl) = parent.and_then(|p| p.layout.as_ref()) else { return }; let (fill_h, fill_v) = (cl.sizing_h == Sizing::Fill, cl.sizing_v == Sizing::Fill); let (grow, stretch) = if pl.mode == LayoutMode::Vertical { (fill_v, fill_h) } else { (fill_h, fill_v) }; @@ -1444,11 +1450,6 @@ impl<'d> Exporter<'d> { if cl.absolute { f.push(("stackPositioning", e("ABSOLUTE"))); } - for (key, w, h) in [("minSize", cl.min_w, cl.min_h), ("maxSize", cl.max_w, cl.max_h)] { - if w.is_some() || h.is_some() { - f.push((key, o(vec![("value", xy(w.unwrap_or(0.0), h.unwrap_or(0.0)))]))); - } - } if pl.mode == LayoutMode::Grid && let Some((cols, rows)) = parent.and_then(|p| self.tracks.get(&self.main_layer(p.id))) { diff --git a/crates/format/src/fig/export_tests.rs b/crates/format/src/fig/export_tests.rs index 8d66b72f..67215b0c 100644 --- a/crates/format/src/fig/export_tests.rs +++ b/crates/format/src/fig/export_tests.rs @@ -1014,6 +1014,37 @@ fn junk_documents_never_panic() { } } +/// A frame's own min/max size survives whatever its parent is, not only under auto layout. +#[test] +fn frame_size_limits_survive_without_auto_layout_parent() { + fn limits(doc: &Document) -> [Option; 4] { + let (bytes, ew) = export(doc, None).unwrap(); + let (back, iw) = import(&bytes, "File").unwrap(); + assert!(ew.is_empty() && iw.is_empty(), "export {ew:?}, import {iw:?}"); + let cl = &back.nodes().find(|n| n.name == "Subject").unwrap().child_layout; + [cl.min_w, cl.max_w, cl.min_h, cl.max_h] + } + let mut doc = Document::new("File"); + let page = doc.first_page().unwrap(); + let id = doc.create(page, NodeKind::Frame, "Subject", 0.0, 0.0, 100.0, 100.0).unwrap(); + let node = doc.node_mut(id).unwrap(); + node.layout = Some(AutoLayout::default()); + node.child_layout.min_w = Some(80.0); + node.child_layout.max_w = Some(150.0); + node.child_layout.min_h = Some(60.0); + node.child_layout.max_h = Some(170.0); + let expected = [Some(80.0), Some(150.0), Some(60.0), Some(170.0)]; + + let mut nested = doc.clone(); + let wrapper = nested.create(page, NodeKind::Frame, "Wrapper", 0.0, 0.0, 300.0, 300.0).unwrap(); + nested.node_mut(wrapper).unwrap().layout = Some(AutoLayout::default()); + nested.reparent(id, wrapper, None).unwrap(); + assert_eq!(limits(&nested), expected, "auto layout parent"); + nested.node_mut(wrapper).unwrap().layout = None; + assert_eq!(limits(&nested), expected, "plain frame parent"); + assert_eq!(limits(&doc), expected, "page parent"); +} + /// Link each instance's layers to its main's by position, as they are imported (the engine /// links them by path after loading). fn link(doc: &mut Document) { diff --git a/crates/format/src/fig/map.rs b/crates/format/src/fig/map.rs index e0642f32..1e365567 100644 --- a/crates/format/src/fig/map.rs +++ b/crates/format/src/fig/map.rs @@ -643,13 +643,17 @@ impl<'a> Mapper<'a> { } let t = v.transform().unwrap_or_default(); o.insert("relativeTransform".into(), rows(space * t)); - // Paints, from the shared style when one is linked (the layer's own copy can be stale). + // Paints, from the shared style when one is linked (the layer's own copy can be stale), + // unless the layer's own paints are bound to variables: those bindings must not be lost. let mut styles = Obj::new(); + let own = |k: &str| Some(v.list(k)).filter(|l| l.iter().any(paint_bound)); let fill_style = self.style(v, "styleIdForFill", "inheritFillStyleID", "fill", &mut styles); - let fills = fill_style.map_or_else(|| v.list("fillPaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default()); + let fills = own("fillPaints") + .unwrap_or_else(|| fill_style.map_or_else(|| v.list("fillPaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default())); o.insert("fills".into(), Value::Array(fills.iter().filter_map(paint).collect())); let stroke_style = self.style(v, "styleIdForStrokeFill", "inheritFillStyleIDForStroke", "stroke", &mut styles); - let strokes = stroke_style.map_or_else(|| v.list("strokePaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default()); + let strokes = own("strokePaints") + .unwrap_or_else(|| stroke_style.map_or_else(|| v.list("strokePaints"), |s| s.get("fillPaints").map(Kv::list).unwrap_or_default())); o.insert("strokes".into(), Value::Array(strokes.iter().filter_map(paint).collect())); let effect_style = self.style(v, "styleIdForEffect", "inheritEffectStyleID", "effect", &mut styles); let effects = effect_style.map_or_else(|| v.list("effects"), |s| s.get("effects").map(Kv::list).unwrap_or_default()); @@ -1564,6 +1568,11 @@ fn child_layout(o: &mut Obj, v: &View, parent: Option<&View>) { } } +/// Whether a paint's colour or one of its gradient stops is bound to a variable. +fn paint_bound(p: &Kv) -> bool { + alias(p.get("colorVar")).is_some() || p.get("stopsVar").map(Kv::list).unwrap_or_default().iter().any(|s| alias(s.get("colorVar")).is_some()) +} + fn paint(p: &Kv) -> Option { let ty = p.get("type").and_then(Kv::str)?; let mut o = Obj::new(); diff --git a/crates/format/src/rest/text.rs b/crates/format/src/rest/text.rs index 5fbb368d..7b97d215 100644 --- a/crates/format/src/rest/text.rs +++ b/crates/format/src/rest/text.rs @@ -126,7 +126,10 @@ fn type_style(cx: &mut Ctx, s: &Value, base: &TextStyle) -> TextStyle { } if let Some(h) = s.get("hyperlink") { match (text(h, "type"), text(h, "url")) { - (Some("URL") | None, Some(url)) if !url.is_empty() => t.link = Some(super::props::clip_str(url).to_string()), + // The destination is data, not a label: `clip_str` is for names that go into warnings, + // and clipping a URL changed it into a different, working-looking one without a word + // (#459). The FIG importer already keeps it whole. + (Some("URL") | None, Some(url)) if !url.is_empty() => t.link = Some(url.to_string()), _ => {} } } diff --git a/crates/format/tests/import_link_contract.rs b/crates/format/tests/import_link_contract.rs new file mode 100644 index 00000000..30a3408a --- /dev/null +++ b/crates/format/tests/import_link_contract.rs @@ -0,0 +1,57 @@ +//! A text hyperlink's destination survives a REST import whole, however long it is (#459). +//! +//! The assertions are the reporter's; the helper returns its findings instead of unwrapping, so +//! that the crate's `unwrap_used`/`panic` lints hold outside a `#[test]` function. + +use figcraft_doc::NodeKind; +use figcraft_format::rest; +use serde_json::json; + +/// The link of the imported text, and the warnings the import reported. `None` for a link means +/// the layer was missing, was not text, or carried no link at all. +fn imported_link(length: usize, override_style: bool) -> (String, Option, Vec) { + let url = format!("https://example.com/{}", "a".repeat(length - 20)); + assert_eq!(url.len(), length); + let mut text = json!({"id":"1:2","type":"TEXT","name":"Link","characters":"A","relativeTransform":[[1,0,0],[0,1,0]]}); + let style = json!({"hyperlink":{"type":"URL","url":url}}); + if override_style { + text["characterStyleOverrides"] = json!([1]); + text["styleOverrideTable"] = json!({"1":style}); + } else { + text["style"] = style; + } + let input = json!({"document":{"type":"DOCUMENT","children":[{"id":"1:1","type":"CANVAS","children":[text]}]}}); + assert!(rest::is_rest_json(&input)); + let Ok((doc, warnings)) = rest::import(&input, &rest::no_images) else { + return (url, None, vec!["import failed".into()]); + }; + let link = match doc.nodes().find(|n| n.name == "Link").map(|n| &n.kind) { + Some(NodeKind::Text(data)) if override_style => data.runs.first().and_then(|r| r.style.link.clone()), + Some(NodeKind::Text(data)) => data.style.link.clone(), + _ => None, + }; + (url, link, warnings) +} + +/// Every length in turn: the destination comes back exactly, and nothing is reported. +fn check(override_style: bool) { + let mut mismatches = Vec::new(); + for length in [200, 201, 240] { + let (url, link, warnings) = imported_link(length, override_style); + assert!(warnings.is_empty(), "import warned at {length}: {warnings:?}"); + if link.as_deref() != Some(url.as_str()) { + mismatches.push((length, link.map_or(0, |l| l.len()))); + } + } + assert!(mismatches.is_empty(), "URLs changed, (input length, output length): {mismatches:?}"); +} + +#[test] +fn base_hyperlink_preserves_url() { + check(false); +} + +#[test] +fn override_hyperlink_preserves_url() { + check(true); +} diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 9704aab1..49fd272f 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -665,8 +665,19 @@ fn screenshot(b: &mut dyn Backend, a: &Map) -> Result