Merge pull request #3249 from vangel76/stopwatch-expression-alt-3192

Timeline: Add Expression from the stopwatch's right-click menu

crates/ui-egui/src/panels/timeline.rs

@@ -4111,6 +4111,7 @@ fn prop_key_navigator(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, r
 /// The stopwatch.
 fn prop_stopwatch(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: &RowCtx<'_>, prop: &Property) {
     let t = g.tl.t;
+    let cid = g.tl.cid;
     let ctx = g.tl.ctx.clone();
     let actions = &mut g.tl.actions;
     let lp = g.lp.clone();
@@ -4120,8 +4121,9 @@ fn prop_stopwatch(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: &
     let indent = rc.indent;
     let uid = &prop.uid;
 
-    // Stopwatch: Alt+click adds an expression; a press toggles the animation, and
-    // dragging on over other stopwatches gives them the same state (#284).
+    // Stopwatch: Alt+click (or Add Expression in its right-click menu) adds an expression; a
+    // press toggles the animation, and dragging on over other stopwatches gives them the same
+    // state (#284).
     if !prop.static_only {
         let swr = Rect::from_center_size(pos2(indent, cy), vec2(15.0, 15.0));
         let resp = ui.interact(swr, egui::Id::unique(("stopwatch", uid)), Sense::click_and_drag());
@@ -4134,18 +4136,13 @@ fn prop_stopwatch(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: &
         };
         icons::paint(&lp, swr, Icon::Stopwatch, col);
         app.auto.add(&format!("timeline.prop.{uid}.stopwatch"), swr, &prop.name);
+        // Right-click: the property's menu, whose Add Expression needs no Alt, which Linux
+        // window managers take for moving the window (#3192).
+        prop_context_menu(app, &ctx, &resp, cid, layer, prop, &mut *actions);
         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::<ExprFocus>(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::<ExprFocus>(expr_focus_id()));
-                }
+                expression_toggled(app, &ctx, prop);
             }
         } 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})));
@@ -4189,10 +4186,41 @@ fn prop_name(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: &RowCt
     if name_resp.clicked() {
         actions.push(("prop.select".into(), json!({"layer": layer.id.0, "prop": uid, "add": ui.input(|i| i.modifiers.shift)})));
     }
-    crate::widgets::context_menu(&name_resp, |ui| prop_menu(ui, &mut app.auto, cid, layer, prop, &mut *actions));
+    prop_context_menu(app, &ctx, &name_resp, cid, layer, prop, &mut *actions);
     name_x
 }
 
+/// `prop`'s context menu ([`prop_menu`]) on `resp`, its name or its stopwatch. Its Add or
+/// Remove Expression does what an Alt-click on the stopwatch does to the editor.
+fn prop_context_menu(
+    app: &mut EffectcraftApp,
+    ctx: &egui::Context,
+    resp: &egui::Response,
+    cid: ItemId,
+    layer: &Layer,
+    prop: &Property,
+    actions: &mut Vec<(String, serde_json::Value)>,
+) {
+    let before = actions.len();
+    crate::widgets::context_menu(resp, |ui| prop_menu(ui, &mut app.auto, cid, layer, prop, &mut *actions));
+    if actions.get(before..).is_some_and(|picked| picked.iter().any(|(id, _)| id == "prop.setExpression")) {
+        expression_toggled(app, ctx, prop);
+    }
+}
+
+/// An expression was just added to `prop` (it had none) or removed: adding one opens its
+/// editor, which comes into view and takes the keyboard with the new expression selected
+/// (#1072); removing it leaves nothing to take the keyboard.
+fn expression_toggled(app: &mut EffectcraftApp, ctx: &egui::Context, prop: &Property) {
+    let uid = prop.uid;
+    if prop.expr.is_none() {
+        app.ui.timeline.expr_closed.remove(&uid);
+        focus_expression(ctx, uid, [usize::MAX, 0]);
+    } else if ctx.data(|d| d.get_temp::<ExprFocus>(expr_focus_id())).is_some_and(|f| f.prop == uid) {
+        ctx.data_mut(|d| d.remove::<ExprFocus>(expr_focus_id()));
+    }
+}
+
 /// The value editors and the property pick whip.
 fn prop_value_cell(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: &RowCtx<'_>, ectx: &EvalCtx<'_>, prop: &Property, name_x: f32) {
     let vis = g.tl.vis;
@@ -5480,7 +5508,8 @@ pub(crate) fn is_position(layer: &Layer, uid: u64) -> bool {
     layer.transform().is_some_and(|tr| ["position", "positionX", "positionY", "positionZ"].iter().any(|m| tr.get(m).is_some_and(|p| p.uid == uid)))
 }
 
-/// A property's context menu (right-click its name in the Timeline), as in After Effects.
+/// A property's context menu: right-click its name in the Timeline, as in After Effects, or its
+/// stopwatch (an Add Expression that needs no Alt-click, #3192).
 fn prop_menu(
     ui: &mut egui::Ui,
     auto: &mut crate::automation::Registry,

crates/ui-egui/tests/it/ui_inline_edit.rs

@@ -637,6 +637,50 @@ fn an_expression_added_at_the_bottom_of_the_timeline_comes_into_view() {
     assert_eq!(expression(&h, layer, opacity).as_deref(), Some("25"));
 }
 
+fn right_click(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
+    h.input_mut().events.push(Event::PointerMoved(p));
+    h.step();
+    h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Secondary, pressed: true, modifiers: Modifiers::NONE });
+    h.step();
+    h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Secondary, pressed: false, modifiers: Modifiers::NONE });
+    h.run_steps(3);
+}
+
+/// Right-click Opacity's `target` (its stopwatch or name) in the Timeline, pick Add Expression
+/// and type `text` without another click: the new expression's editor has the keyboard.
+fn add_expression_from_menu(target: &str, text: &str) {
+    let (mut h, _) = harness();
+    let (layer, opacity, _) = open_transform(&mut h, "Plate");
+    let at = center(&h, &format!("timeline.prop.{opacity}.{target}"));
+    right_click(&mut h, at);
+    let item = format!("timeline.prop.{opacity}.menu.addExpression");
+    assert!(h.state().auto.find(&item).is_some(), "the {target}'s menu offers Add Expression");
+    let animated =
+        h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(opacity).unwrap().is_animated();
+    assert!(!animated, "the right-click didn't toggle the stopwatch");
+    let at = center(&h, &item);
+    click(&mut h, at, 1);
+    assert!(expression(&h, layer, opacity).is_some(), "an expression was added from the {target}'s menu");
+    type_text(&mut h, text);
+    commit_expression(&mut h);
+    assert_eq!(expression(&h, layer, opacity).as_deref(), Some(text), "typing replaced the expression added from the {target}'s menu");
+}
+
+/// #3192: Linux window managers take Alt+click to move the window, so the stopwatch has a
+/// right-click menu whose Add Expression does what the Alt-click does: the expression is added
+/// and its editor takes the keyboard (the stopwatch itself isn't toggled).
+#[test]
+fn add_expression_from_the_stopwatch_menu_focuses_the_new_expression() {
+    add_expression_from_menu("stopwatch", "50");
+}
+
+/// #3192: Add Expression from the property name's menu opens the editor with the keyboard too,
+/// as the Alt-click does (it took another click).
+#[test]
+fn add_expression_from_the_name_menu_focuses_the_new_expression() {
+    add_expression_from_menu("name", "90");
+}
+
 /// A primary-button drag from `from` to `to` with `modifiers` held from the press to the release.
 fn drag_with(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2, modifiers: Modifiers) {
     h.input_mut().events.push(Event::PointerMoved(from));