From fb95d6b8b833a0fe12250ae56b710d2a058a2f53 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Sun, 12 Jul 2026 09:11:49 +0800 Subject: [PATCH] fix(editor): preserve scoped instance history states --- .../op-editor-core/src/instance_override.rs | 335 +++++++++--------- .../src/instance_override_history_tests.rs | 160 +++++++++ .../tests/instance_child_history.rs | 69 ++++ .../instance_fill_history_tests.rs | 185 ++++++++++ .../src/widget_host/instance_panel_tests.rs | 88 +---- 5 files changed, 591 insertions(+), 246 deletions(-) create mode 100644 crates/op-editor-core/src/instance_override_history_tests.rs create mode 100644 crates/op-editor-core/tests/instance_child_history.rs create mode 100644 crates/op-host-native/src/widget_host/instance_fill_history_tests.rs diff --git a/crates/op-editor-core/src/instance_override.rs b/crates/op-editor-core/src/instance_override.rs index a2737ad72..bfa5bca33 100644 --- a/crates/op-editor-core/src/instance_override.rs +++ b/crates/op-editor-core/src/instance_override.rs @@ -36,8 +36,10 @@ //! the color-picker HSV drag sites and the keyboard property-step //! path. Mutators that push history inside the scope would snapshot //! the swapped tree; `finish_instance_write` repairs any snapshot -//! captured during the scope by swapping the original Ref back in. +//! captured during the scope by routing each captured display state back +//! onto the original Ref. +use crate::history::EditorSnapshot; use crate::node_id::NodeId; use crate::pen_node_ext::PenNodeExt; use crate::state::EditorState; @@ -240,173 +242,198 @@ impl EditorState { /// direct props onto the Ref base, everything else into /// `descendants[target]`. Returns true when any write was routed. pub fn finish_instance_write(&mut self, scope: InstanceWriteScope) -> bool { - let InstanceWriteScope { - ref_id, - display_id, - target_id, - route_direct_props, - original_ref, - display_before, - history_push_count_before, - } = scope; - let routed = self.route_display_diff( - &display_id, - &target_id, - route_direct_props, - original_ref, - &display_before, - ); - if self.instance_write_virtual_anchor.as_ref() == Some(&display_id) { + let routed = self.route_display_diff(&scope); + if self.instance_write_virtual_anchor.as_ref() == Some(&scope.display_id) { self.instance_write_virtual_anchor = None; } - self.repair_scope_snapshots(&ref_id, &display_id, history_push_count_before); + self.repair_scope_snapshots(&scope); routed } - fn route_display_diff( - &mut self, - display_id: &NodeId, - target_id: &str, - route_direct_props: bool, - original_ref: PenNode, - before: &Map, - ) -> bool { - let Some(slot) = find_node_mut(self.active_children_mut(), display_id) else { + fn route_display_diff(&mut self, scope: &InstanceWriteScope) -> bool { + let Some(slot) = find_node_mut(self.active_children_mut(), &scope.display_id) else { // The node vanished during the scope (host-level delete); // nothing to restore into. return false; }; - let after = match serde_json::to_value(&*slot) { - Ok(Value::Object(map)) => map, - _ => { - *slot = original_ref; - return false; - } - }; - // Collect changed / removed top-level keys. - let mut direct: Vec<(String, Option)> = Vec::new(); - let mut overrides: Map = Map::new(); - let mut keys: Vec<&String> = before.keys().chain(after.keys()).collect(); - keys.sort(); - keys.dedup(); - for key in keys { - if STRUCTURAL_KEYS.contains(&key.as_str()) { - continue; - } - let old = before.get(key); - let new = after.get(key); - if old == new { - continue; - } - if route_direct_props && INSTANCE_DIRECT_PROPS.contains(&key.as_str()) { - direct.push((key.clone(), new.cloned())); - } else { - // Removed keys persist as explicit nulls so the - // override can CLEAR a component value. - overrides.insert(key.clone(), new.cloned().unwrap_or(Value::Null)); - } - } - if direct.is_empty() && overrides.is_empty() { - *slot = original_ref; - return false; - } - // Rebuild the RefNode JSON with the routed updates. - let mut ref_map = match serde_json::to_value(&original_ref) { - Ok(Value::Object(map)) => map, - _ => { - *slot = original_ref; - return false; - } - }; - for (key, value) in direct { - match value { - Some(v) => { - ref_map.insert(key, v); - } - None => { - ref_map.remove(&key); - } - } - } - if !overrides.is_empty() { - let descendants = ref_map - .entry("descendants") - .or_insert_with(|| Value::Object(Map::new())); - if let Value::Object(d) = descendants { - let entry = d - .entry(target_id.to_string()) - .or_insert_with(|| Value::Object(Map::new())); - if let Value::Object(existing) = entry { - for (key, value) in overrides { - existing.insert(key, value); - } - } - } - } - match serde_json::from_value::(Value::Object(ref_map)) { - Ok(updated) if matches!(updated, PenNode::Ref(_)) => { - *slot = updated; - true - } - _ => { - // A routed update the schema rejects must not erase - // the instance — restore it untouched. - *slot = original_ref; - false - } - } + let (updated, routed) = route_display_state( + &scope.original_ref, + &scope.target_id, + scope.route_direct_props, + &scope.display_before, + slot, + ); + *slot = updated; + routed } /// Any history snapshot pushed DURING the scope captured the - /// swapped display node in place of the Ref — undo would restore - /// a silently-detached instance. Swap the live (routed) Ref back - /// into those snapshots. Pre-scope snapshots are left untouched. - fn repair_scope_snapshots( - &mut self, - ref_id: &NodeId, - display_id: &NodeId, - history_push_count_before: u64, - ) { - let Some(live_ref) = find_node(self.active_children(), ref_id).cloned() else { - return; - }; - if !matches!(live_ref, PenNode::Ref(_)) { + /// swapped display node in place of the Ref — undo would restore a + /// silently-detached instance. Route each snapshot's own display state + /// back into a Ref so compound edits preserve every intermediate state. + fn repair_scope_snapshots(&mut self, scope: &InstanceWriteScope) { + if !matches!(scope.original_ref, PenNode::Ref(_)) { return; } - // Snapshot docs are `Arc`-shared at top-level granularity (remote's - // structurally-shared undo refactor), so this in-place fix is - // copy-on-write: `SharedDoc::repair_swap` `Arc::make_mut`s only the - // affected top-level entry, never contaminating a sibling snapshot. - // Ours: window the repair by PUSH COUNT (robust when the history - // cap evicted entries mid-scope) and sweep BOTH ids — the routed - // display node's id can differ from the Ref's for virtual - // instance-child anchors; `repair_swap` is a no-op when the id - // resolves to a healthy Ref, so the double sweep is idempotent. - let pushed = self + let pushes_in_scope = self .history_push_count - .saturating_sub(history_push_count_before) as usize; - for snap in self.history.past.iter_mut().rev().take(pushed) { - snap.doc.repair_swap(ref_id, &live_ref); - if display_id != ref_id { - snap.doc.repair_swap(display_id, &live_ref); - } + .saturating_sub(scope.history_push_count_before); + let repair_count = pushes_in_scope.min(self.history.past.len() as u64) as usize; + for snap in self.history.past.iter_mut().rev().take(repair_count) { + repair_scope_snapshot( + snap, + &scope.ref_id, + &scope.display_id, + &scope.target_id, + scope.route_direct_props, + &scope.original_ref, + &scope.display_before, + ); } // Pending pre-edit snapshots (colour picker / text edit) taken // inside the scope carry the same contamination signature — a // non-Ref node at the instance id — and the same repair. if let Some(snap) = self.ui.pending_color_history.as_mut() { - snap.doc.repair_swap(ref_id, &live_ref); - if display_id != ref_id { - snap.doc.repair_swap(display_id, &live_ref); - } + repair_scope_snapshot( + snap, + &scope.ref_id, + &scope.display_id, + &scope.target_id, + scope.route_direct_props, + &scope.original_ref, + &scope.display_before, + ); } if let Some(snap) = self.ui.pending_text_edit_history.as_mut() { - snap.doc.repair_swap(ref_id, &live_ref); - if display_id != ref_id { - snap.doc.repair_swap(display_id, &live_ref); + repair_scope_snapshot( + snap, + &scope.ref_id, + &scope.display_id, + &scope.target_id, + scope.route_direct_props, + &scope.original_ref, + &scope.display_before, + ); + } + } +} + +/// Route one display-node state onto the original Ref. This pure helper is +/// shared by the live scope finish and history-snapshot repair so both apply +/// identical direct-prop and descendants-override semantics. +fn route_display_state( + original_ref: &PenNode, + target_id: &str, + route_direct_props: bool, + before: &Map, + display_after: &PenNode, +) -> (PenNode, bool) { + let after = match serde_json::to_value(display_after) { + Ok(Value::Object(map)) => map, + _ => return (original_ref.clone(), false), + }; + let mut direct: Vec<(String, Option)> = Vec::new(); + let mut overrides: Map = Map::new(); + let mut keys: Vec<&String> = before.keys().chain(after.keys()).collect(); + keys.sort(); + keys.dedup(); + for key in keys { + if STRUCTURAL_KEYS.contains(&key.as_str()) { + continue; + } + let old = before.get(key); + let new = after.get(key); + if old == new { + continue; + } + if route_direct_props && INSTANCE_DIRECT_PROPS.contains(&key.as_str()) { + direct.push((key.clone(), new.cloned())); + } else { + // Removed keys persist as explicit nulls so the override can + // clear a component value. + overrides.insert(key.clone(), new.cloned().unwrap_or(Value::Null)); + } + } + if direct.is_empty() && overrides.is_empty() { + return (original_ref.clone(), false); + } + let mut ref_map = match serde_json::to_value(original_ref) { + Ok(Value::Object(map)) => map, + _ => return (original_ref.clone(), false), + }; + for (key, value) in direct { + match value { + Some(value) => { + ref_map.insert(key, value); + } + None => { + ref_map.remove(&key); } } } + if !overrides.is_empty() { + let descendants = ref_map + .entry("descendants") + .or_insert_with(|| Value::Object(Map::new())); + if let Value::Object(descendants) = descendants { + let entry = descendants + .entry(target_id.to_string()) + .or_insert_with(|| Value::Object(Map::new())); + if let Value::Object(existing) = entry { + for (key, value) in overrides { + existing.insert(key, value); + } + } + } + } + match serde_json::from_value::(Value::Object(ref_map)) { + Ok(updated) if matches!(updated, PenNode::Ref(_)) => (updated, true), + _ => (original_ref.clone(), false), + } +} + +fn repair_scope_snapshot( + snapshot: &mut EditorSnapshot, + ref_id: &NodeId, + display_id: &NodeId, + target_id: &str, + route_direct_props: bool, + pre_scope_ref: &PenNode, + display_before: &Map, +) { + let snapshot_node_id = if snapshot + .doc + .snapshot_find_node(snapshot.active_page_index, display_id) + .is_some() + { + display_id + } else if snapshot + .doc + .snapshot_find_node(snapshot.active_page_index, ref_id) + .is_some() + { + ref_id + } else { + return; + }; + let replacement = { + let display_at_snapshot = snapshot + .doc + .snapshot_find_node(snapshot.active_page_index, snapshot_node_id) + .expect("snapshot node checked above"); + if matches!(display_at_snapshot, PenNode::Ref(_)) { + return; + } + route_display_state( + pre_scope_ref, + target_id, + route_direct_props, + display_before, + display_at_snapshot, + ) + .0 + }; + snapshot.doc.repair_swap(snapshot_node_id, &replacement); } /// Run `write` (the same prop-write the panel would do) routed as an @@ -423,6 +450,10 @@ pub fn apply_instance_override( Some(result) } +#[cfg(test)] +#[path = "instance_override_history_tests.rs"] +mod history_tests; + #[cfg(test)] mod tests { use super::*; @@ -708,24 +739,6 @@ mod tests { ); } - #[test] - fn history_pushed_inside_scope_is_repaired_to_hold_the_ref() { - let mut s = state(); - apply_instance_override(&mut s, &NodeId::new("inst1"), |s| { - // Mirrors host arms that push history around the write. - s.commit_history(); - s.set_selected_color(true, "#00ff00") - }); - let snap = s.history.past.back().expect("history entry pushed"); - let snap_doc = snap.doc.materialize(); - let in_snap = crate::walkers::find_node(&snap_doc.children, &NodeId::new("inst1")) - .expect("inst1 in snapshot"); - assert!( - matches!(in_snap, PenNode::Ref(_)), - "scope snapshot repaired — undo must restore a Ref, not the display node" - ); - } - #[test] fn child_scope_history_is_repaired_to_hold_the_ref() { let mut s = state(); diff --git a/crates/op-editor-core/src/instance_override_history_tests.rs b/crates/op-editor-core/src/instance_override_history_tests.rs new file mode 100644 index 000000000..499d23799 --- /dev/null +++ b/crates/op-editor-core/src/instance_override_history_tests.rs @@ -0,0 +1,160 @@ +use super::*; +use crate::walkers::find_node; +use jian_ops_schema::node::container::{AlignItems, JustifyContent}; + +const HISTORY_DOC: &str = r##"{ + "version":"0.8.0", + "children":[ + {"type":"frame","id":"master","name":"Master","reusable":true, + "x":0,"y":0,"width":100,"height":100,"layout":"vertical", + "fill":[{"type":"solid","color":"#222222"}],"children":[]}, + {"type":"ref","id":"inst1","ref":"master","x":120,"y":0} + ] +}"##; + +fn state() -> EditorState { + let doc = jian_ops_schema::load_str(HISTORY_DOC) + .expect("fixture parses") + .value; + let mut state = EditorState::from_document(doc); + state.set_single_selection(NodeId::new("inst1")); + state +} + +fn resolved_instance_alignment( + state: &EditorState, +) -> (Option, Option) { + let node = find_node(state.active_children(), &NodeId::new("inst1")).expect("instance"); + let display = resolve_instance_display_node(&state.doc, node).expect("display"); + let PenNode::Frame(frame) = display else { + panic!("frame display"); + }; + (frame.container.justify_content, frame.container.align_items) +} + +fn apply_layout(state: &mut EditorState, node_id: NodeId, property: &str, value: &str) { + assert!(state.apply(crate::EditorCommand::SetNodeLayoutProp { + node_id, + property: property.to_string(), + value: crate::LayoutPropValue::Keyword(value.to_string()), + })); +} + +fn apply_compound_alignment(state: &mut EditorState) { + apply_instance_override(state, &NodeId::new("inst1"), |state| { + let id = state.selection.anchor.clone(); + state.commit_history(); + apply_layout(state, id.clone(), "justifyContent", "center"); + state.commit_history(); + apply_layout(state, id, "alignItems", "end"); + }); +} + +#[test] +fn history_pushed_inside_scope_is_repaired_to_hold_the_ref() { + let mut state = state(); + apply_instance_override(&mut state, &NodeId::new("inst1"), |state| { + state.commit_history(); + state.set_selected_color(true, "#00ff00") + }); + let snapshot = state.history.past.back().expect("history entry pushed"); + let document = snapshot.doc.materialize(); + let node = find_node(&document.children, &NodeId::new("inst1")).expect("instance in snapshot"); + assert!( + matches!(node, PenNode::Ref(_)), + "scope snapshot repaired — undo must restore a Ref, not the display node" + ); +} + +#[test] +fn scope_repairs_each_history_snapshot_to_its_own_display_state() { + let mut state = state(); + apply_compound_alignment(&mut state); + + assert_eq!(state.history.past.len(), 2); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), Some(AlignItems::End)) + ); + assert!(state.undo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), None) + ); + assert!(state.undo()); + assert_eq!(resolved_instance_alignment(&state), (None, None)); + assert!(!state.undo()); + + assert!(state.redo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), None) + ); + assert!(state.redo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), Some(AlignItems::End)) + ); + assert!(!state.redo()); +} + +#[test] +fn scope_repairs_new_snapshots_when_history_is_at_capacity() { + let mut state = state(); + for _ in 0..crate::HISTORY_CAP { + state.commit_history(); + } + assert_eq!(state.history.past.len(), crate::HISTORY_CAP); + + apply_compound_alignment(&mut state); + + assert_eq!(state.history.past.len(), crate::HISTORY_CAP); + assert!(state.undo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), None) + ); + assert!(state.undo()); + assert_eq!(resolved_instance_alignment(&state), (None, None)); + assert!(state.redo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), None) + ); + assert!(state.redo()); + assert_eq!( + resolved_instance_alignment(&state), + (Some(JustifyContent::Center), Some(AlignItems::End)) + ); +} + +#[test] +fn scope_repairs_pending_history_to_its_captured_display_state() { + let mut state = state(); + let id = NodeId::new("inst1"); + let scope = state.begin_instance_write(&id).expect("instance scope"); + apply_layout(&mut state, id.clone(), "justifyContent", "center"); + state.ui.pending_color_history = Some(state.snapshot_for_history()); + apply_layout(&mut state, id.clone(), "alignItems", "end"); + assert!(state.finish_instance_write(scope)); + + let pending = state + .ui + .pending_color_history + .as_ref() + .expect("pending snapshot retained"); + let document = pending.doc.materialize(); + let node = find_node(&document.children, &id).expect("instance in pending snapshot"); + let display = resolve_instance_display_node(&document, node).expect("pending display resolves"); + let PenNode::Frame(frame) = display else { + panic!("frame display"); + }; + assert_eq!( + frame.container.justify_content, + Some(JustifyContent::Center) + ); + assert_eq!( + frame.container.align_items, None, + "pending snapshot must not inherit the later live write" + ); +} diff --git a/crates/op-editor-core/tests/instance_child_history.rs b/crates/op-editor-core/tests/instance_child_history.rs new file mode 100644 index 000000000..ce0caae71 --- /dev/null +++ b/crates/op-editor-core/tests/instance_child_history.rs @@ -0,0 +1,69 @@ +use jian_ops_schema::node::PenNode; +use op_editor_core::{ + apply_instance_override, resolve_instance_display_node_for_anchor, EditorCommand, EditorState, + LayoutPropValue, NodeId, +}; + +fn state() -> EditorState { + let doc = jian_ops_schema::load_str( + r##"{"version":"0.8.0","children":[ + {"type":"frame","id":"master","name":"Master","reusable":true, + "x":0,"y":0,"width":100,"height":100,"children":[ + {"type":"rectangle","id":"surface","name":"Surface", + "width":80,"height":32,"cornerRadius":4} + ]}, + {"type":"ref","id":"inst","ref":"master","x":120,"y":0} + ]}"##, + ) + .expect("fixture parses") + .value; + let mut state = EditorState::from_document(doc); + state.set_single_selection(NodeId::new("inst__surface")); + state +} + +fn resolved_surface(state: &EditorState) -> (f64, Option) { + let display = + resolve_instance_display_node_for_anchor(&state.doc, &NodeId::new("inst__surface")) + .expect("virtual instance child resolves"); + assert!(matches!(display, PenNode::Rectangle(_))); + let value = serde_json::to_value(display).expect("display serializes"); + ( + value + .get("cornerRadius") + .and_then(serde_json::Value::as_f64) + .expect("corner radius"), + value.get("opacity").and_then(serde_json::Value::as_f64), + ) +} + +#[test] +fn virtual_child_scope_preserves_each_history_state() { + let mut state = state(); + let child = NodeId::new("inst__surface"); + apply_instance_override(&mut state, &child, |state| { + state.commit_history(); + assert!(state.apply(EditorCommand::SetNodeCornerRadius { + node_id: child.clone(), + radius: 8.0, + })); + state.commit_history(); + assert!(state.apply(EditorCommand::SetNodeLayoutProp { + node_id: child.clone(), + property: "opacity".to_string(), + value: LayoutPropValue::Number(0.5), + })); + }) + .expect("virtual child write scope"); + + assert_eq!(resolved_surface(&state), (8.0, Some(0.5))); + assert!(state.undo()); + assert_eq!(resolved_surface(&state), (8.0, None)); + assert!(state.undo()); + assert_eq!(resolved_surface(&state), (4.0, None)); + + assert!(state.redo()); + assert_eq!(resolved_surface(&state), (8.0, None)); + assert!(state.redo()); + assert_eq!(resolved_surface(&state), (8.0, Some(0.5))); +} diff --git a/crates/op-host-native/src/widget_host/instance_fill_history_tests.rs b/crates/op-host-native/src/widget_host/instance_fill_history_tests.rs new file mode 100644 index 000000000..4688f5fb9 --- /dev/null +++ b/crates/op-host-native/src/widget_host/instance_fill_history_tests.rs @@ -0,0 +1,185 @@ +//! Fill-order and compound instance-history tests split from the broader +//! instance panel suite to keep each source file under the repository limit. + +use super::WidgetHostNative; +use jian_ops_schema::node::container::{AlignItems, JustifyContent}; +use jian_ops_schema::node::PenNode; +use jian_ops_schema::style::PenFill; +use op_editor_core::NodeId; + +#[test] +fn native_move_fill_action_dispatches_as_one_undoable_edit() { + let mut host = WidgetHostNative::new(); + let doc = jian_ops_schema::load_str( + r##"{"version":"0.8.0","children":[{ + "type":"rectangle","id":"rect","name":"Rect", + "x":0,"y":0,"width":10,"height":10, + "fill":[ + {"type":"solid","color":"#111111"}, + {"type":"solid","color":"#222222"}, + {"type":"solid","color":"#333333"} + ] + }]}"##, + ) + .expect("fixture parses") + .value; + *host.editor_state_mut() = op_editor_core::EditorState::from_document(doc); + host.editor_state_mut() + .set_single_selection(NodeId::new("rect")); + + host.apply_property_action(op_editor_ui::widgets::PropertyPanelAction::MoveFill { + from: 2, + to: 0, + }); + + let node = op_editor_core::walkers::find_node( + host.editor_state().active_children(), + &NodeId::new("rect"), + ) + .expect("rect exists"); + let colors: Vec<_> = op_editor_core::fills::node_fills(node) + .expect("fills exist") + .iter() + .map(|fill| match fill { + PenFill::Solid(body) => body.color.as_str(), + other => panic!("expected solid, got {other:?}"), + }) + .collect(); + assert_eq!(colors, ["#333333", "#111111", "#222222"]); + assert_eq!(host.editor_state().history.past.len(), 1); +} + +#[test] +fn native_instance_move_fill_undo_restores_the_original_ref() { + let mut host = WidgetHostNative::new(); + let doc = jian_ops_schema::load_str( + r##"{"version":"0.8.0","children":[ + {"type":"rectangle","id":"master","name":"Master","reusable":true, + "x":0,"y":0,"width":10,"height":10, + "fill":[ + {"type":"solid","color":"#111111"}, + {"type":"solid","color":"#222222"}, + {"type":"solid","color":"#333333"} + ]}, + {"type":"ref","id":"inst","ref":"master","x":20,"y":0} + ]}"##, + ) + .expect("fixture parses") + .value; + *host.editor_state_mut() = op_editor_core::EditorState::from_document(doc); + host.editor_state_mut() + .set_single_selection(NodeId::new("inst")); + + host.apply_property_action(op_editor_ui::widgets::PropertyPanelAction::MoveFill { + from: 2, + to: 0, + }); + assert_eq!(host.editor_state().history.past.len(), 1); + assert!(host.editor_state_mut().undo()); + + let node = op_editor_core::walkers::find_node( + host.editor_state().active_children(), + &NodeId::new("inst"), + ) + .expect("instance exists after undo"); + let PenNode::Ref(reference) = node else { + panic!("undo must restore a Ref, got {node:?}"); + }; + assert!( + reference.descendants.is_none(), + "undo must remove the fill-order override" + ); + + assert!(host.editor_state_mut().redo()); + let node = op_editor_core::walkers::find_node( + host.editor_state().active_children(), + &NodeId::new("inst"), + ) + .expect("instance exists after redo"); + let display = op_editor_core::resolve_instance_display_node(&host.editor_state().doc, node) + .expect("instance resolves after redo"); + let colors: Vec<_> = op_editor_core::fills::node_fills(&display) + .expect("display fills") + .iter() + .map(|fill| match fill { + PenFill::Solid(body) => body.color.as_str(), + other => panic!("expected solid, got {other:?}"), + }) + .collect(); + assert_eq!(colors, ["#333333", "#111111", "#222222"]); + assert!( + !host.editor_state_mut().redo(), + "single edit has no ghost redo" + ); +} + +fn resolved_instance_alignment( + host: &WidgetHostNative, + id: &str, +) -> (Option, Option) { + let node = + op_editor_core::walkers::find_node(host.editor_state().active_children(), &NodeId::new(id)) + .expect("instance exists"); + let display = op_editor_core::resolve_instance_display_node(&host.editor_state().doc, node) + .expect("instance resolves"); + let PenNode::Frame(frame) = display else { + panic!("instance must resolve to a frame"); + }; + (frame.container.justify_content, frame.container.align_items) +} + +#[test] +fn native_compound_instance_alignment_undo_redo_preserves_each_history_state() { + use op_editor_ui::widgets::property_panel::{LayoutAlignValue, LayoutJustifyValue}; + use op_editor_ui::widgets::PropertyPanelAction; + + let mut host = WidgetHostNative::new(); + let doc = jian_ops_schema::load_str( + r##"{"version":"0.8.0","children":[ + {"type":"frame","id":"master","name":"Master","reusable":true, + "x":0,"y":0,"width":100,"height":100,"layout":"vertical", + "justifyContent":"start","alignItems":"start","children":[]}, + {"type":"ref","id":"inst","ref":"master","x":120,"y":0} + ]}"##, + ) + .expect("fixture parses") + .value; + *host.editor_state_mut() = op_editor_core::EditorState::from_document(doc); + host.editor_state_mut() + .set_single_selection(NodeId::new("inst")); + + host.apply_property_action(PropertyPanelAction::SetLayoutAlignment { + justify: LayoutJustifyValue::Center, + align: LayoutAlignValue::End, + }); + assert_eq!(host.editor_state().history.past.len(), 2); + assert_eq!( + resolved_instance_alignment(&host, "inst"), + (Some(JustifyContent::Center), Some(AlignItems::End)) + ); + + assert!(host.editor_state_mut().undo()); + assert_eq!( + resolved_instance_alignment(&host, "inst"), + (Some(JustifyContent::Center), Some(AlignItems::Start)), + "first undo must preserve the first half of the compound action" + ); + assert!(host.editor_state_mut().undo()); + assert_eq!( + resolved_instance_alignment(&host, "inst"), + (Some(JustifyContent::Start), Some(AlignItems::Start)) + ); + assert!(!host.editor_state_mut().undo(), "no ghost undo entry"); + + assert!(host.editor_state_mut().redo()); + assert_eq!( + resolved_instance_alignment(&host, "inst"), + (Some(JustifyContent::Center), Some(AlignItems::Start)) + ); + assert!(host.editor_state_mut().redo()); + assert_eq!( + resolved_instance_alignment(&host, "inst"), + (Some(JustifyContent::Center), Some(AlignItems::End)) + ); + assert!(!host.editor_state_mut().redo(), "no ghost redo entry"); +} diff --git a/crates/op-host-native/src/widget_host/instance_panel_tests.rs b/crates/op-host-native/src/widget_host/instance_panel_tests.rs index 9b0a2f1d1..8d263f279 100644 --- a/crates/op-host-native/src/widget_host/instance_panel_tests.rs +++ b/crates/op-host-native/src/widget_host/instance_panel_tests.rs @@ -7,9 +7,11 @@ use super::WidgetHostNative; use jian_ops_schema::node::PenNode; -use jian_ops_schema::style::PenFill; use op_editor_core::{NodeId, PenNodeExt}; +#[path = "instance_fill_history_tests.rs"] +mod fill_history_tests; + const COMPONENT_DOC: &str = r##"{ "version":"0.8.0", "children":[ @@ -35,90 +37,6 @@ fn seeded_host() -> WidgetHostNative { host } -#[test] -fn native_move_fill_action_dispatches_as_one_undoable_edit() { - let mut host = WidgetHostNative::new(); - let doc = jian_ops_schema::load_str( - r##"{"version":"0.8.0","children":[{ - "type":"rectangle","id":"rect","name":"Rect", - "x":0,"y":0,"width":10,"height":10, - "fill":[ - {"type":"solid","color":"#111111"}, - {"type":"solid","color":"#222222"}, - {"type":"solid","color":"#333333"} - ] - }]}"##, - ) - .expect("fixture parses") - .value; - *host.editor_state_mut() = op_editor_core::EditorState::from_document(doc); - host.editor_state_mut() - .set_single_selection(NodeId::new("rect")); - - host.apply_property_action(op_editor_ui::widgets::PropertyPanelAction::MoveFill { - from: 2, - to: 0, - }); - - let node = op_editor_core::walkers::find_node( - host.editor_state().active_children(), - &NodeId::new("rect"), - ) - .expect("rect exists"); - let colors: Vec<_> = op_editor_core::fills::node_fills(node) - .expect("fills exist") - .iter() - .map(|fill| match fill { - PenFill::Solid(body) => body.color.as_str(), - other => panic!("expected solid, got {other:?}"), - }) - .collect(); - assert_eq!(colors, ["#333333", "#111111", "#222222"]); - assert_eq!(host.editor_state().history.past.len(), 1); -} - -#[test] -fn native_instance_move_fill_undo_restores_the_original_ref() { - let mut host = WidgetHostNative::new(); - let doc = jian_ops_schema::load_str( - r##"{"version":"0.8.0","children":[ - {"type":"rectangle","id":"master","name":"Master","reusable":true, - "x":0,"y":0,"width":10,"height":10, - "fill":[ - {"type":"solid","color":"#111111"}, - {"type":"solid","color":"#222222"}, - {"type":"solid","color":"#333333"} - ]}, - {"type":"ref","id":"inst","ref":"master","x":20,"y":0} - ]}"##, - ) - .expect("fixture parses") - .value; - *host.editor_state_mut() = op_editor_core::EditorState::from_document(doc); - host.editor_state_mut() - .set_single_selection(NodeId::new("inst")); - - host.apply_property_action(op_editor_ui::widgets::PropertyPanelAction::MoveFill { - from: 2, - to: 0, - }); - assert_eq!(host.editor_state().history.past.len(), 1); - assert!(host.editor_state_mut().undo()); - - let node = op_editor_core::walkers::find_node( - host.editor_state().active_children(), - &NodeId::new("inst"), - ) - .expect("instance exists after undo"); - let PenNode::Ref(reference) = node else { - panic!("undo must restore a Ref, got {node:?}"); - }; - assert!( - reference.descendants.is_none(), - "undo must remove the fill-order override" - ); -} - fn ref_node(host: &WidgetHostNative) -> &jian_ops_schema::node::RefNode { match op_editor_core::walkers::find_node( host.editor_state().active_children(),