From 7a8d422dec9dc6a88fb7cc0d91867953aa01bf36 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Mon, 9 Nov 2026 14:47:06 -0500 Subject: [PATCH] M8: the Scale field only takes the keyboard when the tool is chosen or the panel returns, and Ctrl/Alt+Enter keep the tool (#2148 review) A stale "field had the keyboard" flag no longer ends the tool on the first frame back from another tab. A selection appearing after K with nothing selected no longer moves the keyboard from the canvas to the Scale field. Ctrl+Enter and Alt+Enter are not the tool's exit. Tests pin the full text selection, the 2x and 1.5 factors, the field letting go after Enter and Escape, idle frames after Tab, and a leftover edit dropped on return. Co-Authored-By: Claude Sonnet 5.5 Model-Effort: Opus 5.5 triaged and briefed; Sonnet 5.5 verified and wrote at medium --- crates/ui-egui/src/field_tests.rs | 94 +++++++++++++++++++++++- crates/ui-egui/src/panels/right.rs | 3 + crates/ui-egui/src/panels/scale_panel.rs | 29 +++++++- 3 files changed, 121 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/field_tests.rs b/crates/ui-egui/src/field_tests.rs index 06b78823c..718d681f6 100644 --- a/crates/ui-egui/src/field_tests.rs +++ b/crates/ui-egui/src/field_tests.rs @@ -763,7 +763,55 @@ fn pressing_k_focuses_the_scale_field_with_its_text_selected() { assert!(scale_field_focused(&u), "the scale field has the keyboard"); let state = egui::TextEdit::load_state(&u.ctx, Id::unique("scale_k").with("te")).expect("the editor exists"); let range = state.cursor.char_range().expect("a selection"); - assert_ne!(range.primary.index, range.secondary.index, "its text is selected"); + let shown = u.ctx.data(|d| d.get_temp::(Id::unique("scale_k").with("editing"))).expect("the field is being edited"); + let (lo, hi) = (range.primary.index.min(range.secondary.index), range.primary.index.max(range.secondary.index)); + assert_eq!((lo, hi), (egui::text::CCursor::new(0).index, egui::text::CCursor::new(shown.chars().count()).index), "all of its text is selected"); +} + +/// K with nothing selected, then a selection: the field doesn't take the keyboard from the canvas. +#[test] +fn a_selection_after_k_does_not_move_the_keyboard_to_the_scale_field() { + let (mut u, r) = scale_setup(); + u.app.run("select.set", json!({"ids": []})); + u.frame(vec![]); + key(&mut u, Key::K, Modifiers::NONE); + for _ in 0..3 { + u.frame(vec![]); + } + u.app.run("select.set", json!({"ids": [r]})); + for _ in 0..3 { + u.frame(vec![]); + } + assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Scale); + assert!(!scale_field_focused(&u), "the canvas keeps the keyboard"); +} + +/// Ctrl+Enter in the factor field is not the tool's exit. +#[test] +fn ctrl_enter_in_the_scale_field_keeps_the_scale_tool() { + let (mut u, _) = scale_setup(); + key(&mut u, Key::K, Modifiers::NONE); + u.frame(vec![]); + key(&mut u, Key::Enter, Modifiers::CTRL); + u.frame(vec![]); + assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Scale); +} + +/// An Enter on the first frame the panel is back from another tab (the field not yet focused again) +/// doesn't end the tool on a stale "had the keyboard" flag. +#[test] +fn enter_on_the_first_frame_back_from_another_tab_keeps_the_scale_tool() { + let (mut u, _) = scale_setup(); + key(&mut u, Key::K, Modifiers::NONE); + u.frame(vec![]); + assert!(scale_field_focused(&u)); + u.app.ui.right_tab = crate::state::RightTab::Prototype; + u.frame(vec![]); + u.frame(vec![]); + u.app.ui.right_tab = crate::state::RightTab::Design; + key(&mut u, Key::Enter, Modifiers::NONE); + u.frame(vec![]); + assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Scale); } /// The Scale tool chosen again while the Design tab was away (Prototype tab, Move, then K): the @@ -786,6 +834,30 @@ fn the_scale_field_takes_the_keyboard_again_after_another_tab() { assert!(scale_field_focused(&u), "the field has the keyboard again"); } +/// A value typed and left unsettled when the panel went away is dropped: back again, the field +/// shows the fresh value with all of its text selected, so typing replaces it. +#[test] +fn the_scale_field_comes_back_fresh_with_its_text_selected() { + let (mut u, r) = scale_setup(); + key(&mut u, Key::K, Modifiers::NONE); + u.frame(vec![]); + u.frame(vec![Event::Text("7".to_string())]); + u.app.ui.right_tab = crate::state::RightTab::Prototype; + u.frame(vec![]); + u.frame(vec![]); + u.app.ui.right_tab = crate::state::RightTab::Design; + u.frame(vec![]); + u.frame(vec![]); + assert!(scale_field_focused(&u)); + let shown = u.ctx.data(|d| d.get_temp::(Id::unique("scale_k").with("editing"))).expect("the field is being edited"); + assert_ne!(shown, "7", "the leftover edit is gone"); + let state = egui::TextEdit::load_state(&u.ctx, Id::unique("scale_k").with("te")).expect("the editor exists"); + let range = state.cursor.char_range().expect("a selection"); + let (lo, hi) = (range.primary.index.min(range.secondary.index), range.primary.index.max(range.secondary.index)); + assert_eq!((lo, hi), (egui::text::CCursor::new(0).index, egui::text::CCursor::new(shown.chars().count()).index)); + assert_eq!(geom(&u, r), (0.0, 0.0, 100.0, 100.0)); +} + /// Tab moves the keyboard from the factor field to the next one; an Enter there leaves the /// Scale tool on. #[test] @@ -798,6 +870,10 @@ fn enter_in_another_field_after_tab_keeps_the_scale_tool() { let ev = |key, pressed| Event::Key { key, physical_key: None, pressed, repeat: false, modifiers: Modifiers::NONE }; u.frame(vec![ev(Key::Tab, true), ev(Key::Tab, false)]); assert!(!scale_field_focused(&u), "Tab moved the keyboard on"); + for _ in 0..4 { + u.frame(vec![]); + assert!(!scale_field_focused(&u), "the field doesn't take the keyboard back"); + } u.frame(vec![ev(Key::Enter, true), ev(Key::Enter, false)]); u.frame(vec![]); assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Scale); @@ -815,6 +891,7 @@ fn k_two_enter_scales_the_selection_by_two() { u.frame(vec![]); // About the panel's origin, FigCraft's top-left default (Figma's grid starts at the centre). assert_eq!(geom(&u, r), (0.0, 0.0, 200.0, 200.0)); + assert!(!scale_field_focused(&u), "the field let go of the keyboard"); assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Move); assert_eq!(u.app.session.ds().unwrap().selection, vec![r]); assert_eq!(undo_steps(&u), steps + 1); @@ -822,6 +899,20 @@ fn k_two_enter_scales_the_selection_by_two() { assert_eq!(geom(&u, r), (0.0, 0.0, 100.0, 100.0)); } +/// "2x" and "1.5" are factors too. +#[test] +fn the_scale_field_takes_2x_and_1_5() { + for (text, size) in [("2x", 200.0), ("1.5", 150.0)] { + let (mut u, r) = scale_setup(); + key(&mut u, Key::K, Modifiers::NONE); + u.frame(vec![]); + u.frame(vec![Event::Text(text.to_string())]); + key(&mut u, Key::Enter, Modifiers::NONE); + u.frame(vec![]); + assert_eq!(geom(&u, r), (0.0, 0.0, size, size), "{text}"); + } +} + /// Escape in the scale field closes it and changes nothing. #[test] fn escape_in_the_scale_field_closes_it_and_changes_nothing() { @@ -833,6 +924,7 @@ fn escape_in_the_scale_field_closes_it_and_changes_nothing() { key(&mut u, Key::Escape, Modifiers::NONE); u.frame(vec![]); assert_eq!(geom(&u, r), (0.0, 0.0, 100.0, 100.0)); + assert!(!scale_field_focused(&u), "the field let go of the keyboard"); assert_eq!(u.app.session.tool.tool, figcraft_engine::ToolId::Move); assert_eq!(u.app.session.ds().unwrap().selection, vec![r]); assert_eq!(undo_steps(&u), steps); diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index bba57c778..84572016f 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -264,6 +264,9 @@ fn design(app: &mut FigApp, ui: &mut Ui) { let sel = ds.selection.clone(); let vector_edit = ds.vector_edit.clone(); if sel.is_empty() { + if app.session.tool.tool == figcraft_engine::ToolId::Scale { + crate::scale_panel::note_no_selection(ui); + } page_section(app, ui, &doc); return; } diff --git a/crates/ui-egui/src/panels/scale_panel.rs b/crates/ui-egui/src/panels/scale_panel.rs index 9b6dd98a1..dea30ae05 100644 --- a/crates/ui-egui/src/panels/scale_panel.rs +++ b/crates/ui-egui/src/panels/scale_panel.rs @@ -15,7 +15,10 @@ use crate::widgets::{self, FieldEdit, Lead}; /// The Scale section, shown in place of Position while the Scale tool is active. pub fn section(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId]) { - let Some(b) = doc.bounds_of(sel) else { return }; + let Some(b) = doc.bounds_of(sel) else { + note_no_selection(ui); + return; + }; widgets::section_header(ui, tr("Scale"), &[], false); // The factor field takes the keyboard, with its text selected, whenever the section shows // after a frame without it: the tool was chosen, or the panel came back from another tab or a @@ -25,7 +28,8 @@ pub fn section(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId]) { let frame = ui.ctx().cumulative_frame_nr(); let last = ui.data(|d| d.get_temp::(shown_id)); ui.data_mut(|d| d.insert_temp(shown_id, frame)); - if last.is_none_or(|l| l.saturating_add(1) < frame) { + let reappeared = last.is_none_or(|l| l.saturating_add(1) < frame); + if reappeared { widgets::request_edit(ui, k_id); } // Enter or Escape in the factor field ends the tool (back to Move, the selection kept). Whether @@ -33,8 +37,14 @@ pub fn section(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId]) { // shortcut layer may let go of it before this frame's panel runs, and a click or Tab that moved // it to another field during the last frame counts. let in_k_id = Id::unique("scale_in_k"); - let in_k = ui.data(|d| d.get_temp::(in_k_id)).unwrap_or(false); - let (enter, esc) = ui.input(|i| (i.key_pressed(egui::Key::Enter), i.key_pressed(egui::Key::Escape))); + // A flag left from before a gap (another tab, a hidden panel) is stale: the field didn't have + // the keyboard on the frame before this one. + let in_k = !reappeared && ui.data(|d| d.get_temp::(in_k_id)).unwrap_or(false); + // Plain Enter or Escape only: Ctrl+Enter and Alt+Enter are not the tool's exit. + let (enter, esc) = ui.input(|i| { + let plain = !(i.modifiers.ctrl || i.modifiers.alt || i.modifiers.command); + (plain && i.key_pressed(egui::Key::Enter), plain && i.key_pressed(egui::Key::Escape)) + }); let col = widgets::col_width(ui.available_width()); // Anchor: column and row (0 start, 1 middle, 2 end) of the point that stays put. let anchor = app.session.tool.scale_anchor; @@ -93,6 +103,17 @@ pub fn section(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId]) { } } +/// The Scale tool is on but nothing is selected, so the section isn't shown: the frame still +/// counts as shown (a selection that appears later is not the tool being chosen, and the field must +/// not take the keyboard from the canvas), and the field is not the one with the keyboard. +pub fn note_no_selection(ui: &Ui) { + let frame = ui.ctx().cumulative_frame_nr(); + ui.data_mut(|d| { + d.insert_temp(Id::unique("scale_shown_frame"), frame); + d.insert_temp(Id::unique("scale_in_k"), false); + }); +} + /// The factor (clamped to 0.01–100) and pivot for scaling `b` by `k` around `anchor`; None when /// nothing would change or `k` isn't a usable number. fn scale_about(b: figcraft_geom::Rect, anchor: (u8, u8), k: f64) -> Option<(f64, f64, f64)> {