From 1916e882c31374800ac83f9c8bc6a362c71320ca Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 24 Sep 2026 02:36:46 -0700 Subject: [PATCH] feat: delete a selected picture and leave it with the arrow keys as OneNote does Delete or Backspace removes the selected picture as one undo step. Left and Right leave it for the end of the text outline before it in page order or the start of the one after, as OneNote 2010 does; OneNote has no arrow nudge for pictures. Its Up and Down wrap the picture into a new outline with a caption below, which needs pictures inside outlines and is not modelled. Assisted-by: claude-opus-5.5 --- crates/canvas/src/editor.rs | 163 +++++++++++++++++++++++++++++++++++ crates/snowbound/src/main.rs | 25 ++++++ 2 files changed, 188 insertions(+) diff --git a/crates/canvas/src/editor.rs b/crates/canvas/src/editor.rs index c29b8789cb0ee4fe092e0dc0d5389d997206f15a..722e7da771ad871ddfd4b268133ca9436c53e459 100644 --- a/crates/canvas/src/editor.rs +++ b/crates/canvas/src/editor.rs @@ -624,6 +624,11 @@ enum History { image: ExGuid, layout: onestore::document::Layout, }, + /// Restores the picture at `index` in paint order, or removes the one there when `None`. + Picture { + index: usize, + image: Option>, + }, Remove { outline: onestore::ExGuid, focus: RestoreFocus, @@ -1336,6 +1341,62 @@ impl CanvasEditor { Ok(()) } + /// Leaves a picture as OneNote's Left and Right arrows do: the caret goes to the end of the + /// text outline before it in page order, or to the start of the one after. False when there + /// is none. + pub fn step_from_image( + &mut self, + engine: &mut TextEngine, + id: ExGuid, + forward: bool, + ) -> Result { + let index = self + .objects + .iter() + .position(|object| matches!(object, page::Content::Image(image) if image.id == id)) + .ok_or(EditError::InvalidRange)?; + let editable = |object: &page::Content| match object { + page::Content::Editable(outline) => Some(*outline), + _ => None, + }; + let outline = if forward { + self.objects[index + 1..].iter().find_map(editable) + } else { + self.objects[..index].iter().rev().find_map(editable) + }; + let Some(outline) = outline else { + return Ok(false); + }; + self.focus_outline(outline)?; + let edge = if forward { + Movement::DocumentStart + } else { + Movement::DocumentEnd + }; + self.move_selection(engine, edge, false)?; + Ok(true) + } + + pub fn remove_image(&mut self, id: ExGuid) -> Result<(), EditError> { + let index = self + .objects + .iter() + .position(|object| { + matches!(object, page::Content::Image(image) if image.id == id && !image.background) + }) + .ok_or(EditError::InvalidRange)?; + self.finish_composition(); + let page::Content::Image(image) = self.objects.remove(index) else { + unreachable!() + }; + self.undo.push(History::Picture { + index, + image: Some(Box::new(image)), + }); + self.redo.clear(); + Ok(()) + } + pub fn selection(&self) -> Selection { self.active_outline().selection } @@ -2315,6 +2376,13 @@ impl CanvasEditor { self.outlines.iter().any(|item| item.id == *outline) } History::Image { image, .. } => self.image(*image).is_some(), + History::Picture { + index, + image: Some(_), + } => *index <= self.objects.len(), + History::Picture { index, image: None } => { + matches!(self.objects.get(*index), Some(page::Content::Image(_))) + } History::Remove { outline, focus } => { self.outlines.iter().any(|item| item.id == *outline) && match focus { @@ -2466,6 +2534,19 @@ impl CanvasEditor { image, layout: std::mem::replace(&mut self.image_mut(image).unwrap().layout, layout), }, + History::Picture { index, image } => History::Picture { + index, + image: match image { + Some(image) => { + self.objects.insert(index, page::Content::Image(*image)); + None + } + None => match self.objects.remove(index) { + page::Content::Image(image) => Some(Box::new(image)), + _ => unreachable!(), + }, + }, + }, History::Remove { outline, focus } => { let index = self .outlines @@ -4479,6 +4560,88 @@ mod tests { assert!(editor.redo(&mut engine).unwrap()); assert!(editor.redo(&mut engine).unwrap()); assert_eq!(stored(&editor), resized); + + let ids = |editor: &CanvasEditor| { + editor + .page() + .unwrap() + .objects + .iter() + .map(PageObject::id) + .collect::>() + }; + let before = ids(&editor); + assert_eq!( + editor.remove_image(background_id), + Err(EditError::InvalidRange) + ); + editor.remove_image(id).unwrap(); + assert!(editor.image_placement(id).is_none()); + assert_eq!(ids(&editor), [background_id]); + assert!(editor.undo(&mut engine).unwrap()); + assert_eq!(ids(&editor), before); + assert_eq!(stored(&editor), resized); + assert!(editor.redo(&mut engine).unwrap()); + assert_eq!(ids(&editor), [background_id]); + } + + #[test] + fn arrows_leave_a_picture_for_the_neighbouring_outlines_in_page_order() { + use onestore::page::{Image, Page, PageObject}; + let mut engine = TextEngine::default(); + let outline = |engine: &mut TextEngine, text: &str, x| { + PageObject::Outline( + TextOutline::new( + engine, + TextDocument::new(vec![Paragraph::new(text.into(), Default::default())]) + .unwrap(), + 120.0, + [x, 0.0], + ) + .unwrap() + .snapshot(), + ) + }; + let picture = Image { + size: None, + id: onestore::page::text::new_id().unwrap(), + layout: onestore::document::Layout { + x: Some(200.0), + y: Some(0.0), + max_width: Some(40.0), + max_height: Some(40.0), + ..Default::default() + }, + bytes: Some(std::sync::Arc::from(b"deferred image payload".as_slice())), + alt: None, + background: false, + }; + let id = picture.id; + let objects = vec![ + outline(&mut engine, "Before", 0.0), + PageObject::Image(picture), + outline(&mut engine, "After", 300.0), + ]; + let [before, after] = [0, 2].map(|index| objects[index].id()); + let mut editor = CanvasEditor::from_page( + Page { + title: String::new(), + identity: None, + created: None, + margin_origin: [36.0, 14.4], + definitions: BTreeMap::new(), + objects, + }, + &mut engine, + ) + .unwrap(); + let caret = |paragraph, offset| [TextPosition { paragraph, offset }; 2]; + assert!(editor.step_from_image(&mut engine, id, false).unwrap()); + assert_eq!(editor.active_outline().id, before); + assert_eq!(editor.selection().positions, caret(0, 6)); + assert!(editor.step_from_image(&mut engine, id, true).unwrap()); + assert_eq!(editor.active_outline().id, after); + assert_eq!(editor.selection().positions, caret(0, 0)); } #[test] diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index 2ada41bfb62d13332bbd9063db3c631c82178735..29d4a2eada308668ac875fc241e98cd26bab947e 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -889,6 +889,26 @@ impl State { let shift = self.modifiers.shift_key(); let command = self.modifiers.super_key(); let option = self.modifiers.alt_key(); + if let Some(ObjectFocus::Image(id)) = self.object_focus + && matches!(key, Key::Named(NamedKey::Backspace | NamedKey::Delete)) + { + self.editor.remove_image(id)?; + self.set_object_focus(None); + return self.changed(); + } + if let Some(ObjectFocus::Image(id)) = self.object_focus + && !(shift || command || option || self.modifiers.control_key()) + && let Key::Named(NamedKey::ArrowLeft | NamedKey::ArrowRight) = key + && self.editor.step_from_image( + &mut self.engine, + id, + key == &Key::Named(NamedKey::ArrowRight), + )? + { + self.set_object_focus(None); + self.reveal_focus()?; + return self.changed(); + } if let Some(focus) = self.object_focus { let undo = matches!(focus, ObjectFocus::Image(_)) && command @@ -1020,6 +1040,11 @@ impl State { } else { self.editor.undo(&mut self.engine)?; } + if let Some(ObjectFocus::Image(id)) = self.object_focus + && self.editor.image_placement(id).is_none() + { + self.set_object_focus(None); + } } "c" | "x" => { let [anchor, focus] = self.editor.selection().positions; -- 2.54.0