diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 3d9c34880..5c89d3468 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -267,10 +267,25 @@ fn whip_into_id(prop: u64) -> egui::Id { egui::Id::new(("tl-pickwhip-into", prop)) } -/// After a pick whip into the expression being edited: its editor takes the focus back, the -/// cursor this many characters before the end (just after the reference). -fn expr_refocus_id(prop: u64) -> egui::Id { - egui::Id::new(("tl-expr-refocus", prop)) +/// An expression editor to take the keyboard when it is next drawn: its property, the text to +/// select as counts of characters before the end (so it fits the expression as it will be then) +/// and the pass it was asked for in. After a pick whip into the expression being edited, the +/// cursor goes just after the reference; after an Alt-click on a stopwatch adds an expression, +/// its whole text is selected, so typing replaces it. +#[derive(Clone, Copy, Debug)] +struct ExprFocus { + prop: u64, + select: [usize; 2], + pass: u64, +} + +fn expr_focus_id() -> egui::Id { + egui::Id::new("tl-expr-focus") +} + +fn focus_expression(ctx: &egui::Context, prop: u64, select: [usize; 2]) { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| d.insert_temp(expr_focus_id(), ExprFocus { prop, select, pass })); } #[derive(Clone, Debug)] @@ -1968,6 +1983,19 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let rh = t.row_h; let total_h: f32 = rows.iter().map(|r| row_height(r, rh)).sum(); let max_scroll = (total_h - rows_rect.height() + rh).max(0.0); + // An expression editor asked to take the keyboard comes into view; a request whose + // expression didn't appear, or that wasn't taken within a few passes, is dropped (it never + // takes the keyboard later). + if let Some(f) = ctx.data(|d| d.get_temp::(expr_focus_id())) { + match rows.iter().position(|r| matches!(r.kind, RowKind::Expr { uid, .. } if uid == f.prop)) { + Some(i) if ctx.cumulative_pass_nr() <= f.pass.saturating_add(8) => { + let top: f32 = rows.iter().take(i).map(|r| row_height(r, rh)).sum(); + let fits = (top + rows.get(i).map_or(rh, |r| row_height(r, rh)) - rows_rect.height()).max(0.0).min(top); + app.ui.timeline.scroll_y = app.ui.timeline.scroll_y.min(top).max(fits); + } + _ => ctx.data_mut(|d| d.remove::(expr_focus_id())), + } + } if scroll_to == Some(crate::state::ScrollTo::Layer) { // The topmost selected layer, in stack order. let sel = &app.session.state.selected_layers; @@ -2781,12 +2809,15 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, t.expr_bg); let er = Rect::from_min_max(pos2(graph_x0 + 8.0, r.min.y + 3.0), pos2(rect.max.x - 14.0, r.max.y - 3.0)); let mut buf: String = ctx.data(|d| d.get_temp(buf_id)).unwrap_or_else(|| ex.text.clone()); - if let Some(after) = ctx.data_mut(|d| d.remove_temp::(expr_refocus_id(*uid))) { - // A pick whip just put a reference in: editing goes on in the new text, - // the cursor after the reference. + if let Some(f) = ctx.data(|d| d.get_temp::(expr_focus_id())).filter(|f| f.prop == *uid) { + // Asked for once (a pick whip put a reference in, or an Alt-click added the + // expression): editing starts in the expression as it is now. + ctx.data_mut(|d| d.remove::(expr_focus_id())); buf = ex.text.clone(); + let n = buf.chars().count(); + let [a, b] = f.select.map(|before_end| egui::text::CCursor::new(n.saturating_sub(before_end))); let mut st = egui::text_edit::TextEditState::load(&ctx, edit_id).unwrap_or_default(); - st.cursor.set_char_range(Some(egui::text::CCursorRange::one(egui::text::CCursor::new(buf.chars().count().saturating_sub(after))))); + st.cursor.set_char_range(Some(egui::text::CCursorRange::two(a, b))); st.store(&ctx, edit_id); ctx.memory_mut(|m| m.request_focus(edit_id)); } @@ -2884,6 +2915,15 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if ui.input(|i| i.modifiers.alt) { if resp.clicked() { actions.push(("prop.setExpression".into(), json!({"layer": layer.id.0, "prop": uid}))); + if prop.expr.is_none() { + // Adding one: its editor opens, comes into view and takes the + // keyboard with the new expression selected (#1072). + app.ui.timeline.expr_closed.remove(uid); + focus_expression(&ctx, *uid, [usize::MAX, 0]); + } else if ctx.data(|d| d.get_temp::(expr_focus_id())).is_some_and(|f| f.prop == *uid) { + // Removing it: nothing is left to take the keyboard. + ctx.data_mut(|d| d.remove::(expr_focus_id())); + } } } else if let Some((value, merge)) = switch_gesture(ui, &resp, r, "stopwatch", prop.is_animated()) { actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid, "value": value, "merge": merge}))); @@ -3208,7 +3248,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { params["expression"] = json!(text); params["range"] = json!([a, b]); let after = text.chars().count().saturating_sub(b); - ctx.data_mut(|d| d.insert_temp(expr_refocus_id(src_prop), after)); + focus_expression(&ctx, src_prop, [after, after]); } actions.push(("prop.pickWhip".into(), params)); } diff --git a/crates/ui-egui/tests/it/ui_inline_edit.rs b/crates/ui-egui/tests/it/ui_inline_edit.rs index d7e532d76..c9be6e73b 100644 --- a/crates/ui-egui/tests/it/ui_inline_edit.rs +++ b/crates/ui-egui/tests/it/ui_inline_edit.rs @@ -540,6 +540,98 @@ fn arrow_keys_move_the_caret_in_rename_fields() { assert_eq!(layer_name, "Pla-te"); } +/// A solid named `name` with its Transform group open in the Timeline; returns the layer, its +/// Opacity uid and its Rotation uid. +fn open_transform(h: &mut Harness<'_, EffectcraftApp>, name: &str) -> (u64, u64, u64) { + let (layer, opacity, rotation, transform) = { + let s = &mut h.state_mut().session; + let l = s.execute("layer.newSolid", json!({"name": name, "color": "#406080"})).unwrap()["layer"].as_u64().unwrap(); + let tr = s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.sub("transform").unwrap().clone(); + (l, tr.get("opacity").unwrap().uid, tr.get("rotation").unwrap().uid, tr.uid) + }; + h.state_mut().ui.timeline.open_layers.insert(layer); + h.state_mut().ui.timeline.open_groups.insert(transform); + h.run_steps(3); + (layer, opacity, rotation) +} + +fn expression(h: &Harness<'_, EffectcraftApp>, layer: u64, prop: u64) -> Option { + let comp = h.state().session.active_comp().unwrap(); + comp.layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(prop).unwrap().expr.as_ref().map(|e| e.text.clone()) +} + +fn commit_expression(h: &mut Harness<'_, EffectcraftApp>) { + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::COMMAND)); + h.input_mut().events.push(Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::COMMAND }); + h.step(); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); + h.run_steps(3); +} + +/// #1072: Alt-clicking a stopwatch adds an expression and its editor takes the keyboard at once, +/// the new expression selected, so typing replaces it (it took another click). That happens +/// once: a field clicked next keeps the keyboard, and removing the expression with another +/// Alt-click leaves nothing to take it. +#[test] +fn alt_clicking_a_stopwatch_focuses_the_new_expression() { + let (mut h, _) = harness(); + let (layer, opacity, rotation) = open_transform(&mut h, "Plate"); + let alt = Modifiers { alt: true, ..Modifiers::NONE }; + let stopwatch = |h: &Harness<'_, EffectcraftApp>, prop: u64| center(h, &format!("timeline.prop.{prop}.stopwatch")); + let at = stopwatch(&h, opacity); + click_with(&mut h, at, alt); + assert!(expression(&h, layer, opacity).is_some(), "an expression was added"); + type_text(&mut h, "50"); + commit_expression(&mut h); + assert_eq!(expression(&h, layer, opacity).as_deref(), Some("50"), "typing replaced the new expression"); + // A field clicked right after an Alt-click keeps the keyboard. + let at = stopwatch(&h, rotation); + click_with(&mut h, at, alt); + let added = expression(&h, layer, rotation).expect("an expression on Rotation"); + let p = center(&h, "timeline.timecode"); + click(&mut h, p, 2); + type_text(&mut h, "0:00:01:00"); + key(&mut h, Key::Enter); + let frame = h.state().session.active_comp().unwrap().frame_rate.frame_at(h.state().session.time()); + assert_eq!((frame, expression(&h, layer, rotation)), (24, Some(added)), "the time took the typing, not the expression"); + // Alt-click again removes the expression; nothing takes the keyboard after it. + let at = stopwatch(&h, opacity); + click_with(&mut h, at, alt); + h.run_steps(5); + assert_eq!(expression(&h, layer, opacity), None); + assert!(!h.ctx.egui_wants_keyboard_input(), "nothing has the keyboard"); +} + +/// #1072: an expression added at the bottom of the Timeline brings its editor into view to take +/// the keyboard (its row is added below the last one shown). +#[test] +fn an_expression_added_at_the_bottom_of_the_timeline_comes_into_view() { + let (mut h, _) = harness(); + let (layer, opacity, _) = open_transform(&mut h, "Bottom"); + for i in 0..40 { + h.state_mut().session.execute("layer.newSolid", json!({"name": format!("Layer {i}"), "color": "#406080"})).unwrap(); + } + h.run_steps(3); + // Scroll to the end (the layer is at the bottom of the stack), then until Opacity is the + // last row shown. + h.state_mut().ui.timeline.scroll_y = 1.0e6; + h.run_steps(3); + let rows = h.state().auto.find("timeline.scroll").unwrap().rect; + let bottom = rows[1] + rows[3]; + let row = h.state().auto.find(&format!("timeline.prop.{opacity}.row")).unwrap().rect; + h.state_mut().ui.timeline.scroll_y += row[1] + row[3] - bottom; + h.run_steps(3); + let row = h.state().auto.find(&format!("timeline.prop.{opacity}.row")).unwrap().rect; + assert!((row[1] + row[3] - bottom).abs() < 1.0, "Opacity is at the bottom: {row:?}"); + let at = center(&h, &format!("timeline.prop.{opacity}.stopwatch")); + click_with(&mut h, at, Modifiers { alt: true, ..Modifiers::NONE }); + let editor = h.state().auto.find(&format!("timeline.prop.{opacity}.expression")).expect("the editor is drawn").rect; + assert!(editor[1] >= rows[1] - 0.5 && editor[1] + editor[3] <= bottom + 0.5, "the editor is in view: {editor:?} in {rows:?}"); + type_text(&mut h, "25"); + commit_expression(&mut h); + assert_eq!(expression(&h, layer, opacity).as_deref(), Some("25")); +} + /// #1434: with several 3D layers selected, typing a Z Position sets Z on each of them; their X /// and Y stay their own. Every selected layer used to take the typed layer's whole Position, so /// they stacked on top of it and seemed to disappear.