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 <noreply@anthropic.com>
Model-Effort: Opus 5.5 triaged and briefed; Sonnet 5.5 verified and wrote at medium
This commit is contained in:
Trevor BrownandClaude Sonnet 5.5 committed 2026-11-09 14:47:06 -05:00
1 parent 197a390a84
commit 7a8d422dec
3 files changed
+121 -5

No files matched your search

+93 -1
View File
@@ -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::<String>(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::<String>(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);
+3
View File
@@ -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;
}
+25 -4
View File
@@ -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::<u64>(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::<bool>(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::<bool>(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)> {