Layers panel: rename commits on click elsewhere, Enter or Tab; one at a time (#314) (#326)

crates/ui-egui/src/layer_row_ui.rs

@@ -223,5 +223,113 @@ pub fn recorded(ctx: &egui::Context) -> Vec<RowRects> {
     ctx.data(|d| d.get_temp::<Vec<RowRects>>(rects_id())).unwrap_or_default()
 }
 
+// ------------------------------------------------------------------ in-place rename (#314)
+
+/// The Layers panel's in-place rename. One at a time, like Photoshop: Enter, Tab or a click
+/// anywhere else commits it, Esc cancels it, and starting another rename commits this one.
+#[derive(Clone, Debug, PartialEq)]
+pub struct Rename {
+    pub layer: u64,
+    /// The name when the rename started (unchanged text renames nothing).
+    pub original: String,
+    pub text: String,
+    /// Focus was given to the field (only once: asking every frame kept it from ever losing focus).
+    pub focused: bool,
+    /// The pass its field was last drawn in.
+    pub pass: u64,
+}
+
+impl Rename {
+    /// The command that applies it, if it changes the name.
+    pub fn commit_action(&self) -> Option<(String, Value)> {
+        let name = self.text.trim();
+        (!name.is_empty() && name != self.original).then(|| ("layer.setProps".to_string(), json!({"layer": self.layer, "name": name})))
+    }
+}
+
+fn rename_id() -> egui::Id {
+    egui::Id::new("layer-rename")
+}
+
+/// The rename in progress, if any.
+pub fn rename(ctx: &egui::Context) -> Option<Rename> {
+    ctx.data(|d| d.get_temp::<Rename>(rename_id()))
+}
+
+/// The layer being renamed, if any.
+pub fn renaming(ctx: &egui::Context) -> Option<u64> {
+    rename(ctx).map(|r| r.layer)
+}
+
+/// Is a rename field on screen (drawn this pass or the last)? It owns the keyboard meanwhile.
+pub fn rename_active(ctx: &egui::Context) -> bool {
+    rename(ctx).is_some_and(|r| r.pass + 1 >= ctx.cumulative_pass_nr())
+}
+
+fn store(ctx: &egui::Context, r: Option<Rename>) {
+    ctx.data_mut(|d| match r {
+        Some(r) => {
+            d.insert_temp(rename_id(), r);
+        }
+        None => {
+            d.remove::<Rename>(rename_id());
+        }
+    });
+}
+
+/// Start renaming `layer` (now called `name`). A rename open on another layer is committed: its
+/// command is returned for the caller to run.
+pub fn start_rename(ctx: &egui::Context, layer: u64, name: &str) -> Option<(String, Value)> {
+    let previous = rename(ctx);
+    if previous.as_ref().is_some_and(|r| r.layer == layer) {
+        return None;
+    }
+    store(ctx, Some(Rename { layer, original: name.to_string(), text: name.to_string(), focused: false, pass: ctx.cumulative_pass_nr() }));
+    previous.and_then(|r| r.commit_action())
+}
+
+/// End the rename in progress: commit it (its command is returned) or cancel it.
+pub fn end_rename(ctx: &egui::Context, commit: bool) -> Option<(String, Value)> {
+    let r = rename(ctx)?;
+    store(ctx, None);
+    if commit { r.commit_action() } else { None }
+}
+
+/// Draw the rename field of `layer` in `rect` when it is the one being renamed, and handle its
+/// keys and focus. Returns the rename command when it was committed this frame.
+pub fn rename_field(ui: &mut egui::Ui, layer: u64, rect: Rect) -> Option<(String, Value)> {
+    let ctx = ui.ctx().clone();
+    let mut r = rename(&ctx).filter(|r| r.layer == layer)?;
+    let id = egui::Id::new(("layer-rename-field", layer));
+    let te = ui.put(rect, egui::TextEdit::singleline(&mut r.text).id(id).font(FontId::proportional(12.5)));
+    r.pass = ctx.cumulative_pass_nr();
+    if !r.focused {
+        te.request_focus();
+        // Photoshop selects the whole name, so typing replaces it.
+        if let Some(mut state) = egui::TextEdit::load_state(&ctx, id) {
+            let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(r.text.chars().count()));
+            state.cursor.set_char_range(Some(all));
+            state.store(&ctx, id);
+        }
+        r.focused = true;
+        store(&ctx, Some(r));
+        return None;
+    }
+    let (enter, tab, esc) = ctx.input(|i| (i.key_pressed(egui::Key::Enter), i.key_pressed(egui::Key::Tab), i.key_pressed(egui::Key::Escape)));
+    if esc {
+        store(&ctx, None);
+        None
+    } else if enter || tab || te.lost_focus() || !te.has_focus() {
+        store(&ctx, None);
+        r.commit_action()
+    } else {
+        store(&ctx, Some(r));
+        None
+    }
+}
+
 #[cfg(test)]
 mod tests;
+
+#[cfg(test)]
+mod rename_tests;

crates/ui-egui/src/layer_row_ui/rename_tests.rs

@@ -0,0 +1,192 @@
+//! #314: renaming a layer in place works like Photoshop. A click anywhere else, Enter or Tab
+//! commits; Esc cancels; only one rename is open at a time, and starting another (double-click,
+//! the context menu or Layer › Rename Layer) commits the first.
+
+use egui::{Event, Key, Modifiers, PointerButton, Pos2, pos2, vec2};
+use egui_kittest::Harness;
+use serde_json::json;
+
+use super::{recorded, rename, renaming};
+use crate::PhotocraftApp;
+
+/// A document with layers "Alpha", "Beta" and "Gamma" (Gamma on top and active).
+fn session() -> (photocraft_engine::Session, [u64; 3]) {
+    let mut s = photocraft_engine::Session::new();
+    s.execute("file.new", json!({"width": 64, "height": 48})).unwrap();
+    let ids = ["Alpha", "Beta", "Gamma"].map(|n| s.execute("layer.new.layer", json!({"name": n})).unwrap()["layer"].as_u64().unwrap());
+    (s, ids)
+}
+
+fn harness(ppp: f32) -> (Harness<'static, PhotocraftApp>, [u64; 3]) {
+    let (s, ids) = session();
+    let mut h =
+        Harness::builder().with_size(vec2(1440.0, 900.0)).with_pixels_per_point(ppp).with_step_dt(1.0 / 60.0).with_max_steps(64).build_eframe(move |cc| {
+            PhotocraftApp::setup_context(&cc.egui_ctx, Default::default());
+            PhotocraftApp::new(s, crate::Services::default())
+        });
+    let ctx = h.ctx.clone();
+    let (req, _rx) = crate::control::ControlRequest::new("ui.set", json!({"dock": {"collapsed": ["color", "properties", "history", "navigator"]}}));
+    crate::control::handle(h.state_mut(), &ctx, &req);
+    h.run_steps(8);
+    (h, ids)
+}
+
+fn name(h: &Harness<'_, PhotocraftApp>, layer: u64) -> String {
+    h.state().session.active().unwrap().doc.layer(photocraft_doc::LayerId(layer)).unwrap().name.clone()
+}
+
+/// Where `layer`'s name is drawn in the Layers panel.
+fn name_at(h: &Harness<'_, PhotocraftApp>, layer: u64) -> Pos2 {
+    let row = recorded(&h.ctx).into_iter().find(|r| r.layer == layer).expect("row drawn");
+    row.name.map_or(row.row.center(), |r| r.center())
+}
+
+fn button(h: &mut Harness<'_, PhotocraftApp>, at: Pos2, pressed: bool) {
+    h.event(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+}
+
+fn click(h: &mut Harness<'_, PhotocraftApp>, at: Pos2) {
+    h.hover_at(at);
+    h.run_steps(1);
+    button(h, at, true);
+    h.run_steps(1);
+    button(h, at, false);
+    h.run_steps(3);
+}
+
+fn double_click(h: &mut Harness<'_, PhotocraftApp>, at: Pos2) {
+    h.hover_at(at);
+    // Two clicks 2 frames apart (60 fps) are a double click; egui counts a click within 0.6 s of
+    // the click before the last as a triple click, so let the clock move past earlier clicks.
+    for _ in 0..40 {
+        h.step();
+    }
+    for _ in 0..2 {
+        button(h, at, true);
+        h.step();
+        button(h, at, false);
+        h.step();
+    }
+    h.run_steps(3);
+}
+
+fn key(h: &mut Harness<'_, PhotocraftApp>, k: Key) {
+    h.event(Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
+    h.event(Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE });
+    h.run_steps(3);
+}
+
+fn type_text(h: &mut Harness<'_, PhotocraftApp>, text: &str) {
+    h.event(Event::Text(text.into()));
+    h.run_steps(2);
+}
+
+/// Is `layer`'s rename field on screen (drawn last frame)?
+fn field_shown(h: &Harness<'_, PhotocraftApp>, layer: u64) -> bool {
+    h.ctx.read_response(egui::Id::new(("layer-rename-field", layer))).is_some()
+}
+
+/// A point on the canvas, away from every panel.
+fn canvas(h: &Harness<'_, PhotocraftApp>) -> Pos2 {
+    let r = h.ctx.content_rect();
+    pos2(r.width() * 0.4, r.height() * 0.5)
+}
+
+/// Start renaming `layer` by double-clicking its name; the name is selected, so typing replaces it.
+fn start(h: &mut Harness<'_, PhotocraftApp>, layer: u64) {
+    let at = name_at(h, layer);
+    double_click(h, at);
+    assert_eq!(renaming(&h.ctx), Some(layer), "double-click starts renaming");
+    assert!(field_shown(h, layer));
+}
+
+#[test]
+fn clicking_elsewhere_commits_the_rename() {
+    for ppp in [1.0, 2.0] {
+        let (mut h, [a, ..]) = harness(ppp);
+        start(&mut h, a);
+        type_text(&mut h, "Sky");
+        assert_eq!(name(&h, a), "Alpha", "not applied while typing");
+        let at = canvas(&h);
+        click(&mut h, at);
+        assert_eq!(name(&h, a), "Sky", "@{ppp}x: a click elsewhere commits");
+        assert_eq!(renaming(&h.ctx), None, "@{ppp}x: and closes the field");
+        h.run_steps(2);
+        assert!(!field_shown(&h, a));
+    }
+}
+
+#[test]
+fn enter_and_tab_commit_and_escape_cancels() {
+    let (mut h, [a, b, c]) = harness(1.0);
+    start(&mut h, a);
+    type_text(&mut h, "By Enter");
+    key(&mut h, Key::Enter);
+    assert_eq!((name(&h, a), renaming(&h.ctx)), ("By Enter".into(), None));
+
+    start(&mut h, b);
+    type_text(&mut h, "By Tab");
+    key(&mut h, Key::Tab);
+    assert_eq!((name(&h, b), renaming(&h.ctx)), ("By Tab".into(), None));
+
+    start(&mut h, c);
+    type_text(&mut h, "Never");
+    key(&mut h, Key::Escape);
+    assert_eq!((name(&h, c), renaming(&h.ctx)), ("Gamma".into(), None), "Esc cancels");
+    // Esc only cancelled the rename: the document and the layer selection are untouched.
+    assert_eq!(h.state().session.active().unwrap().active_layer, Some(photocraft_doc::LayerId(c)));
+
+    // An empty or unchanged name renames nothing.
+    start(&mut h, c);
+    type_text(&mut h, "   ");
+    key(&mut h, Key::Enter);
+    assert_eq!(name(&h, c), "Gamma");
+}
+
+#[test]
+fn starting_another_rename_commits_the_first_and_only_one_is_open() {
+    for ppp in [1.0, 2.0] {
+        let (mut h, [a, b, _]) = harness(ppp);
+        start(&mut h, a);
+        type_text(&mut h, "First");
+        start(&mut h, b);
+        assert_eq!(name(&h, a), "First", "@{ppp}x: the first rename was committed");
+        assert!(!field_shown(&h, a), "@{ppp}x: its field closed");
+        assert!(field_shown(&h, b));
+        type_text(&mut h, "Second");
+        assert_eq!(rename(&h.ctx).unwrap().text, "Second", "typing goes to the open field");
+        key(&mut h, Key::Enter);
+        assert_eq!((name(&h, a), name(&h, b)), ("First".into(), "Second".into()));
+        assert_eq!(renaming(&h.ctx), None);
+    }
+}
+
+#[test]
+fn layer_rename_layer_from_the_menu_commits_an_open_rename() {
+    let (mut h, [a, b, _]) = harness(1.0);
+    start(&mut h, a);
+    type_text(&mut h, "Kept");
+    let ctx = h.ctx.clone();
+    crate::menus::invoke(h.state_mut(), &ctx, "layer.renameLayer", json!({"layer": b})).unwrap();
+    h.run_steps(3);
+    assert_eq!(name(&h, a), "Kept");
+    assert_eq!(renaming(&h.ctx), Some(b));
+    assert!(field_shown(&h, b) && !field_shown(&h, a));
+    type_text(&mut h, "From menu");
+    key(&mut h, Key::Enter);
+    assert_eq!(name(&h, b), "From menu");
+}
+
+#[test]
+fn a_rename_ends_when_its_layer_goes_away() {
+    let (mut h, [a, ..]) = harness(1.0);
+    start(&mut h, a);
+    h.state_mut().run("layer.select", json!({"layer": a})).unwrap();
+    h.state_mut().run("layer.delete", json!({})).unwrap();
+    assert!(h.state().session.active().unwrap().doc.layer(photocraft_doc::LayerId(a)).is_none());
+    h.run_steps(3);
+    assert_eq!(renaming(&h.ctx), None);
+    // The keyboard is back to the tools: V selects the Move tool.
+    key(&mut h, Key::V);
+    assert_eq!(h.state().ui.tool, crate::state::Tool::Move);
+}

crates/ui-egui/src/menus.rs

@@ -206,7 +206,10 @@ pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id:
             let st = app.session.active().ok_or("no document")?;
             let id = params.get("layer").and_then(Value::as_u64).or(st.active_layer.map(|l| l.0)).ok_or("no active layer")?;
             let name = st.doc.layer(photocraft_doc::LayerId(id)).map(|l| l.name.clone()).ok_or("no such layer")?;
-            ctx.data_mut(|d| d.insert_temp(egui::Id::new(("rename", id)), name));
+            // Another rename in progress is committed first (#314).
+            if let Some((cmd, p)) = crate::layer_row_ui::start_rename(ctx, id, &name) {
+                app.run(&cmd, p)?;
+            }
             Ok(Value::Null)
         }
         "file.new" if params.as_object().is_none_or(|o| o.is_empty()) => {

crates/ui-egui/src/panels.rs

@@ -1386,6 +1386,14 @@ fn layers(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
                 }
                 crate::smart_ui::filter_rows(app, ui, l, depth, &mut actions);
             }
+            // A rename whose row is gone (deleted, filtered out, inside a closed group) ends,
+            // committed: nothing else could commit or cancel it.
+            if let Some(layer) = crate::layer_row_ui::renaming(ui.ctx())
+                && !crate::layer_row_ui::recorded(ui.ctx()).iter().any(|r| r.layer == layer)
+                && let Some(done) = crate::layer_row_ui::end_rename(ui.ctx(), true)
+            {
+                actions.push(done);
+            }
         });
     // End any layer drag after every row has had a chance to accept the drop.
     if ctx.input(|i| i.pointer.any_released()) {
@@ -1540,11 +1548,7 @@ fn layer_row(
     // A row scrolled out of view only keeps its place (#125): a layout's hundreds of rows would
     // otherwise lay out names, icons and thumbnails every frame. Group rows stay whole: their
     // disclosure triangle is a widget (accessibility, scroll-to).
-    if !ui.is_rect_visible(rect)
-        && !l.is_group()
-        && !resp.context_menu_opened()
-        && ctx.data(|d| d.get_temp::<String>(egui::Id::new(("rename", l.id.0)))).is_none()
-    {
+    if !ui.is_rect_visible(rect) && !l.is_group() && !resp.context_menu_opened() && crate::layer_row_ui::renaming(ctx) != Some(l.id.0) {
         return;
     }
     let painter = ui.painter_at(rect.expand(1.0));
@@ -1675,36 +1679,26 @@ fn layer_row(
     }
     // Double-click the name to rename in place (Photoshop ergonomics). The Background can't be
     // renamed while it's locked, so a double-click turns it into a normal layer instead.
-    let rename_id = egui::Id::new(("rename", l.id.0));
     if resp.double_clicked() {
         if crate::doc_props_ui::is_background(doc, l) {
             actions.push(("layer.new.layerFromBackground".into(), json!({})));
-        } else {
-            ctx.data_mut(|d| d.insert_temp(rename_id, l.name.clone()));
+        } else if let Some(done) = crate::layer_row_ui::start_rename(ctx, l.id.0, &l.name) {
+            // One rename at a time: starting this one commits any other (#314).
+            actions.push(done);
         }
     }
-    if let Some(mut text) = ctx.data(|d| d.get_temp::<String>(rename_id)) {
-        let edit_rect = Rect::from_min_max(pos2(x - 3.0, rect.center().y - 11.0), pos2(name_right.max(x + 40.0), rect.center().y + 11.0));
-        let te = ui.put(edit_rect, egui::TextEdit::singleline(&mut text).font(egui::FontId::proportional(12.5)));
-        te.request_focus();
-        let (enter, esc) = ui.input(|i| (i.key_pressed(egui::Key::Enter), i.key_pressed(egui::Key::Escape)));
-        if esc {
-            ctx.data_mut(|d| d.remove::<String>(rename_id));
-        } else if enter || te.lost_focus() {
-            ctx.data_mut(|d| d.remove::<String>(rename_id));
-            if !text.trim().is_empty() && text != l.name {
-                actions.push(("layer.setProps".into(), json!({"layer": l.id.0, "name": text.trim()})));
-            }
-        } else {
-            ctx.data_mut(|d| d.insert_temp(rename_id, text));
-        }
+    let edit_rect = Rect::from_min_max(pos2(x - 3.0, rect.center().y - 11.0), pos2(name_right.max(x + 40.0), rect.center().y + 11.0));
+    if let Some(done) = crate::layer_row_ui::rename_field(ui, l.id.0, edit_rect) {
+        actions.push(done);
     }
     // Right-click context menu.
     resp.context_menu(|ui| {
         // Right-clicking inside a multi-selection keeps it and acts on every selected layer.
         let on_set = row.multi && selected;
-        if crate::layer_menu_ui::show(app, ui, l, on_set, actions) {
-            ui.ctx().data_mut(|d| d.insert_temp(rename_id, l.name.clone()));
+        if crate::layer_menu_ui::show(app, ui, l, on_set, actions)
+            && let Some(done) = crate::layer_row_ui::start_rename(ui.ctx(), l.id.0, &l.name)
+        {
+            actions.push(done);
         }
     });
 }

crates/ui-egui/src/shortcut_dispatch.rs

@@ -95,7 +95,8 @@ pub enum Focus {
 
 impl Focus {
     pub fn of(ctx: &egui::Context) -> Focus {
-        if ctx.text_edit_focused() {
+        // A layer rename keeps its keys even in the frame egui drops its focus (Esc, Tab).
+        if ctx.text_edit_focused() || crate::layer_row_ui::rename_active(ctx) {
             Focus::Text
         } else if ctx.egui_wants_keyboard_input() {
             Focus::Widget