#147: wire Picture Format UI for inserted pictures (#150)

* #147: wire Picture Format UI for inserted pictures

* #147: fix Picture Format state bugs from review (alt/crop buffers, undo coalescing, pending target, tab fallback)

* #147: review round 2 (cancel pending change on next action, alt media check, drag-only undo join)

---------

Co-authored-by: Tony <gittonyp@gmail.com>

crates/engine/src/cmd/objects.rs

@@ -570,7 +570,6 @@ mod tests {
         image::DynamicImage::ImageRgba8(img).write_to(&mut std::io::Cursor::new(&mut b), image::ImageFormat::Png).unwrap();
         b
     }
-
     #[test]
     fn picture_pipeline() {
         let mut s = Session::new(wordcraft_doc::Document::new());
@@ -596,4 +595,17 @@ mod tests {
         s.run("text.insert", &json!({"text": "x"})).unwrap();
         assert!(s.run("picture.crop", &json!({"left": 0.1})).is_err());
     }
+
+    #[test]
+    fn picture_alt_and_crop_round_trip() {
+        let mut s = Session::new(wordcraft_doc::Document::new());
+        let data = super::super::insert::base64_encode(&png(20, 10));
+        s.run("insert.picture", &json!({"data": data})).unwrap();
+        s.run("picture.altText", &json!({"text": "A red box"})).unwrap();
+        s.run("picture.crop", &json!({"left": 0.1, "top": 0.2, "right": 0.05, "bottom": 0.0})).unwrap();
+        let (_, o) = selected(&s).unwrap();
+        let InlineObject::Image { alt, crop, .. } = o else { panic!("expected image") };
+        assert_eq!(alt, "A red box");
+        assert_eq!(crop, [0.1, 0.2, 0.05, 0.0]);
+    }
 }

crates/ui-egui/src/canvas.rs

@@ -806,6 +806,12 @@ fn context_menu(app: &mut WordApp, ui: &mut Ui) {
     item(ui, app, "Paragraph…", "ui.dialog", json!({"name": "paragraph"}));
     item(ui, app, "Link…", "ui.dialog", json!({"name": "link"}));
     item(ui, app, "New Comment", "review.newComment", json!({}));
+    if crate::ribbon::has_picture_selected(&app.session) {
+        ui.separator();
+        item(ui, app, "Change Picture…", "ui.changePicture", json!({}));
+        item(ui, app, "Reset Picture", "picture.reset", json!({}));
+        item(ui, app, "Size and Crop…", "ui.tab", json!({"tab": "Picture Format"}));
+    }
     if app.session.sel.focus.path.cell().is_some() {
         ui.separator();
         ui.menu_button(tl!("Insert"), |ui| {

crates/ui-egui/src/lib.rs

@@ -103,6 +103,8 @@ pub struct WordApp {
     pub dialog: Option<dialogs::Dialog>,
     pub status_msg: Option<(String, f64)>,
     pub previews: previews::Previews,
+    /// Media key of the picture a pending Change Picture replaces (#147).
+    pub change_picture_target: Option<String>,
     /// macOS: the window has no title bar; leave room for the traffic lights.
     pub integrated_titlebar: bool,
     control_rx: Option<std::sync::mpsc::Receiver<ControlRequest>>,
@@ -150,6 +152,7 @@ impl WordApp {
             autosave: true,
             word_count: (0, 0),
             last_autosave: 0.0,
+            change_picture_target: None,
         }
     }
 
@@ -180,6 +183,10 @@ impl WordApp {
 
     /// Run a command; UI-level commands (`ui.*`) are handled here, the rest by the engine.
     pub fn run(&mut self, id: &str, params: Value) -> Result<Value, String> {
+        // A pending Change Picture only survives until the next action (#147).
+        if !matches!(id, "ui.changePicture" | "picture.change" | "insert.picture") {
+            self.change_picture_target = None;
+        }
         if let Some(r) = self.ui_command(id, &params) {
             return r;
         }
@@ -275,6 +282,14 @@ impl WordApp {
                 self.dialog = None;
                 json!({})
             }
+            "ui.changePicture" => {
+                let Some(media) = self.selected_picture_media() else {
+                    return Some(Err("select a picture first".into()));
+                };
+                self.change_picture_target = Some(media);
+                self.pick_picture();
+                json!({"pending": self.change_picture_target.is_some()})
+            }
             "ui.collapseRibbon" => {
                 self.ui.ribbon_collapsed = !self.ui.ribbon_collapsed;
                 json!({"collapsed": self.ui.ribbon_collapsed})
@@ -333,6 +348,35 @@ impl WordApp {
         }
     }
 
+    /// Media key of the selected picture, if any.
+    pub(crate) fn selected_picture_media(&self) -> Option<String> {
+        match wordcraft_engine::cmd::objects::selected(&self.session) {
+            Some((_, wordcraft_doc::para::InlineObject::Image { media, .. })) => Some(media),
+            _ => None,
+        }
+    }
+
+    /// A picked image replaces the pending Change Picture target, else it is inserted.
+    fn insert_or_change_picture(&mut self, params: Value) -> Result<Value, String> {
+        let target = std::mem::take(&mut self.change_picture_target);
+        match (target, self.selected_picture_media()) {
+            (Some(t), Some(m)) if t == m => self.run("picture.change", params),
+            _ => self.run("insert.picture", params),
+        }
+    }
+
+    /// Drop a pending Change Picture once the selection moved to another picture.
+    fn clear_stale_change_picture(&mut self) {
+        let stale = match (&self.change_picture_target, self.selected_picture_media()) {
+            (Some(t), Some(m)) => t != &m,
+            (Some(_), None) => true,
+            _ => false,
+        };
+        if stale {
+            self.change_picture_target = None;
+        }
+    }
+
     fn pick_picture(&mut self) {
         if let Some(f) = &self.services.open_async {
             f("picture");
@@ -340,7 +384,9 @@ impl WordApp {
         }
         let picked = self.services.pick_open.as_ref().and_then(|f| f("picture"));
         if let Some(path) = picked {
-            let _ = self.run("insert.picture", json!({"path": path}));
+            let _ = self.insert_or_change_picture(json!({"path": path}));
+        } else {
+            self.change_picture_target = None;
         }
     }
 
@@ -377,6 +423,7 @@ impl WordApp {
             self.applied_dark = Some(dark);
         }
         self.drain_control(ctx);
+        self.clear_stale_change_picture();
         self.drain_inbox();
         // AutoSave: write a saved document a couple of seconds after the last change.
         let now = now_ms();
@@ -471,7 +518,8 @@ impl WordApp {
             let lower = name.to_ascii_lowercase();
             let data = wordcraft_engine::cmd::insert::base64_encode(&bytes);
             let img = [".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp"].iter().any(|e| lower.ends_with(e));
-            let r = if img { self.run("insert.picture", json!({"data": data})) } else { self.run("file.open", json!({"path": name, "data": data})) };
+            let r =
+                if img { self.insert_or_change_picture(json!({"data": data})) } else { self.run("file.open", json!({"path": name, "data": data})) };
             if r.is_ok() {
                 self.ui.backstage = false;
             }
@@ -690,4 +738,75 @@ mod tests {
         assert!(a.ui.dark);
         assert!(!a.ui.backstage);
     }
+
+    fn png_bytes(c: [u8; 4]) -> Vec<u8> {
+        let img = image::RgbaImage::from_fn(20, 10, |_, _| image::Rgba(c));
+        let mut b = Vec::new();
+        image::DynamicImage::ImageRgba8(img).write_to(&mut std::io::Cursor::new(&mut b), image::ImageFormat::Png).unwrap();
+        b
+    }
+
+    fn insert_picture(a: &mut WordApp, c: [u8; 4]) {
+        let data = wordcraft_engine::cmd::insert::base64_encode(&png_bytes(c));
+        a.session.run("insert.picture", &json!({"data": data})).unwrap();
+    }
+
+    fn object_count(a: &mut WordApp) -> usize {
+        a.session.run("arrange.selectionPane", &json!({})).unwrap().as_array().map(|x| x.len()).unwrap_or(0)
+    }
+
+    /// A pending Change Picture replaces the selected image instead of inserting (#147).
+    #[test]
+    fn change_picture_replaces_instead_of_inserting() {
+        let mut a = app();
+        a.services.inbox = Some(std::sync::Arc::new(std::sync::Mutex::new(Vec::new())));
+        insert_picture(&mut a, [200, 30, 30, 255]);
+        assert_eq!(object_count(&mut a), 1);
+        let before = a.selected_picture_media().unwrap();
+        a.change_picture_target = a.selected_picture_media();
+        a.services.inbox.as_ref().unwrap().lock().unwrap().push(("new.png".into(), png_bytes([30, 200, 30, 255])));
+        a.drain_inbox();
+        assert_eq!(object_count(&mut a), 1);
+        assert_ne!(a.selected_picture_media().unwrap(), before);
+        assert!(a.change_picture_target.is_none());
+    }
+
+    /// Without a pending change, inbox images insert; stale targets clear on selection change.
+    #[test]
+    fn insert_without_pending_adds_and_stale_target_clears() {
+        let mut a = app();
+        a.services.inbox = Some(std::sync::Arc::new(std::sync::Mutex::new(Vec::new())));
+        insert_picture(&mut a, [200, 30, 30, 255]);
+        a.session.run("select.collapse", &json!({"end": true})).unwrap();
+        a.services.inbox.as_ref().unwrap().lock().unwrap().push(("second.png".into(), png_bytes([30, 30, 200, 255])));
+        a.drain_inbox();
+        assert_eq!(object_count(&mut a), 2);
+        // Stale target (another picture's key) clears instead of replacing the new selection.
+        a.change_picture_target = Some("m0000-gone.png".into());
+        a.clear_stale_change_picture();
+        assert!(a.change_picture_target.is_none());
+    }
+
+    /// `ui.changePicture` needs a selected picture; cancelling the picker clears the target.
+    #[test]
+    fn change_picture_guards_and_cancel_clears() {
+        let mut a = app();
+        assert!(a.run("ui.changePicture", json!({})).is_err());
+        assert!(a.change_picture_target.is_none());
+        insert_picture(&mut a, [200, 30, 30, 255]);
+        // No pickers in tests, so the picker "cancels" and the target clears.
+        assert!(a.run("ui.changePicture", json!({})).is_ok());
+        assert!(a.change_picture_target.is_none());
+    }
+
+    /// Any unrelated action cancels a pending Change Picture (#147 web-picker cancel case).
+    #[test]
+    fn unrelated_command_clears_change_picture_target() {
+        let mut a = app();
+        insert_picture(&mut a, [200, 30, 30, 255]);
+        a.change_picture_target = a.selected_picture_media();
+        assert!(a.change_picture_target.is_some());
+        let _ = a.run("format.bold", json!({}));
+        assert!(a.change_picture_target.is_none());
+    }
 }

crates/ui-egui/src/ribbon.rs

@@ -9,12 +9,40 @@ use crate::{WordApp, icons};
 
 pub const TABS: [&str; 11] = ["File", "Home", "Insert", "Draw", "Design", "Layout", "References", "Mailings", "Review", "View", "Help"];
 
-fn in_table(app: &WordApp) -> bool {
-    app.session.sel.focus.path.cell().is_some()
+/// True when the caret/selection touches a picture (#147).
+pub fn has_picture_selected(s: &wordcraft_engine::Session) -> bool {
+    matches!(wordcraft_engine::cmd::objects::selected(s), Some((_, wordcraft_doc::para::InlineObject::Image { .. })))
+}
+
+/// Contextual tabs for the current selection (pure, tested).
+pub fn contextual_tabs(s: &wordcraft_engine::Session) -> Vec<&'static str> {
+    let mut tabs = Vec::new();
+    if s.sel.focus.path.cell().is_some() {
+        tabs.push("Table Design");
+        tabs.push("Table Layout");
+    }
+    if has_picture_selected(s) {
+        tabs.push("Picture Format");
+    }
+    tabs
+}
+
+/// Tab to show when the stored tab is no longer applicable (pure, tested).
+pub fn resolve_tab<'a>(current: &'a str, available: &[&str]) -> &'a str {
+    if available.contains(&current) { current } else { "Home" }
 }
 
 pub fn show(app: &mut WordApp, ui: &mut Ui) {
     let t = Tokens::get(ui.ctx());
+    // A contextual tab (Table, Picture Format) may be stored while the selection moved away.
+    {
+        let mut tabs: Vec<&str> = TABS.to_vec();
+        tabs.extend(contextual_tabs(&app.session));
+        let next = resolve_tab(&app.ui.tab, &tabs);
+        if next != app.ui.tab {
+            app.ui.tab = next.to_string();
+        }
+    }
     // Tab strip.
     egui::Panel::top("tabs")
         .exact_size(30.0)
@@ -23,12 +51,13 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
             ui.horizontal_centered(|ui| {
                 ui.spacing_mut().item_spacing = vec2(2.0, 0.0);
                 let mut tabs: Vec<&str> = TABS.to_vec();
-                if in_table(app) {
-                    tabs.push("Table Design");
-                    tabs.push("Table Layout");
+                for ct in contextual_tabs(&app.session) {
+                    if !tabs.contains(&ct) {
+                        tabs.push(ct);
+                    }
                 }
                 for tab in tabs {
-                    let contextual = tab.starts_with("Table ");
+                    let contextual = tab.starts_with("Table ") || tab == "Picture Format";
                     let shown = tl!(tab);
                     let w = ui.ctx().fonts_mut(|f| f.layout_no_wrap(shown.to_string(), medium(12.5), t.text).size().x) + 18.0;
                     let (r, resp) = ui.allocate_exact_size(vec2(w, 30.0), Sense::click());
@@ -113,6 +142,7 @@ pub fn show(app: &mut WordApp, ui: &mut Ui) {
                         "Help" => help(app, ui),
                         "Table Design" => table_design(app, ui),
                         "Table Layout" => table_layout(app, ui),
+                        "Picture Format" => picture_format(app, ui),
                         _ => home(app, ui),
                     }
                 });
@@ -925,6 +955,133 @@ fn help(app: &mut WordApp, ui: &mut Ui) {
     });
 }
 
+fn picture_format(app: &mut WordApp, ui: &mut Ui) {
+    group(ui, "Adjust", None, app, |ui, app| {
+        menu_button(ui, app, "picture", Some("Corrections"), "Brightness, contrast and sharpness", true, |ui, app| {
+            for (l, b, c) in
+                [("Brighter +20%", 20.0, 0.0), ("Darker −20%", -20.0, 0.0), ("More Contrast +20%", 0.0, 20.0), ("Less Contrast −20%", 0.0, -20.0)]
+            {
+                mi(ui, app, l, "picture.corrections", json!({"brightness": b, "contrast": c}));
+            }
+            ui.separator();
+            mi(ui, app, "Sharpen", "picture.corrections", json!({"sharpen": 40.0}));
+            mi(ui, app, "Soften", "picture.corrections", json!({"sharpen": -40.0}));
+        });
+        menu_button(ui, app, "picture", Some("Color"), "Saturation and recolor", true, |ui, app| {
+            for (l, v) in [("Full Saturation 100%", 100.0), ("Muted 50%", 50.0), ("Gray 0%", 0.0), ("Vivid 200%", 200.0)] {
+                mi(ui, app, l, "picture.color", json!({"mode": "saturation", "saturation": v}));
+            }
+            ui.separator();
+            mi(ui, app, "Grayscale", "picture.color", json!({"mode": "grayscale"}));
+            mi(ui, app, "Sepia", "picture.color", json!({"mode": "sepia"}));
+            mi(ui, app, "Washout", "picture.color", json!({"mode": "washout"}));
+        });
+        menu_button(ui, app, "picture", Some("Transparency"), "Picture transparency", true, |ui, app| {
+            for v in [0.0, 15.0, 30.0, 50.0, 65.0, 80.0, 95.0] {
+                mi(ui, app, &format!("{v:.0}%"), "picture.transparency", json!({"percent": v}));
+            }
+        });
+        stack(ui, |ui| {
+            small(ui, app, "picture", Some("Change"), "Change Picture", "ui.changePicture", json!({}), false);
+            small(ui, app, "picture", Some("Reset"), "Reset Picture", "picture.reset", json!({}), false);
+        });
+    });
+    group(ui, "Picture Styles", None, app, |ui, app| {
+        menu_button(ui, app, "picture", Some("Styles"), "Frames and effects", true, |ui, app| {
+            for (l, s) in [
+                ("Simple Frame", "simpleFrame"),
+                ("Thick Frame", "thickFrame"),
+                ("Rounded", "rounded"),
+                ("Soft Edge", "softEdge"),
+                ("Shadow", "shadow"),
+            ] {
+                mi(ui, app, l, "picture.style", json!({"style": s}));
+            }
+        });
+        menu_button(ui, app, "picture", Some("Border"), "Picture border", true, |ui, app| {
+            for (l, w) in [("Thin", 2), ("Medium", 4), ("Thick", 8)] {
+                mi(ui, app, l, "picture.border", json!({"width": w}));
+            }
+        });
+    });
+    group(ui, "Arrange", None, app, |ui, app| {
+        big(ui, app, "position", "Position", "arrange.position", json!({}), false);
+        big(ui, app, "wrapText", "Wrap\nText", "arrange.wrap", json!({}), false);
+        stack(ui, |ui| {
+            small(ui, app, "rotate", Some("Rotate"), "Rotate", "arrange.rotate", json!({}), false);
+            small(ui, app, "align", Some("Align"), "Align", "arrange.align", json!({}), false);
+        });
+    });
+    group(ui, "Size", None, app, |ui, app| {
+        let (media, off, w0, h0, crop0, alt0) = match wordcraft_engine::cmd::objects::selected(&app.session) {
+            Some((pos, wordcraft_doc::para::InlineObject::Image { media, w, h, alt, crop, .. })) => (media, pos.off, w, h, crop, alt),
+            _ => (String::new(), 0, 0.0, 0.0, [0.0; 4], String::new()),
+        };
+        // `picture.size` locks the aspect ratio unless told otherwise, so one dimension suffices.
+        stack(ui, |ui| {
+            crate::widgets::row(ui, |ui| {
+                ui.label(egui::RichText::new(tl!("W:")).small());
+                let mut w = w0;
+                let r = ui.add(egui::DragValue::new(&mut w).speed(1.0).range(4.0..=2000.0).suffix(" pt"));
+                if r.changed() {
+                    if r.dragged() && !r.drag_started() {
+                        app.session.join_next_undo();
+                    }
+                    let _ = app.run("picture.size", json!({"width": w}));
+                }
+                ui.label(egui::RichText::new(tl!("H:")).small());
+                let mut h = h0;
+                let r = ui.add(egui::DragValue::new(&mut h).speed(1.0).range(4.0..=2000.0).suffix(" pt"));
+                if r.changed() {
+                    if r.dragged() && !r.drag_started() {
+                        app.session.join_next_undo();
+                    }
+                    let _ = app.run("picture.size", json!({"height": h}));
+                }
+            });
+            ui.add_space(2.0);
+            crate::widgets::row(ui, |ui| {
+                for (i, side) in ["L", "T", "R", "B"].iter().enumerate() {
+                    ui.label(egui::RichText::new(tl!(side)).small());
+                    let mut v = crop0[i] * 100.0;
+                    let r = ui.add(egui::DragValue::new(&mut v).speed(0.5).range(0.0..=45.0).suffix("%"));
+                    if r.changed() {
+                        if r.dragged() && !r.drag_started() {
+                            app.session.join_next_undo();
+                        }
+                        let mut c = crop0;
+                        c[i] = (v / 100.0).clamp(0.0, 0.45);
+                        let _ = app.run("picture.crop", json!({"left": c[0], "top": c[1], "right": c[2], "bottom": c[3]}));
+                    }
+                }
+                if ui.button(egui::RichText::new(tl!("Reset")).small()).on_hover_text(tl!("Reset Crop")).clicked() {
+                    let _ = app.run("picture.crop", json!({"left": 0.0, "top": 0.0, "right": 0.0, "bottom": 0.0}));
+                }
+            });
+            ui.add_space(2.0);
+            crate::widgets::row(ui, |ui| {
+                ui.label(egui::RichText::new(tl!("Alt:")).small());
+                // Frame-local buffers lose keystrokes; keep the draft in egui temp memory keyed
+                // by picture (same pattern as the comment editor in panes.rs).
+                let key = egui::Id::new(("picture_alt", media.clone(), off));
+                let mut alt = ui.data(|d| d.get_temp::<String>(key)).unwrap_or_else(|| alt0.clone());
+                let r = ui.add(egui::TextEdit::singleline(&mut alt).desired_width(120.0).hint_text(tl!("Alt text")));
+                if r.changed() {
+                    ui.data_mut(|d| d.insert_temp(key, alt.clone()));
+                }
+                if r.lost_focus() {
+                    ui.data_mut(|d| d.remove::<String>(key));
+                    // The click that took focus may have selected another picture; never commit
+                    // one picture's draft onto another.
+                    if alt != alt0 && app.selected_picture_media().as_deref() == Some(media.as_str()) {
+                        let _ = app.run("picture.altText", json!({"text": alt}));
+                    }
+                }
+            });
+        });
+    });
+}
+
 fn table_design(app: &mut WordApp, ui: &mut Ui) {
     let look = app.session.sel.focus.path.cell().and_then(|(tp, _, _)| app.session.doc.table(app.session.sel.focus.story, &tp).map(|t| t.props.look));
     group(ui, "Table Style Options", None, app, |ui, app| {
@@ -1064,3 +1221,40 @@ fn table_layout(app: &mut WordApp, ui: &mut Ui) {
         });
     });
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use wordcraft_engine::Session;
+
+    fn png_data() -> String {
+        let img = image::RgbaImage::from_fn(20, 10, |x, y| {
+            if x > 2 && x < 17 && y > 2 && y < 7 { image::Rgba([200, 30, 30, 255]) } else { image::Rgba([255, 255, 255, 255]) }
+        });
+        let mut b = Vec::new();
+        image::DynamicImage::ImageRgba8(img).write_to(&mut std::io::Cursor::new(&mut b), image::ImageFormat::Png).unwrap();
+        wordcraft_engine::cmd::insert::base64_encode(&b)
+    }
+
+    #[test]
+    fn picture_format_tab_appears_when_picture_selected() {
+        let mut s = Session::new(wordcraft_doc::Document::new());
+        assert!(!has_picture_selected(&s));
+        assert!(!contextual_tabs(&s).contains(&"Picture Format"));
+        s.run("insert.picture", &json!({"data": png_data()})).unwrap();
+        assert!(has_picture_selected(&s));
+        assert!(contextual_tabs(&s).contains(&"Picture Format"));
+        s.run("select.collapse", &json!({"end": true})).unwrap();
+        s.run("text.insert", &json!({"text": "x"})).unwrap();
+        assert!(!has_picture_selected(&s));
+    }
+
+    #[test]
+    fn resolve_tab_falls_back_when_contextual_tab_expires() {
+        let all = ["Home", "Table Design", "Picture Format"];
+        assert_eq!(resolve_tab("Picture Format", &all), "Picture Format");
+        assert_eq!(resolve_tab("Picture Format", &["Home"]), "Home");
+        assert_eq!(resolve_tab("Table Design", &["Home"]), "Home");
+        assert_eq!(resolve_tab("Home", &["Home"]), "Home");
+    }
+}