UI: clicking into a value or inline text field selects all of it (#170)

Cause: the click-to-edit fields (hot numbers and revolutions, the Timeline
time display and layer comments, Project rename/comment, the Render Queue
output path, Essential Graphics control names) opened a text edit with
egui's default caret at the end of the text, so typing appended to the old
value. The Timeline time display, layer comment and Essential Graphics name
editors also re-requested focus before checking `lost_focus()`, which hid
the loss, so Enter, Escape and clicking away never closed them.

Fix: a shared `widgets::keep_focus` keeps an open inline editor focused and
selects its whole text on the frame it opens (`widgets::select_all`, which
the layer rename now reuses). Those editors check for a lost focus first,
and Escape now cancels in the time display and the Essential Graphics
name editor.

Test: ui_angles `typing_replaces_the_clicked_value` (degrees and revolutions
in Effect Controls, Timeline and Properties, typed without Ctrl+A);
ui_inline_edit `project_rename_replaces_the_name` and
`timecode_field_replaces_commits_and_cancels`.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
bflatasticandClaude Opus 5.5 committed 2026-10-07 14:23:18 +07:00
1 parent 6e2f0d9682
commit bcf5f32d9b
7 files changed
+122 -16

No files matched your search

+4 -4
View File
@@ -26,8 +26,8 @@ use effectcraft_engine::project::{ItemId, ItemKind, LayerSource, ParamUi, Proper
use egui::{Rect, RichText};
use serde_json::{Value, json};
use crate::EffectcraftApp;
use crate::panels::DragPayload;
use crate::{EffectcraftApp, widgets};
type Actions = Vec<(String, Value)>;
@@ -296,13 +296,13 @@ fn name_label(app: &mut EffectcraftApp, ui: &mut egui::Ui, cid: ItemId, c: &EgCo
Some(mut buf) => {
let r = ui.add(egui::TextEdit::singleline(&mut buf).desired_width(120.0));
app.auto.add(&format!("essential.control.{}.rename", c.id), r.rect, &c.name);
r.request_focus();
if r.lost_focus() || ui.input(|i| i.key_pressed(egui::Key::Enter)) {
if !buf.trim().is_empty() && buf.trim() != c.name {
if r.lost_focus() {
if !ui.input(|i| i.key_pressed(egui::Key::Escape)) && !buf.trim().is_empty() && buf.trim() != c.name {
actions.push(("essential.rename".into(), json!({"comp": cid.0, "control": c.id, "name": buf.trim()})));
}
ui.ctx().data_mut(|d| d.remove::<String>(edit_id));
} else {
widgets::keep_focus(ui.ctx(), &r, &buf);
ui.ctx().data_mut(|d| d.insert_temp(edit_id, buf));
}
}
+1 -1
View File
@@ -480,7 +480,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
actions.push((cmd.into(), params));
}
} else {
resp.request_focus();
widgets::keep_focus(&ctx, &resp, &buf);
ctx.data_mut(|d| d.insert_temp::<Editing>(edit_id(), (eid, ef, buf)));
}
true
+1 -1
View File
@@ -715,7 +715,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
}
editing = None;
} else {
resp.request_focus();
widgets::keep_focus(ui.ctx(), &resp, &buf);
editing = Some((it.id, buf));
}
} else {
+7 -9
View File
@@ -915,11 +915,13 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
if let Some(mut buf) = ctx.data(|d| d.get_temp::<String>(egui::Id::new("tl-tc-edit"))) {
let mut child = ui.new_child(egui::UiBuilder::new().max_rect(tc_rect));
let r = child.add(egui::TextEdit::singleline(&mut buf).font(Tokens::semibold(16.0)).desired_width(150.0));
r.request_focus();
if r.lost_focus() {
let _ = app.session.execute("time.set", json!({"timecode": buf}));
if !ui.input(|i| i.key_pressed(egui::Key::Escape)) {
let _ = app.session.execute("time.set", json!({"timecode": buf}));
}
ctx.data_mut(|d| d.remove::<String>(egui::Id::new("tl-tc-edit")));
} else {
widgets::keep_focus(&ctx, &r, &buf);
ctx.data_mut(|d| d.insert_temp(egui::Id::new("tl-tc-edit"), buf));
}
}
@@ -1357,15 +1359,11 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
let renaming: Option<(u64, String)> = ctx.data(|d| d.get_temp(rename_id));
if let Some((rl, mut buf)) = renaming.filter(|(rl, _)| *rl == layer.id.0) {
let mut child = ui.new_child(egui::UiBuilder::new().max_rect(name_rect.shrink2(vec2(0.0, 2.0))));
let out = egui::TextEdit::singleline(&mut buf).font(Tokens::ui(12.0)).desired_width(name_rect.width()).show(&mut child);
let er = out.response;
let er = child.add(egui::TextEdit::singleline(&mut buf).font(Tokens::ui(12.0)).desired_width(name_rect.width()));
if ctx.data_mut(|d| d.remove_temp::<bool>(rename_focus_id())).unwrap_or(false) {
// Just started: focus the field with the whole name selected.
er.request_focus();
let mut state = out.state;
let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(buf.chars().count()));
state.cursor.set_char_range(Some(all));
state.store(&ctx, er.id);
widgets::select_all(&ctx, er.id, &buf);
}
if er.lost_focus() {
if !ui.input(|i| i.key_pressed(egui::Key::Escape)) {
@@ -2488,13 +2486,13 @@ fn layer_cells(
if let Some((el, mut buf)) = editing.filter(|(el, _)| *el == lid) {
let mut child = ui.new_child(egui::UiBuilder::new().max_rect(cr));
let er = child.add(egui::TextEdit::singleline(&mut buf).font(Tokens::ui(11.5)).desired_width(cr.width()));
er.request_focus();
if er.lost_focus() {
if !ui.input(|i| i.key_pressed(egui::Key::Escape)) && buf != layer.comment {
actions.push(("layer.setComment".into(), json!({"layers": [el], "comment": buf})));
}
ctx.data_mut(|d| d.remove::<(u64, String)>(edit_id));
} else {
widgets::keep_focus(&ctx, &er, &buf);
ctx.data_mut(|d| d.insert_temp(edit_id, (el, buf)));
}
} else {
+18 -1
View File
@@ -38,8 +38,8 @@ pub fn hot_number_at(
}
ui.data_mut(|d| d.remove::<String>(editing_id));
} else {
keep_focus(ui.ctx(), &r, &buf);
ui.data_mut(|d| d.insert_temp(editing_id, buf));
r.request_focus();
}
return (er, out, out.is_some());
}
@@ -94,6 +94,23 @@ pub fn hot_int_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: i64, s
(rect, (step != 0.0).then(|| value.saturating_add(step as i64)), done)
}
/// Keep the keyboard on an inline editor that is open until it loses the focus. The frame it
/// opens (it has no focus yet) its whole `text` is selected, so typing replaces the value.
pub fn keep_focus(ctx: &egui::Context, field: &Response, text: &str) {
if !ctx.memory(|m| m.has_focus(field.id)) {
select_all(ctx, field.id, text);
}
field.request_focus();
}
/// Select all of the text field `id`'s `text`, so typing replaces it.
pub fn select_all(ctx: &egui::Context, id: egui::Id, text: &str) {
let mut state = egui::TextEdit::load_state(ctx, id).unwrap_or_default();
let all = egui::text::CCursorRange::two(egui::text::CCursor::default(), egui::text::CCursor::new(text.chars().count()));
state.cursor.set_char_range(Some(all));
state.store(ctx, id);
}
/// A colour swatch; returns the response (click opens the picker in the caller).
pub fn swatch(ui: &mut Ui, rect: Rect, c: [f32; 4], id: egui::Id, t: &Tokens) -> Response {
let resp = ui.interact(rect, id, Sense::click());
+16
View File
@@ -156,6 +156,22 @@ fn valid_angles_commit_on_click_away_with_undo() {
commit_edits(true);
}
/// Clicking a value selects all of it: typing replaces it without selecting it first (#170).
#[test]
fn typing_replaces_the_clicked_value() {
for panel in [Panel::EffectControls, Panel::Timeline, Panel::Properties] {
for (revolutions, text, expected) in [(true, "3", 1110.5), (false, "45.5", 405.5)] {
let (mut h, layer, uid) = harness(panel, 390.5);
let p = component(&h, panel, uid, revolutions);
click(&mut h, p);
h.input_mut().events.push(Event::Text(text.into()));
h.step();
key(&mut h, Key::Enter, Modifiers::NONE);
assert_eq!(angle(&h, layer, uid), expected, "{panel:?}");
}
}
}
fn commit_edits(click_away: bool) {
for panel in [Panel::EffectControls, Panel::Timeline, Panel::Properties] {
for (revolutions, text, expected) in [(true, "3", 1110.5), (false, "45.5", 405.5)] {
+75
View File
@@ -0,0 +1,75 @@
//! Inline text fields (a name, the current time) open with their whole text selected so typing
//! replaces it, commit on Enter and cancel on Escape (#170).
use effectcraft_engine::Session;
use effectcraft_ui_egui::EffectcraftApp;
use egui::{Event, Key, Modifiers, PointerButton, Pos2, pos2};
use egui_kittest::Harness;
use serde_json::json;
fn harness() -> (Harness<'static, EffectcraftApp>, u64) {
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "duration": 4, "fps": 24})).unwrap();
let comp = s.active_comp_id().unwrap().0;
let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s));
h.run_steps(4);
(h, comp)
}
fn center(h: &Harness<'_, EffectcraftApp>, id: &str) -> Pos2 {
let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}"));
pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0)
}
fn click(h: &mut Harness<'_, EffectcraftApp>, p: Pos2, times: usize) {
h.input_mut().events.push(Event::PointerMoved(p));
h.step();
// All in one frame (the harness' frames are longer apart than a double-click).
for _ in 0..times {
h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
}
h.run_steps(3);
}
fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key) {
h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE });
h.run_steps(2);
}
fn type_text(h: &mut Harness<'_, EffectcraftApp>, text: &str) {
assert!(h.ctx.egui_wants_keyboard_input(), "the field has the keyboard");
h.input_mut().events.push(Event::Text(text.into()));
h.step();
}
#[test]
fn project_rename_replaces_the_name() {
let (mut h, comp) = harness();
let name = |h: &Harness<'_, EffectcraftApp>| h.state().session.project.items.values().find(|i| i.id.0 == comp).unwrap().name.clone();
let p = center(&h, &format!("project.item.{comp}.name"));
click(&mut h, p, 1);
key(&mut h, Key::Enter);
type_text(&mut h, "Hero");
key(&mut h, Key::Enter);
assert_eq!(name(&h), "Hero");
assert!(!h.ctx.egui_wants_keyboard_input(), "Enter releases the keyboard");
}
#[test]
fn timecode_field_replaces_commits_and_cancels() {
let (mut h, _) = harness();
let frame = |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().frame_rate.frame_at(h.state().session.time());
let p = center(&h, "timeline.timecode");
click(&mut h, p, 2);
type_text(&mut h, "0:00:01:00");
key(&mut h, Key::Enter);
assert_eq!(frame(&h), 24, "Enter commits the typed time");
assert!(!h.ctx.egui_wants_keyboard_input(), "Enter releases the keyboard");
click(&mut h, p, 2);
type_text(&mut h, "0:00:02:00");
key(&mut h, Key::Escape);
assert_eq!(frame(&h), 24, "Escape cancels");
assert!(!h.ctx.egui_wants_keyboard_input(), "Escape releases the keyboard");
}