diff --git a/crates/ui-egui/examples/guide_edit_scenarios.rs b/crates/ui-egui/examples/guide_edit_scenarios.rs new file mode 100644 index 0000000000..d0d671a8ea --- /dev/null +++ b/crates/ui-egui/examples/guide_edit_scenarios.rs @@ -0,0 +1,139 @@ +//! JSONL adapter for interaction-check. Drives the shipped guide dialog and egui frame path. +//! See docs/guide-edit-interactions.md for the pinned runner and scenario command. +use std::io::{self, BufRead, Write}; + +use photocraft_ui_egui::{PhotocraftApp, Services, canvas, control, dialogs, guide_select, new_guide_ui}; +use serde_json::{Value, json}; + +struct Driver { + app: PhotocraftApp, + ctx: egui::Context, + time: f64, +} + +impl Driver { + /// Create a fresh synthetic document and two guides for each independent scenario. + fn new() -> Result { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Services::default()); + app.run("file.new", json!({"width":400,"height":200,"resolution":100}))?; + app.run("prefs.set", json!({"path":"unitsAndRulers.rulers","value":"pixels"}))?; + app.run("view.newGuide", json!({"orientation":"vertical","position":10}))?; + app.run("view.newGuide", json!({"orientation":"vertical","position":20}))?; + let ctx = egui::Context::default(); + PhotocraftApp::setup_context(&ctx, Default::default()); + let mut driver = Self { app, ctx, time: 0.0 }; + driver.frames(false); + Ok(driver) + } + + /// Run real egui canvas and dialog frames, optionally delivering Escape on the first. + fn frames(&mut self, escape: bool) { + self.app.sync_views(); + for frame in 0..3 { + self.time += 1.0 / 60.0; + let mut input = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1000.0, 700.0))), + time: Some(self.time), + ..Default::default() + }; + if escape && frame == 0 { + input.events.push(egui::Event::Key { + key: egui::Key::Escape, + physical_key: None, + pressed: true, + repeat: false, + modifiers: egui::Modifiers::NONE, + }); + } + let mut output = self.ctx.run_ui(input, |ui| { + let ctx = ui.ctx().clone(); + egui::CentralPanel::default().show(ui, |ui| canvas::document_area(&mut self.app, ui)); + dialogs::show(&mut self.app, &ctx); + }); + // Semantic checks do not upload textures; screenshot checks use the snapshot example. + output.textures_delta.clear(); + } + } + + /// Locate the current guide dialog, returning an error after it has closed. + fn dialog(&self) -> Result { + self.app.ui.dialogs.iter().find(|d| d.fields.contains_key("__edit")).map(|d| d.id).ok_or("no guide dialog".into()) + } + + /// Dispatch one scenario event through the shipped UI or engine paths. + fn event(&mut self, event: &Value) -> Result<(), String> { + match event.get("op").and_then(Value::as_str) { + Some("begin") => { + guide_select::select(&mut self.app, true, 0); + new_guide_ui::open_edit(&mut self.app)?; + } + Some("set") => { + let params = json!({"dialog":self.dialog()?,"field":event.get("field"),"value":event.get("value")}); + let (req, _rx) = control::ControlRequest::new("ui.dialog.set", params); + match control::handle(&mut self.app, &self.ctx, &req) { + control::Outcome::Done(v) if v.get("ok") == Some(&json!(true)) => {} + _ => return Err("ui.dialog.set failed".into()), + } + } + Some("confirm") => { + let id = self.dialog()?; + dialogs::confirm(&mut self.app, id)?; + } + Some("cancel") => { + let id = self.dialog()?; + dialogs::cancel(&mut self.app, id)?; + } + Some("escape") => self.frames(true), + Some("command") => { + let id = event.get("id").and_then(Value::as_str).ok_or("command needs id")?; + self.app.run(id, event.get("params").cloned().unwrap_or(json!({})))?; + } + Some("undo") if self.app.session.undo() => {} + Some("redo") if self.app.session.redo() => {} + Some("other_document") => { + self.app.run("file.new", json!({"width":400,"height":200}))?; + self.app.run("view.newGuide", json!({"orientation":"vertical","position":10}))?; + } + _ => return Err("unknown or unavailable event".into()), + } + Ok(()) + } + + /// Report observable document, preview, dialog and history state after an event. + fn state(&self, result: Result<(), String>) -> Value { + let active = self.app.session.active(); + let preview = active.and_then(|st| new_guide_ui::edit_preview(&self.app, &st.doc)); + let dialog = self.app.ui.dialogs.iter().find(|d| d.fields.contains_key("__edit")); + json!({ + "rejected": result.is_err(), "error": result.err(), + "vertical": active.map(|st| &st.doc.guides.vertical), + "horizontal": active.map(|st| &st.doc.guides.horizontal), + "colors": active.map(|st| &st.doc.guides.colors), + "preview": preview, + "history": active.map(|st| st.history.past_len()), + "redo": active.is_some_and(|st| st.history.can_redo()), + "editing": dialog.is_some(), + "invalid": dialog.is_some_and(|d| d.fields.contains_key("__error")), + "editCommands": self.app.session.journal.iter().filter(|(id, _)| id == "view.editGuide").count(), + }) + } +} + +/// Exchange one JSONL response per validated protocol request until stdin closes. +fn main() -> Result<(), Box> { + let mut driver = Driver::new()?; + let mut out = io::BufWriter::new(io::stdout().lock()); + for line in io::stdin().lock().lines() { + let request: Value = serde_json::from_str(&line?)?; + if request.get("protocol") != Some(&json!(1)) || request.get("id").and_then(Value::as_u64).is_none() { + return Err("expected interaction-check protocol 1 and an unsigned id".into()); + } + let event = request.get("event").ok_or("missing event")?; + let result = driver.event(event); + driver.frames(false); + let response = json!({"protocol":1,"id":request.get("id"),"state":driver.state(result)}); + writeln!(out, "{response}")?; + out.flush()?; + } + Ok(()) +} diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 41af7fe81c..57df5baffc 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -114,6 +114,7 @@ impl PhotocraftApp { proxy: None, proxy_uploaded: None, outline_cache: None, + guide_edit: None, filter_preview: None, smart_filter_targets: Default::default(), #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 9ebeeede52..7b6662970e 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1308,7 +1308,10 @@ fn paint_primary_overlays(app: &mut PhotocraftApp, ui: &mut egui::Ui, view: &mut if app.ui.extras.grid && app.ui.view.extras { crate::rulers::draw_grid(app, painter, xf, doc); } - if app.ui.view.shows(app.ui.view.show.canvas_guides) || crate::guide_layout_ui::preview(app, doc).is_some() { + if app.ui.view.shows(app.ui.view.show.canvas_guides) + || crate::guide_layout_ui::preview(app, doc).is_some() + || crate::new_guide_ui::edit_preview(app, doc).is_some() + { crate::rulers::draw_guides(app, painter, xf, doc); } // The selection's ants go over the grid and guides, as in Photoshop (#10671). diff --git a/crates/ui-egui/src/canvas/rulers.rs b/crates/ui-egui/src/canvas/rulers.rs index ce1ae599b5..e96e91ca30 100644 --- a/crates/ui-egui/src/canvas/rulers.rs +++ b/crates/ui-egui/src/canvas/rulers.rs @@ -216,7 +216,8 @@ pub fn draw_guides(app: &PhotocraftApp, painter: &egui::Painter, xf: &ViewXform, let drag = app.guide_drag; let g = &app.session.prefs().guides_grid_and_slices; let preview = crate::guide_layout_ui::preview(app, doc); - let guides = preview.as_ref().map(|(guides, _)| guides).unwrap_or(&doc.guides); + let edit = crate::new_guide_ui::edit_preview(app, doc); + let guides = edit.or_else(|| preview.as_ref().map(|(guides, _)| guides)).unwrap_or(&doc.guides); let (guide, style) = (preview.as_ref().map(|(_, color)| *color).unwrap_or_else(|| pref_color(&g.guide_color, Color32::from_rgb(74, 255, 255))), g.guide_style); let line = |vertical: bool, pos: f64, color: Color32| { @@ -226,12 +227,12 @@ pub fn draw_guides(app: &PhotocraftApp, painter: &egui::Painter, xf: &ViewXform, // A guide given a colour in New Guide keeps it; the rest use the preference colour (or, while // New Guide Layout previews, the colour it will set). The selected guide is Photoshop's blue. let own = |vertical: bool, p: f64| { - if crate::guide_select::is_selected(app, vertical, p) { + if edit.is_none() && crate::guide_select::is_selected(app, vertical, p) { return crate::guide_select::SELECTED_COLOR; } guides.color(vertical, p).map_or(guide, |[r, g, b]| Color32::from_rgb(r, g, b)) }; - if app.ui.extras.guides || preview.is_some() { + if app.ui.extras.guides || preview.is_some() || edit.is_some() { for (vertical, list) in [(true, &guides.vertical), (false, &guides.horizontal)] { for (i, p) in list.iter().enumerate() { if drag.is_some_and(|d| d.vertical == vertical && d.index == Some(i)) { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fc0c268097..bc9733704d 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -554,6 +554,9 @@ fn default_max_width(d: &Dialog, compact: bool, curves: bool, about_tabs: bool, } pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { + if crate::guide_edit::sync(app) { + ctx.request_repaint(); + } crate::layer_style::color_picker::prune(app); crate::type_panels_ui::color_picker::prune(app); crate::edit_history::track_dialogs(app, ctx); @@ -768,6 +771,9 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { } } } + if crate::guide_edit::sync(app) { + ctx.request_repaint(); + } ctx.data_mut(|m| m.insert_temp(egui::Id::unique(RECTS), shown)); // A colour picked this frame shows in its target dialog on the next one. if crate::color_picker_ui::live_preview::sync(app) { @@ -1078,6 +1084,7 @@ pub fn confirm(app: &mut PhotocraftApp, id: u64) -> Result { /// Cancel a dialog and its dependent Layer Style picker, without applying edits. pub fn cancel(app: &mut PhotocraftApp, id: u64) -> Result { let closed = app.ui.close_dialog(id).ok_or_else(|| format!("no dialog {id}"))?; + crate::guide_edit::cancel(app, id); // A Color Picker puts back what its live preview showed in its target. crate::color_picker_ui::live_preview::end(app, id); // A field Color Picker of this dialog has no target left: it closes with the dialog. @@ -1188,6 +1195,7 @@ pub(super) mod filter_preview_ui; #[cfg(not(target_arch = "wasm32"))] pub(super) mod filter_preview_worker; pub mod gallery_ui; +pub(crate) mod guide_edit; pub mod guide_layout_ui; pub mod history_options_ui; pub mod jpeg_options_ui; diff --git a/crates/ui-egui/src/dialogs/guide_edit.rs b/crates/ui-egui/src/dialogs/guide_edit.rs new file mode 100644 index 0000000000..70a2606999 --- /dev/null +++ b/crates/ui-egui/src/dialogs/guide_edit.rs @@ -0,0 +1,206 @@ +//! A guide dialog's draft. Rendering and command validation stay in PhotoCraft; the +//! [`edit_session`](crate::edit_session) module owns proposal validity, stale-context rejection +//! and the single-use/no-op commit decision. +use crate::edit_session::{EditSession, FinishError}; +use photocraft_doc::{DocId, Document, Guides}; +use photocraft_engine::prefs::{PointSize, Unit}; +use serde_json::{Map, Value}; + +use crate::PhotocraftApp; + +#[derive(Clone, Debug, PartialEq)] +struct Guide { + vertical: bool, + position: f64, + color: Option<[u8; 3]>, +} + +#[derive(Debug, PartialEq)] +struct Context { + doc: DocId, + revision: u64, + rulers: Unit, + points: PointSize, + locked: bool, +} + +pub(crate) struct Draft { + pub(crate) dialog: u64, + target: Value, + initial_position: String, + edit: EditSession, + params: Option, + guides: Option, + error: Option, +} + +/// Identity, revision, units and lock state that must still match at confirmation. +fn context(app: &PhotocraftApp) -> Option { + let st = app.session.active()?; + let units = &app.session.prefs().units_and_rulers; + Some(Context { doc: st.doc.id, revision: st.revision, rulers: units.rulers, points: units.point_size, locked: app.ui.extras.lock_guides }) +} + +/// Read a guide and its explicit colour without trusting a saved index. +fn guide(g: &Guides, vertical: bool, index: usize) -> Option { + let position = *if vertical { &g.vertical } else { &g.horizontal }.get(index)?; + Some(Guide { vertical, position, color: g.color(vertical, position) }) +} + +/// Decode a target only when its orientation and index are representable. +fn target(f: &Value) -> Option<(bool, usize)> { + let vertical = match f.get("orientation")?.as_str()? { + "vertical" => true, + "horizontal" => false, + _ => return None, + }; + Some((vertical, usize::try_from(f.get("index")?.as_u64()?).ok()?)) +} + +/// Capture the original target and context, retiring any previous guide dialog. +pub(crate) fn begin(app: &mut PhotocraftApp, dialog: u64, fields: &Map) -> Result<(), String> { + let target_value = fields.get("__edit").ok_or("missing guide target")?; + let (vertical, index) = target(target_value).ok_or("invalid guide target")?; + let st = app.session.active().ok_or("no document")?; + let value = guide(&st.doc.guides, vertical, index).ok_or("no such guide")?; + let context = context(app).ok_or("no document")?; + if let Some(previous) = app.guide_edit.take() { + app.ui.close_dialog(previous.dialog); + previous.edit.cancel(); + } + app.guide_edit = Some(Draft { + dialog, + target: target_value.clone(), + initial_position: fields.get("position").and_then(Value::as_str).unwrap_or_default().into(), + edit: EditSession::new(value, context), + params: None, + guides: None, + error: None, + }); + Ok(()) +} + +/// Read the live target for comparison with the draft's original context. +fn current(app: &PhotocraftApp, draft: &Draft) -> Option<(Guide, Context)> { + let (vertical, index) = target(&draft.target)?; + Some((guide(&app.session.active()?.doc.guides, vertical, index)?, context(app)?)) +} + +/// Invalid text remains editable; a changed target or context requires a new draft. +fn stale(app: &PhotocraftApp, draft: &Draft) -> bool { + current(app, draft).is_none_or(|(live, ctx)| !matches!(draft.edit.check(&live, &ctx), Ok(()) | Err(FinishError::InvalidProposal))) +} + +/// Allow retry only while the retained draft still belongs to the same live guide. +pub(crate) fn retryable(app: &PhotocraftApp) -> bool { + app.guide_edit.as_ref().is_some_and(|d| !stale(app, d)) +} + +/// Validate changed parameters on a COW document, retaining the last valid preview on error. +fn prepare(app: &PhotocraftApp, draft: &mut Draft, fields: &Map) -> Result<(), String> { + if stale(app, draft) || fields.get("__edit") != Some(&draft.target) { + return Err("The document or guide changed. Reopen Edit Selected Guides.".into()); + } + let mut params = crate::new_guide_ui::params(app, fields); + // Opening a fractional position in cm/percent rounds its display. Leaving the text and + // orientation alone must not move the guide or consume Redo through that rounding. + if fields.get("position").and_then(Value::as_str) == Some(&draft.initial_position) + && (params.get("newOrientation").and_then(Value::as_str) == Some("vertical")) == draft.edit.original().vertical + { + params["position"] = serde_json::json!(draft.edit.original().position); + } + if draft.params.as_ref() == Some(¶ms) { + return draft.error.clone().map_or(Ok(()), Err); + } + let st = app.session.active().ok_or("no document")?; + // A cheap COW document on parameter changes only. The existing engine owns units, + // validation, colour inheritance and turning a guide into the other orientation. + let mut preview = photocraft_engine::Session::new(); + preview.prefs = app.session.prefs.clone(); + preview.add_document((*st.doc).clone(), None); + let result = preview.execute("view.editGuide", params.clone()).map_err(|e| e.to_string()).and_then(|_| { + let g = &preview.active().ok_or("no preview document")?.doc.guides; + let (from, old_index) = target(&draft.target).ok_or("invalid guide target")?; + let to = params.get("newOrientation").and_then(Value::as_str) == Some("vertical"); + let index = if from == to { old_index } else { if to { g.vertical.len() } else { g.horizontal.len() }.checked_sub(1).ok_or("no preview guide")? }; + let value = guide(g, to, index).ok_or("no preview guide")?; + draft.guides = Some(g.clone()); + Ok(value) + }); + draft.params = Some(params); + draft.edit.propose(result) +} + +/// Keep the draft and its validation message available after either valid or invalid input. +pub(crate) fn update(app: &mut PhotocraftApp, fields: &Map) -> Result<(), String> { + let mut draft = app.guide_edit.take().ok_or("Reopen Edit Selected Guides to start an edit.")?; + let result = prepare(app, &mut draft, fields); + draft.error = result.as_ref().err().cloned(); + app.guide_edit = Some(draft); + result +} + +/// Commit once through the ordinary command path; a no-op preserves Redo. +/// A command refusal restores the proposal against its original context for a guarded retry. +pub(crate) fn finish(app: &mut PhotocraftApp, fields: &Map) -> Result, String> { + update(app, fields)?; + let mut draft = app.guide_edit.take().ok_or("no guide draft")?; + let (live, ctx) = current(app, &draft).ok_or("the guide is gone")?; + let change = draft.edit.finish(&live, &ctx).map_err(|_| "The document or guide changed. Reopen Edit Selected Guides.")?; + let Some(change) = change else { return Ok(None) }; + let params = draft.params.clone().ok_or("no guide parameters")?; + match app.run("view.editGuide", params.clone()) { + Ok(result) => Ok(Some((params, result))), + Err(error) => { + // Reuse the pre-command context, never a fresh document/revision. The caller's + // retryable check rejects this draft if the failed command changed that context. + draft.edit = EditSession::new(live, ctx); + draft.edit.propose::(Ok(change.after))?; + app.guide_edit = Some(draft); + Err(error) + } + } +} + +/// Discard only the draft owned by the closing dialog, without touching history. +pub(crate) fn cancel(app: &mut PhotocraftApp, id: u64) { + if app.guide_edit.as_ref().is_some_and(|d| d.dialog == id) + && let Some(draft) = app.guide_edit.take() + { + draft.edit.cancel(); + } +} + +/// Refresh after dialog fields change, and drop drafts whose dialog has gone away. +pub(crate) fn sync(app: &mut PhotocraftApp) -> bool { + let Some(id) = app.guide_edit.as_ref().map(|d| d.dialog) else { return false }; + let Some(fields) = app.ui.dialogs.iter().find(|d| d.id == id).map(|d| d.fields.clone()) else { + cancel(app, id); + return false; + }; + let old = app.guide_edit.as_ref().and_then(|d| d.params.clone()); + let result = update(app, &fields); + let changed = old != app.guide_edit.as_ref().and_then(|d| d.params.clone()); + if let Some(d) = app.ui.dialog_mut(id) { + match result { + Ok(()) if changed => { + d.fields.remove("__error"); + } + // Unchanged, valid parameters may still have a command-policy refusal to show. + Ok(()) => {} + Err(error) => { + d.fields.insert("__error".into(), Value::String(error)); + } + } + } + changed +} + +/// Expose the last valid preview only for its unchanged document and still-open dialog. +pub(crate) fn preview<'a>(app: &'a PhotocraftApp, doc: &Document) -> Option<&'a Guides> { + let draft = app.guide_edit.as_ref()?; + if draft.edit.context().doc != doc.id || stale(app, draft) || !app.ui.dialogs.iter().any(|d| d.id == draft.dialog) { + return None; + } + draft.guides.as_ref() +} diff --git a/crates/ui-egui/src/dialogs/new_guide_ui.rs b/crates/ui-egui/src/dialogs/new_guide_ui.rs index 8657576b57..25b058ea87 100644 --- a/crates/ui-egui/src/dialogs/new_guide_ui.rs +++ b/crates/ui-egui/src/dialogs/new_guide_ui.rs @@ -53,6 +53,7 @@ pub(crate) fn color_label(name: &str) -> &'static str { } } +/// Accept the named guide colours and the explicit custom-colour choice. fn is_color_name(name: &str) -> bool { name == CUSTOM || GUIDE_COLORS.iter().any(|(n, ..)| *n == name) } @@ -65,10 +66,12 @@ fn custom_color(f: &Map) -> String { .map_or_else(|| "#4affff".to_string(), str::to_string) } +/// Identify the specialized New Guide or Edit Selected Guides dialog fields. pub fn owns(fields: &Map) -> bool { fields.contains_key(MARK) } +/// Read the ruler unit used for guide positions without an explicit suffix. fn ruler_unit(app: &PhotocraftApp) -> Unit { app.session.prefs().units_and_rulers.rulers } @@ -145,9 +148,15 @@ pub fn open_edit(app: &mut PhotocraftApp) -> Result { f.insert(INITIAL_COLOR.into(), json!(if color == CUSTOM { custom.clone() } else { color.clone() })); f.insert("color".into(), json!(color)); f.insert("customColor".into(), json!(custom)); - Ok(app.ui.open_dialog(DialogKind::Command, f)) + let id = app.ui.open_dialog(DialogKind::Command, f.clone()); + if let Err(error) = crate::guide_edit::begin(app, id, &f) { + app.ui.close_dialog(id); + return Err(error); + } + Ok(id) } +/// Edit the dialog fields and show validation errors without changing the live guide. pub fn body(ui: &mut egui::Ui, f: &mut Map) { let mut orientation = f.get("orientation").and_then(Value::as_str).unwrap_or("horizontal").to_string(); let mut position = f.get("position").and_then(Value::as_str).unwrap_or_default().to_string(); @@ -212,10 +221,11 @@ pub fn params(app: &PhotocraftApp, f: &Map) -> Value { } } +/// Dispatch New Guide or guarded guide editing, retaining typed fields on a retryable error. pub fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result { let params = params(app, f); if f.contains_key(EDIT) { - return confirm_edit(app, f, ¶ms); + return confirm_edit(app, f); } match app.run(COMMAND, params.clone()) { Ok(result) => { @@ -244,27 +254,39 @@ pub fn confirm(app: &mut PhotocraftApp, f: &Map) -> Result, params: &Value) -> Result { - match app.run(EDIT_COMMAND, params.clone()) { - Ok(result) => { - let (from, to) = (params["orientation"] == "vertical", params["newOrientation"] == "vertical"); - let index = if from == to { - params["index"].as_u64().and_then(|i| usize::try_from(i).ok()) +fn confirm_edit(app: &mut PhotocraftApp, f: &Map) -> Result { + let (params, result) = match crate::guide_edit::finish(app, f) { + Ok(Some(applied)) => applied, + Ok(None) => return Ok(Value::Null), + Err(error) => { + if crate::guide_edit::retryable(app) { + let mut again = f.clone(); + again.insert("__error".into(), json!(error)); + let id = app.ui.open_dialog(DialogKind::Command, again); + if let Some(draft) = app.guide_edit.as_mut() { + draft.dialog = id; + } } else { - app.session.active().map(|st| if to { st.doc.guides.vertical.len() } else { st.doc.guides.horizontal.len() }.saturating_sub(1)) - }; - if let Some(i) = index { - crate::guide_select::select(app, to, i); + app.guide_edit = None; } - Ok(result) - } - Err(e) => { - let mut again = f.clone(); - again.insert("__error".into(), json!(e)); - app.ui.open_dialog(DialogKind::Command, again); - Err(e) + return Err(error); } + }; + let (from, to) = (params["orientation"] == "vertical", params["newOrientation"] == "vertical"); + let index = if from == to { + params["index"].as_u64().and_then(|i| usize::try_from(i).ok()) + } else { + app.session.active().map(|st| if to { st.doc.guides.vertical.len() } else { st.doc.guides.horizontal.len() }.saturating_sub(1)) + }; + if let Some(i) = index { + crate::guide_select::select(app, to, i); } + Ok(result) +} + +/// The validated draft shown on the canvas, without changing the document or its history. +pub fn edit_preview<'a>(app: &'a PhotocraftApp, doc: &photocraft_doc::Document) -> Option<&'a photocraft_doc::Guides> { + crate::guide_edit::preview(app, doc) } #[cfg(test)] diff --git a/crates/ui-egui/src/dialogs/new_guide_ui/tests.rs b/crates/ui-egui/src/dialogs/new_guide_ui/tests.rs index 4bf4fa80d6..c08b6d4948 100644 --- a/crates/ui-egui/src/dialogs/new_guide_ui/tests.rs +++ b/crates/ui-egui/src/dialogs/new_guide_ui/tests.rs @@ -12,21 +12,25 @@ fn app(unit: &str) -> (PhotocraftApp, egui::Context) { (app, egui::Context::default()) } +/// Open New Guide through the same menu entry used by the shell. fn open(app: &mut PhotocraftApp, ctx: &egui::Context) -> u64 { menu(app, ctx, "view.newGuide", json!({})).unwrap()["dialog"].as_u64().unwrap() } +/// Change the typed orientation and position without confirming the dialog. fn set(app: &mut PhotocraftApp, d: u64, orientation: &str, position: &str) { let f = &mut app.ui.dialog_mut(d).unwrap().fields; f.insert("orientation".into(), json!(orientation)); f.insert("position".into(), json!(position)); } +/// Read committed guide positions, excluding any local draft preview. fn guides(app: &PhotocraftApp) -> (Vec, Vec) { let g = &app.session.active().unwrap().doc.guides; (g.horizontal.clone(), g.vertical.clone()) } +/// Guide positions accept explicit units and reopen in the current ruler unit. #[test] fn opens_in_the_ruler_unit_and_takes_photoshop_units() { let (mut app, ctx) = app("cm"); @@ -53,6 +57,7 @@ fn opens_in_the_ruler_unit_and_takes_photoshop_units() { assert_eq!(guides(&app).0, vec![100.0, 50.0]); } +/// Bare positions use percent when the ruler unit is percent. #[test] fn percent_rulers_open_at_zero_percent() { let (mut app, ctx) = app("percent"); @@ -63,6 +68,7 @@ fn percent_rulers_open_at_zero_percent() { assert_eq!(guides(&app).0, vec![50.0], "25% of the 200 px height"); } +/// Invalid New Guide input is retained without recording a guide or preferences. #[test] fn a_refused_position_reopens_the_dialog_without_a_guide() { let (mut app, ctx) = app("pixels"); @@ -79,6 +85,7 @@ fn a_refused_position_reopens_the_dialog_without_a_guide() { assert!(!app.session.prefs().dialogs.contains_key("view.newGuide")); } +/// Invalid saved orientation and position fall back to a usable new-guide dialog. #[test] fn remembered_values_that_no_longer_parse_are_ignored() { let (mut app, ctx) = app("mm"); @@ -88,10 +95,12 @@ fn remembered_values_that_no_longer_parse_are_ignored() { assert_eq!((f["orientation"].as_str(), f["position"].as_str()), (Some("horizontal"), Some("0 mm"))); } +/// Read the colour stored on a committed guide. fn color_of(app: &PhotocraftApp, vertical: bool, position: f64) -> Option<[u8; 3]> { app.session.active().unwrap().doc.guides.color(vertical, position) } +/// Named and custom guide colours persist on the guide and in dialog preferences. #[test] fn guides_store_the_chosen_colour_cyan_by_default() { let (mut app, ctx) = app("pixels"); @@ -120,6 +129,7 @@ fn guides_store_the_chosen_colour_cyan_by_default() { assert_eq!((f["color"].as_str(), f["customColor"].as_str()), (Some("custom"), Some("#abcdef"))); } +/// Invalid saved colours fall back to the default cyan choice. #[test] fn remembered_colours_that_are_not_on_the_list_are_ignored() { let (mut app, ctx) = app("pixels"); @@ -129,6 +139,7 @@ fn remembered_colours_that_are_not_on_the_list_are_ignored() { assert_eq!((f["color"].as_str(), f["customColor"].as_str()), (Some("cyan"), Some("#4affff"))); } +/// A remembered pixel position is displayed in each current ruler unit. #[test] fn the_last_position_reopens_in_whatever_the_ruler_unit_is_now() { let (mut app, ctx) = app("pixels"); @@ -142,3 +153,187 @@ fn the_last_position_reopens_in_whatever_the_ruler_unit_is_now() { crate::dialogs::cancel(&mut app, d).unwrap(); } } + +// These regressions exercise the ordinary dialog handlers, not a separate model. +/// Open a draft on the first of two guides and propose moving it to 30 px. +fn edit_fixture() -> (PhotocraftApp, u64) { + let (mut app, _) = app("pixels"); + app.run("view.newGuide", json!({"orientation": "vertical", "position": 10})).unwrap(); + app.run("view.newGuide", json!({"orientation": "vertical", "position": 20})).unwrap(); + crate::guide_select::select(&mut app, true, 0); + let id = open_edit(&mut app).unwrap(); + set(&mut app, id, "vertical", "30 px"); + (app, id) +} + +/// A document switch rejects the old draft without touching the new document. +#[test] +fn draft_cannot_edit_a_guide_in_another_document() { + let (mut app, id) = edit_fixture(); + app.run("file.new", json!({"width": 400, "height": 200})).unwrap(); + app.run("view.newGuide", json!({"orientation": "vertical", "position": 10})).unwrap(); + let history = app.session.active().unwrap().history.past_len(); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!(guides(&app).1, vec![10.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), history); +} + +/// Deleting the target invalidates its draft even when another guide reuses the index. +#[test] +fn draft_cannot_edit_the_guide_that_reused_a_deleted_index() { + let (mut app, id) = edit_fixture(); + app.run("view.deleteGuide", json!({"orientation": "vertical", "index": 0})).unwrap(); + let history = app.session.active().unwrap().history.past_len(); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!(guides(&app).1, vec![20.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), history); +} + +/// Confirming unchanged values records no command and retains the redo stack. +#[test] +fn draft_unchanged_confirmation_preserves_redo() { + let (mut app, id) = edit_fixture(); + crate::dialogs::cancel(&mut app, id).unwrap(); + app.run("view.moveGuide", json!({"orientation": "vertical", "index": 0, "position": 15})).unwrap(); + assert!(app.session.undo()); + crate::guide_select::select(&mut app, true, 0); + let id = open_edit(&mut app).unwrap(); + let history = app.session.active().unwrap().history.past_len(); + crate::dialogs::confirm(&mut app, id).unwrap(); + assert_eq!(app.session.active().unwrap().history.past_len(), history); + assert!(app.session.active().unwrap().history.can_redo()); +} + +/// Preview changes stay local; invalid input can be corrected into exactly one commit. +#[test] +fn draft_preview_is_isolated_and_invalid_text_keeps_the_last_good_preview() { + let (mut app, id) = edit_fixture(); + crate::guide_edit::sync(&mut app); + let shown = |app: &PhotocraftApp| edit_preview(app, &app.session.active().unwrap().doc).unwrap().vertical.clone(); + assert_eq!(shown(&app), vec![30.0, 20.0]); + assert_eq!(guides(&app).1, vec![10.0, 20.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), 2); + set(&mut app, id, "vertical", "far"); + crate::guide_edit::sync(&mut app); + assert_eq!(shown(&app), vec![30.0, 20.0]); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + let id = app.ui.dialogs.last().unwrap().id; + assert!(app.ui.dialog_mut(id).unwrap().fields.contains_key("__error")); + set(&mut app, id, "vertical", "40 px"); + crate::dialogs::confirm(&mut app, id).unwrap(); + assert_eq!(guides(&app).1, vec![40.0, 20.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), 3); + assert_eq!(app.session.journal.iter().filter(|(cmd, _)| cmd == "view.editGuide").count(), 1); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); +} + +/// Opening another edit invalidates the earlier dialog; cancellation removes the draft. +#[test] +fn draft_reopening_retires_the_previous_dialog() { + let (mut app, first) = edit_fixture(); + let second = open_edit(&mut app).unwrap(); + assert!(crate::dialogs::confirm(&mut app, first).is_err()); + crate::dialogs::cancel(&mut app, second).unwrap(); + assert!(app.guide_edit.is_none()); + assert_eq!(guides(&app).1, vec![10.0, 20.0]); +} + +/// Locking guides invalidates an open edit before it can record a command. +#[test] +fn draft_cannot_commit_after_guides_are_locked() { + let (mut app, id) = edit_fixture(); + app.ui.extras.lock_guides = true; + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!(guides(&app).1, vec![10.0, 20.0]); + assert!(app.guide_edit.is_none()); +} + +/// A late policy refusal retains the typed proposal and history until a successful retry. +#[test] +fn refused_edit_command_preserves_the_draft_for_retry() { + let (mut app, id) = edit_fixture(); + crate::dialogs::cancel(&mut app, id).unwrap(); + app.run("view.moveGuide", json!({"orientation": "vertical", "index": 0, "position": 15})).unwrap(); + assert!(app.session.undo()); + crate::guide_select::select(&mut app, true, 0); + let id = open_edit(&mut app).unwrap(); + set(&mut app, id, "vertical", "30 px"); + let history = app.session.active().unwrap().history.past_len(); + let revision = app.session.active().unwrap().revision; + app.automation_input = true; + app.services.automation_command = Some(Box::new(|id, _| if id == "view.editGuide" { Err("guide edits are denied".into()) } else { Ok(()) })); + let mut id = id; + for _ in 0..2 { + assert_eq!(crate::dialogs::confirm(&mut app, id).unwrap_err(), "guide edits are denied"); + assert_eq!(guides(&app).1, vec![10.0, 20.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), history); + assert_eq!(app.session.active().unwrap().revision, revision); + assert!(app.session.active().unwrap().history.can_redo()); + assert_eq!(app.session.journal.iter().filter(|(cmd, _)| cmd == "view.editGuide").count(), 0); + crate::guide_edit::sync(&mut app); + let again = app.ui.dialogs.iter().find(|d| d.fields.contains_key(EDIT)).expect("retry dialog remains open"); + id = again.id; + assert_eq!(again.fields["position"], "30 px"); + assert_eq!(again.fields["orientation"], "vertical"); + assert_eq!(again.fields["__error"], "guide edits are denied"); + assert_eq!(edit_preview(&app, &app.session.active().unwrap().doc).unwrap().vertical, vec![30.0, 20.0]); + } + app.services.automation_command = None; + crate::dialogs::confirm(&mut app, id).unwrap(); + assert_eq!(guides(&app).1, vec![30.0, 20.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), history + 1); + assert_eq!(app.session.journal.iter().filter(|(cmd, _)| cmd == "view.editGuide").count(), 1); + assert!(app.guide_edit.is_none()); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert!(app.session.undo()); + assert_eq!(guides(&app).1, vec![10.0, 20.0]); + assert!(app.session.redo()); + assert_eq!(guides(&app).1, vec![30.0, 20.0]); +} + +/// Restoring a refused proposal must not bind it to a different document at retry time. +#[test] +fn refused_edit_draft_cannot_follow_a_document_switch() { + let (mut app, id) = edit_fixture(); + app.automation_input = true; + app.services.automation_command = Some(Box::new(|_, _| Err("denied".into()))); + assert_eq!(crate::dialogs::confirm(&mut app, id).unwrap_err(), "denied"); + let id = app.ui.dialogs.last().unwrap().id; + app.services.automation_command = None; + app.run("file.new", json!({"width": 400, "height": 200})).unwrap(); + app.run("view.newGuide", json!({"orientation": "vertical", "position": 10})).unwrap(); + let history = app.session.active().unwrap().history.past_len(); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!(guides(&app).1, vec![10.0]); + assert_eq!(app.session.active().unwrap().history.past_len(), history); + assert!(app.guide_edit.is_none()); + assert!(!app.ui.dialogs.iter().any(|d| d.fields.contains_key(EDIT))); +} + +/// Normal UI frames retain the command refusal; editing the position clears it. +#[test] +fn refused_edit_shows_the_error_and_typed_fields_in_the_dialog() { + use egui_kittest::kittest::Queryable; + let (mut app, id) = edit_fixture(); + app.automation_input = true; + app.services.automation_command = Some(Box::new(|_, _| Err("guide edits are denied".into()))); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + let evidence = std::env::var_os("PHOTOCRAFT_GUIDE_RETRY_EVIDENCE"); + let builder = egui_kittest::Harness::builder().with_size(egui::vec2(1000.0, 700.0)); + let builder = if evidence.is_some() { builder.wgpu() } else { builder }; + let mut h = builder.build_eframe(move |cc| { + PhotocraftApp::setup_context(&cc.egui_ctx, Default::default()); + app + }); + h.run_steps(3); + h.get_by_label("guide edits are denied"); + assert_eq!(h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Position").value().as_deref(), Some("30 px")); + if let Some(path) = evidence { + h.render().unwrap().save(std::path::Path::new(&path)).unwrap(); + } + let id = h.state().ui.dialogs.last().unwrap().id; + set(h.state_mut(), id, "vertical", "40 px"); + h.run_steps(3); + assert!(h.query_by_label("guide edits are denied").is_none()); + assert_eq!(guides(h.state()).1, vec![10.0, 20.0]); +} diff --git a/crates/ui-egui/src/edit_session.rs b/crates/ui-egui/src/edit_session.rs new file mode 100644 index 0000000000..887a6161b4 --- /dev/null +++ b/crates/ui-egui/src/edit_session.rs @@ -0,0 +1,111 @@ +//! A local draft of a value, independent of rendering and the application's undo engine. +//! +//! The host validates proposals and owns all document mutations. Dropping a session cancels +//! only its local draft. Store small parameters here, not a full document per pointer event. +//! A context must include stable document/target identity and a revision that changes on +//! every relevant edit (including undo/redo). Comparing values alone cannot detect ABA edits. + +/// Why a draft cannot be committed to the current host state. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum FinishError { + /// The most recent proposal was rejected. Correct it before finishing. + InvalidProposal, + /// Document, target, selection or relevant revision has changed. + ContextChanged, + /// The live value changed without the context being updated. + ValueChanged, +} + +/// A proposed change. The caller must apply it through its existing command engine. +#[derive(Debug, PartialEq, Eq)] +pub struct Change { + pub before: T, + pub after: T, +} + +/// A single-use edit. It never writes to a document or records history itself. +/// +/// This type intentionally does not implement `Clone`. `finish` consumes it, so the same +/// session cannot accidentally emit two commits. A failed finish also consumes the draft; +/// call `check` first if invalid input should remain editable in the UI. +#[must_use = "a draft must be retained, finished, or deliberately cancelled"] +#[derive(Debug)] +pub struct EditSession { + original: T, + preview: T, + context: C, + valid: bool, +} + +impl EditSession { + /// Begin from a value already validated by the application. + pub fn new(original: T, context: C) -> Self { + Self { preview: original.clone(), original, context, valid: true } + } +} + +impl EditSession { + pub fn original(&self) -> &T { + &self.original + } + + /// Last valid preview, even when a later incomplete/invalid input is present. + #[cfg(test)] + pub fn preview(&self) -> &T { + &self.preview + } + + pub fn context(&self) -> &C { + &self.context + } + + #[cfg(test)] + pub fn is_valid(&self) -> bool { + self.valid + } + + /// Accept a host-validated proposal. Errors invalidate commit without replacing the + /// last valid preview. Units, mixed values, finite checks and bounds belong to the host. + pub fn propose(&mut self, proposal: Result) -> Result<(), E> { + match proposal { + Ok(value) => { + self.preview = value; + self.valid = true; + Ok(()) + } + Err(error) => { + self.valid = false; + Err(error) + } + } + } + + /// Discard the local draft. No rollback commands or history entries are generated. + pub fn cancel(self) {} +} + +impl EditSession { + /// Check immediately before applying on the host's serialized command path. + /// This comparison is not a lock and is not a distributed transaction. + pub fn check(&self, live: &T, context: &C) -> Result<(), FinishError> { + if *context != self.context { + Err(FinishError::ContextChanged) + } else if *live != self.original { + Err(FinishError::ValueChanged) + } else if !self.valid { + Err(FinishError::InvalidProposal) + } else { + Ok(()) + } + } + + /// Emit at most one change; a no-op emits none, preserving the host's redo stack. + /// The host still validates and applies the command and handles execution failure. + pub fn finish(self, live: &T, context: &C) -> Result>, FinishError> { + self.check(live, context)?; + if self.original == self.preview { Ok(None) } else { Ok(Some(Change { before: self.original, after: self.preview })) } + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/ui-egui/src/edit_session/tests.rs b/crates/ui-egui/src/edit_session/tests.rs new file mode 100644 index 0000000000..3ed6a35fb9 --- /dev/null +++ b/crates/ui-egui/src/edit_session/tests.rs @@ -0,0 +1,75 @@ +use super::{Change, EditSession, FinishError}; + +#[test] +fn many_previews_produce_one_final_change() { + let mut draft = EditSession::new(100, ("photo", "layer-1", 9)); + for value in 0..1000 { + assert_eq!(draft.propose::<()>(Ok(value)), Ok(())); + assert_eq!(*draft.original(), 100); + } + assert_eq!(draft.finish(&100, &("photo", "layer-1", 9)), Ok(Some(Change { before: 100, after: 999 }))); +} + +#[test] +fn invalid_input_retains_preview_but_blocks_commit_until_corrected() { + let mut draft = EditSession::new(10, 0); + assert_eq!(draft.propose::<&str>(Ok(12)), Ok(())); + assert_eq!(draft.propose(Err("incomplete number")), Err("incomplete number")); + assert_eq!(*draft.preview(), 12); + assert!(!draft.is_valid()); + assert_eq!(draft.check(&10, &0), Err(FinishError::InvalidProposal)); + assert_eq!(draft.propose::<&str>(Ok(14)), Ok(())); + assert_eq!(draft.finish(&10, &0), Ok(Some(Change { before: 10, after: 14 }))); +} + +#[test] +fn unchanged_and_return_to_original_have_no_commit() { + assert_eq!(EditSession::new(20, 0).finish(&20, &0), Ok(None)); + let mut draft = EditSession::new(20, 0); + assert_eq!(draft.propose::<()>(Ok(50)), Ok(())); + assert_eq!(draft.propose::<()>(Ok(20)), Ok(())); + assert_eq!(draft.finish(&20, &0), Ok(None)); +} + +#[test] +fn stale_document_target_or_revision_cannot_commit() { + for context in [(2, 5, 8), (1, 6, 8), (1, 5, 9)] { + let draft = EditSession::new(100, (1, 5, 8)); + assert_eq!(draft.finish(&100, &context), Err(FinishError::ContextChanged)); + } +} + +#[test] +fn unversioned_value_change_is_also_rejected() { + assert_eq!(EditSession::new(100, 1).finish(&50, &1), Err(FinishError::ValueChanged)); +} + +#[test] +fn undo_back_to_equal_value_is_stale_when_revision_has_advanced() { + assert_eq!(EditSession::new(100, 1).finish(&100, &3), Err(FinishError::ContextChanged)); +} + +#[test] +fn invalid_final_input_does_not_silently_commit_last_valid_value() { + let mut draft = EditSession::new(10, 0); + assert_eq!(draft.propose::<&str>(Ok(12)), Ok(())); + assert!(draft.propose(Err("not finite")).is_err()); + assert_eq!(draft.finish(&10, &0), Err(FinishError::InvalidProposal)); +} + +#[test] +fn optional_values_preserve_inheritance_instead_of_flattening_it() { + let mut draft = EditSession::new(None::, 0); + assert_eq!(draft.propose::<()>(Ok(Some(20))), Ok(())); + assert_eq!(draft.propose::<()>(Ok(None)), Ok(())); + assert_eq!(draft.finish(&None, &0), Ok(None)); +} + +#[test] +fn cancel_does_not_touch_host_value() { + let live = String::from("original"); + let mut draft = EditSession::new(live.clone(), 1); + assert_eq!(draft.propose::<()>(Ok("preview".into())), Ok(())); + draft.cancel(); + assert_eq!(live, "original"); +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8563f65faf..569f40841b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -29,6 +29,7 @@ pub mod canvas; pub mod control; pub mod dialogs; pub mod dock; +pub(crate) mod edit_session; pub mod file_dialog; pub mod file_open; pub mod frame_stages; @@ -152,6 +153,7 @@ pub use dialogs::fill_ui; pub use dialogs::filter_dialog; use dialogs::filter_preview_ui; pub use dialogs::gallery_ui; +use dialogs::guide_edit; pub use dialogs::guide_layout_ui; pub use dialogs::history_options_ui; pub use dialogs::jpeg_options_ui; @@ -508,6 +510,7 @@ pub struct PhotocraftApp { pub(crate) proxy_uploaded: Option<(DocId, u64)>, /// Selection outline cache: (doc, revision, segments). /// Live filter preview (proxy document with the filter applied). + pub(crate) guide_edit: Option, pub(crate) filter_preview: Option, pub(crate) smart_filter_targets: smart_filter_dialog::Targets, #[cfg(not(target_arch = "wasm32"))] diff --git a/docs/guide-edit-interactions.md b/docs/guide-edit-interactions.md new file mode 100644 index 0000000000..0bc9884cfe --- /dev/null +++ b/docs/guide-edit-interactions.md @@ -0,0 +1,40 @@ +# Guide editing drafts and interaction checks + +Edit Selected Guides previews orientation, position and colour on the canvas without editing +the document. OK sends one `view.editGuide` command; Cancel and Escape discard the preview. +An unchanged OK leaves Undo/Redo intact. Invalid text keeps the last valid preview and cannot +commit. A document revision, target or ruler-unit change invalidates the draft; reopen the +dialog to start again. Undoing a concurrent edit does not revive an old draft. +If the final command is refused (for example by the automation policy), the dialog reopens +with the typed fields, error and last valid preview. Retrying uses the original target and +revision; switching documents or editing the target before retry still invalidates the draft. + +The UI uses the `edit_session` module in ui-egui (`crates/ui-egui/src/edit_session.rs`) for the draft lifecycle and no-op/stale checks. +PhotoCraft retains guide selection, parsing, rendering and history ownership. Validation runs +the existing command on a temporary COW document only when parameters change. The live document +is changed only by the ordinary command after successful confirmation. Rounded ruler text that +was left untouched preserves the exact original position. + +## Running the scenarios + +The [interaction-check](https://github.com/PerchAnton/interaction-check) runner is development-only; +Python is not a PhotoCraft runtime dependency. To run the scenarios locally, use the +revision below. Automated execution needs a separate shared workflow and maintainer review. +The adapter belongs to PhotoCraft and calls the shipped dialog handlers, control field setter, +egui canvas/dialog frames, engine commands and history. Expectations are never passed to the +adapter. These are semantic integration checks, not a substitute for screenshot or pointer tests. + +```sh +git clone https://github.com/PerchAnton/interaction-check ../interaction-check +git -C ../interaction-check checkout --detach f8d4282d61576ffe78d18553c642d215887e2d9f +cargo build --locked -p photocraft-ui-egui --example guide_edit_scenarios +python3 ../interaction-check/interaction_check.py --timeout 30 \ + --report guide-edit-report.json tests/interactions/guide-edit/*.json -- \ + target/debug/examples/guide_edit_scenarios +``` + +Each scenario gets a fresh application with a 400 × 200 document at 100 ppi and vertical guides +at 10 and 20 px. Reported state reads committed and preview guides, history depth, Redo, dialog +validity and the real session command journal. The scenarios cover a single committed edit, +Undo/Redo, cancellation, invalid input and recovery, no-ops, fractional positions, orientation +and colour, document switching, deleted index reuse and an edit followed by Undo. diff --git a/tests/interactions/guide-edit/another-document.json b/tests/interactions/guide-edit/another-document.json new file mode 100644 index 0000000000..ef10decbe5 --- /dev/null +++ b/tests/interactions/guide-edit/another-document.json @@ -0,0 +1,67 @@ +{ + "schema": 1, + "name": "another-document", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "30 px" + }, + "expect": { + "preview": { + "vertical": [ + 30, + 20 + ] + } + } + }, + { + "event": { + "op": "other_document" + }, + "expect": { + "vertical": [ + 10 + ], + "preview": null, + "invalid": true + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": true, + "vertical": [ + 10 + ], + "history": 1, + "editCommands": 0, + "editing": false + } + } + ] +} diff --git a/tests/interactions/guide-edit/cancel-and-escape.json b/tests/interactions/guide-edit/cancel-and-escape.json new file mode 100644 index 0000000000..7f24cdb409 --- /dev/null +++ b/tests/interactions/guide-edit/cancel-and-escape.json @@ -0,0 +1,106 @@ +{ + "schema": 1, + "name": "cancel-and-escape", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "30 px" + }, + "expect": { + "preview": { + "vertical": [ + 30, + 20 + ] + } + } + }, + { + "event": { + "op": "cancel" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": null, + "editing": false + } + }, + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "40 px" + }, + "expect": { + "preview": { + "vertical": [ + 40, + 20 + ] + } + } + }, + { + "event": { + "op": "escape" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": null, + "editing": false + } + } + ] +} diff --git a/tests/interactions/guide-edit/changed-ruler-units.json b/tests/interactions/guide-edit/changed-ruler-units.json new file mode 100644 index 0000000000..7e2c48d094 --- /dev/null +++ b/tests/interactions/guide-edit/changed-ruler-units.json @@ -0,0 +1,54 @@ +{ + "schema": 1, + "name": "changed-ruler-units", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "command", + "id": "prefs.set", + "params": { + "path": "unitsAndRulers.rulers", + "value": "inches" + } + }, + "expect": { + "preview": null, + "invalid": true + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0 + } + } + ] +} diff --git a/tests/interactions/guide-edit/deleted-index-reused.json b/tests/interactions/guide-edit/deleted-index-reused.json new file mode 100644 index 0000000000..f8b607030c --- /dev/null +++ b/tests/interactions/guide-edit/deleted-index-reused.json @@ -0,0 +1,57 @@ +{ + "schema": 1, + "name": "deleted-index-reused", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "command", + "id": "view.deleteGuide", + "params": { + "orientation": "vertical", + "index": 0 + } + }, + "expect": { + "vertical": [ + 20 + ], + "preview": null, + "invalid": true + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": true, + "vertical": [ + 20 + ], + "history": 3, + "editCommands": 0, + "editing": false + } + } + ] +} diff --git a/tests/interactions/guide-edit/fractional-unchanged.json b/tests/interactions/guide-edit/fractional-unchanged.json new file mode 100644 index 0000000000..d6d3c30142 --- /dev/null +++ b/tests/interactions/guide-edit/fractional-unchanged.json @@ -0,0 +1,57 @@ +{ + "schema": 1, + "name": "fractional-unchanged", + "steps": [ + { + "event": { + "op": "command", + "id": "view.moveGuide", + "params": { + "orientation": "vertical", + "index": 0, + "position": 10.123456789 + } + }, + "expect": { + "vertical": [ + 10.123456789, + 20 + ] + } + }, + { + "event": { + "op": "command", + "id": "prefs.set", + "params": { + "path": "unitsAndRulers.rulers", + "value": "cm" + } + }, + "expect": { + "rejected": false + } + }, + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "vertical": [ + 10.123456789, + 20 + ], + "history": 3, + "editCommands": 0 + } + } + ] +} diff --git a/tests/interactions/guide-edit/invalid-then-correct.json b/tests/interactions/guide-edit/invalid-then-correct.json new file mode 100644 index 0000000000..d72e709e1e --- /dev/null +++ b/tests/interactions/guide-edit/invalid-then-correct.json @@ -0,0 +1,103 @@ +{ + "schema": 1, + "name": "invalid-then-correct", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "30 px" + }, + "expect": { + "preview": { + "vertical": [ + 30, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "far" + }, + "expect": { + "invalid": true, + "preview": { + "vertical": [ + 30, + 20 + ] + }, + "vertical": [ + 10, + 20 + ] + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": true, + "invalid": true, + "editing": true, + "history": 2, + "editCommands": 0 + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "40 px" + }, + "expect": { + "invalid": false, + "preview": { + "vertical": [ + 40, + 20 + ] + } + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": false, + "vertical": [ + 40, + 20 + ], + "history": 3, + "editCommands": 1 + } + } + ] +} diff --git a/tests/interactions/guide-edit/orientation-and-color.json b/tests/interactions/guide-edit/orientation-and-color.json new file mode 100644 index 0000000000..b7ac67db4a --- /dev/null +++ b/tests/interactions/guide-edit/orientation-and-color.json @@ -0,0 +1,95 @@ +{ + "schema": 1, + "name": "orientation-and-color", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "orientation", + "value": "horizontal" + }, + "expect": { + "preview": { + "vertical": [ + 20 + ], + "horizontal": [ + 10 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "50%" + }, + "expect": { + "preview": { + "horizontal": [ + 100 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "color", + "value": "magenta" + }, + "expect": { + "colors": [], + "preview": { + "colors": [ + { + "vertical": false, + "position": 100, + "rgb": [ + 255, + 0, + 255 + ] + } + ] + } + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "vertical": [ + 20 + ], + "horizontal": [ + 100 + ], + "history": 3, + "editCommands": 1 + } + } + ] +} diff --git a/tests/interactions/guide-edit/preview-commit-undo-redo.json b/tests/interactions/guide-edit/preview-commit-undo-redo.json new file mode 100644 index 0000000000..a27d2a45a1 --- /dev/null +++ b/tests/interactions/guide-edit/preview-commit-undo-redo.json @@ -0,0 +1,86 @@ +{ + "schema": 1, + "name": "preview-commit-undo-redo", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "set", + "field": "position", + "value": "25%" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "preview": { + "vertical": [ + 100, + 20 + ] + }, + "history": 2, + "editCommands": 0 + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "vertical": [ + 100, + 20 + ], + "history": 3, + "editCommands": 1, + "editing": false, + "preview": null + } + }, + { + "event": { + "op": "undo" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "redo": true + } + }, + { + "event": { + "op": "redo" + }, + "expect": { + "vertical": [ + 100, + 20 + ], + "redo": false + } + } + ] +} diff --git a/tests/interactions/guide-edit/unchanged-preserves-redo.json b/tests/interactions/guide-edit/unchanged-preserves-redo.json new file mode 100644 index 0000000000..6ee7c55ce8 --- /dev/null +++ b/tests/interactions/guide-edit/unchanged-preserves-redo.json @@ -0,0 +1,67 @@ +{ + "schema": 1, + "name": "unchanged-preserves-redo", + "steps": [ + { + "event": { + "op": "command", + "id": "view.moveGuide", + "params": { + "orientation": "vertical", + "index": 0, + "position": 15 + } + }, + "expect": { + "vertical": [ + 15, + 20 + ] + } + }, + { + "event": { + "op": "undo" + }, + "expect": { + "redo": true, + "history": 2 + } + }, + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "history": 2, + "redo": true, + "editCommands": 0, + "editing": false + } + } + ] +} diff --git a/tests/interactions/guide-edit/undo-does-not-revive-draft.json b/tests/interactions/guide-edit/undo-does-not-revive-draft.json new file mode 100644 index 0000000000..019dd79014 --- /dev/null +++ b/tests/interactions/guide-edit/undo-does-not-revive-draft.json @@ -0,0 +1,72 @@ +{ + "schema": 1, + "name": "undo-does-not-revive-draft", + "steps": [ + { + "event": { + "op": "begin" + }, + "expect": { + "editing": true, + "vertical": [ + 10, + 20 + ], + "history": 2, + "editCommands": 0, + "preview": { + "vertical": [ + 10, + 20 + ] + } + } + }, + { + "event": { + "op": "command", + "id": "view.moveGuide", + "params": { + "orientation": "vertical", + "index": 0, + "position": 15 + } + }, + "expect": { + "vertical": [ + 15, + 20 + ], + "preview": null + } + }, + { + "event": { + "op": "undo" + }, + "expect": { + "vertical": [ + 10, + 20 + ], + "redo": true, + "preview": null + } + }, + { + "event": { + "op": "confirm" + }, + "expect": { + "rejected": true, + "vertical": [ + 10, + 20 + ], + "redo": true, + "history": 2, + "editCommands": 0 + } + } + ] +}