Merge pull request #1472 from arnav-hiwarkar/fix/1456-properties-reveal

fix(properties): reveal transform rows on double-click

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

@@ -244,6 +244,17 @@ fn transform_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter,
     }
     p.text(pos2(r.min.x + 42.0, cy), Align2::LEFT_CENTER, &pr.name, Tokens::ui(12.0), t.text);
     let vx = r.min.x + (r.width() * 0.5).max(130.0);
+    let label = Rect::from_min_max(pos2(r.min.x + 42.0, r.min.y), pos2(vx - 20.0, r.max.y));
+    let resp = ui.interact(label, egui::Id::new(("props-label", uid)), Sense::click());
+    app.auto.add(&format!("{base}.label"), label, &pr.name);
+    if resp.double_clicked() {
+        app.ui.timeline.search.clear();
+        app.ui.timeline.scroll_to = Some(crate::state::ScrollTo::Property(uid));
+        if let Err(e) = crate::menus::frontend(app, ui.ctx(), "timeline.revealProps", json!({"props": [{"layer": lid, "prop": uid}]})) {
+            app.ui.status = e;
+        }
+        app.show_panel(PanelKind::Timeline);
+    }
     let merge = format!("props-{uid}");
     // With several layers selected the edit applies to all: a scrub by the same change, a typed
     // value as it is.

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

@@ -1895,6 +1895,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
         if let Some(i) = rows.iter().position(|r| matches!(r.kind, RowKind::Layer) && Some(r.layer) == top) {
             app.ui.timeline.scroll_y = rows.iter().take(i).map(|r| row_height(r, rh)).sum();
         }
+    } else if let Some(crate::state::ScrollTo::Property(uid)) = scroll_to
+        && let Some(i) = rows.iter().position(|r| matches!(r.kind, RowKind::Prop { uid: p } if p == uid))
+    {
+        app.ui.timeline.scroll_y = rows.iter().take(i).map(|r| row_height(r, rh)).sum();
     }
     if ui.rect_contains_pointer(rows_rect) {
         let (dy, dx, zoom) = ui.input(|i| (i.smooth_scroll_delta.y, i.smooth_scroll_delta.x, i.modifiers.alt));

crates/ui-egui/src/state.rs

@@ -335,13 +335,15 @@ impl TimelineState {
     }
 }
 
-/// Where a Timeline shortcut scrolls to.
+/// Where the Timeline scrolls to.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum ScrollTo {
     /// D: the current time, centred in the time graph.
     Time,
     /// X: the topmost selected layer, at the top of the layers.
     Layer,
+    /// A revealed property, at the top of the rows.
+    Property(u64),
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -419,7 +421,7 @@ pub struct TimelineState {
     /// The name column shows Source Name instead of Layer Name (click its header).
     #[serde(default)]
     pub source_name: bool,
-    /// A scroll asked for by a shortcut (D, X), done when the Timeline is next drawn.
+    /// A pending scroll, applied when the Timeline is next drawn.
     #[serde(skip)]
     pub scroll_to: Option<ScrollTo>,
 }

crates/ui-egui/tests/ui_panels.rs

@@ -125,6 +125,15 @@ fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
     h.run_steps(2);
 }
 
+fn double_click_at(h: &mut Harness<'_, EffectcraftApp>, pos: Pos2) {
+    h.input_mut().events.push(Event::PointerMoved(pos));
+    h.step();
+    for pressed in [true, false, true, false] {
+        h.input_mut().events.push(Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() });
+    }
+    h.run_steps(3);
+}
+
 fn hover(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) {
     h.input_mut().events.push(Event::PointerMoved(p));
     h.run_steps(3);
@@ -506,6 +515,83 @@ fn properties_scale_link_shares_the_timeline_state() {
     assert!(!h.state().ui.timeline.unlinked.contains(&uid), "re-linking clears the shared state");
 }
 
+#[test]
+fn properties_label_double_click_reveals_without_editing() {
+    let mut h = harness();
+    let layer = h.state().session.active_comp().unwrap().layers[0].id;
+    let other = h.state_mut().session.execute("layer.newSolid", json!({"name": "Other", "color": "#204060"})).unwrap()["layer"].as_u64().unwrap();
+    h.state_mut().session.execute("prop.addKey", json!({"layer": layer.0, "path": "transform/opacity", "time": 1.0, "value": 50.0})).unwrap();
+    h.state_mut().session.execute("layer.select", json!({"layers": [layer.0]})).unwrap();
+    let comp = h.state().session.active_comp().unwrap();
+    let opacity = comp.layer(layer).unwrap().props.prop("transform/opacity").unwrap().uid;
+    let other_scale = comp.layers.iter().find(|l| l.id.0 == other).unwrap().props.prop("transform/scale").unwrap().uid;
+    let ctx = h.ctx.clone();
+    effectcraft_ui_egui::menus::frontend(h.state_mut(), &ctx, "timeline.revealProps", json!({"props": [{"layer": other, "prop": other_scale}]})).unwrap();
+    open(&mut h, "properties");
+    open(&mut h, "renderQueue");
+    h.state_mut().ui.timeline.search = "no matching property".into();
+    h.run_steps(3);
+    let value = h.state().auto.find(&format!("properties.prop.{opacity}.value")).unwrap().rect;
+    let label = pos2(value[0] - 60.0, value[1] + value[3] / 2.0);
+    let row = format!("timeline.prop.{opacity}.row");
+    click_at(&mut h, label);
+    assert_eq!(h.state().ui.timeline.search, "no matching property");
+    assert!(h.state().auto.find(&row).is_none());
+    h.run_steps(30);
+    let before = serde_json::to_value(h.state().session.project.as_ref()).unwrap();
+    let undo = h.state().session.history.undo.len();
+    let selected = h.state().session.state.selected_layers.clone();
+    for _ in 0..2 {
+        double_click_at(&mut h, label);
+        assert!(h.state().auto.find(&row).is_some(), "Opacity is visible for keyframing");
+        assert!(h.state().auto.find(&format!("timeline.prop.{other_scale}.row")).is_some(), "the other layer keeps its reveal");
+        assert_eq!(h.state().ui.timeline.search, "");
+        h.run_steps(30);
+    }
+    assert_eq!(serde_json::to_value(h.state().session.project.as_ref()).unwrap(), before, "revealing preserves values and keyframes");
+    assert_eq!(h.state().session.history.undo.len(), undo);
+    assert_eq!(h.state().session.state.selected_layers, selected);
+}
+
+#[test]
+fn properties_reveal_scrolls_to_the_displayed_layer_in_a_multiple_selection() {
+    let mut h = harness();
+    let layer = h.state().session.active_comp().unwrap().layers[0].id;
+    for _ in 0..40 {
+        h.state_mut().session.execute("layer.newSolid", json!({"name": "Spacer", "color": "#204060"})).unwrap();
+    }
+    let top = h.state().session.active_comp().unwrap().layers[0].id;
+    h.state_mut().session.execute("layer.select", json!({"layers": [layer.0, top.0]})).unwrap();
+    let uid = h.state().session.active_comp().unwrap().layer(layer).unwrap().props.prop("transform/opacity").unwrap().uid;
+    open(&mut h, "properties");
+    h.state_mut().toggle_maximize(PanelKind::Properties);
+    h.run_steps(3);
+    let r = h.state().auto.find(&format!("properties.prop.{uid}.label")).unwrap().rect;
+    double_click_at(&mut h, pos2(r[0] + r[2] / 2.0, r[1] + r[3] / 2.0));
+    assert!(h.state().auto.find(&format!("timeline.prop.{uid}.row")).is_some(), "the displayed layer's property is in view");
+    assert_eq!(h.state().session.state.selected_layers, [layer, top]);
+}
+
+#[test]
+fn properties_transform_labels_reveal_the_corresponding_property() {
+    let mut h = harness();
+    let layer = h.state().session.active_comp().unwrap().layers[0].id;
+    h.state_mut().session.execute("layer.select", json!({"layers": [layer.0]})).unwrap();
+    let props: Vec<u64> =
+        h.state().session.active_comp().unwrap().layer(layer).unwrap().transform().unwrap().props().filter(|p| !p.three_d_only).map(|p| p.uid).collect();
+    open(&mut h, "properties");
+    for uid in &props {
+        h.state_mut().ui.timeline.open_layers.remove(&layer.0);
+        h.run_steps(30);
+        assert!(h.state().auto.find(&format!("timeline.prop.{uid}.row")).is_none());
+        let r = h.state().auto.find(&format!("properties.prop.{uid}.label")).unwrap().rect;
+        double_click_at(&mut h, pos2(r[0] + r[2] / 2.0, r[1] + r[3] / 2.0));
+        for candidate in &props {
+            assert_eq!(h.state().auto.find(&format!("timeline.prop.{candidate}.row")).is_some(), candidate == uid);
+        }
+    }
+}
+
 /// Gap 7: the Backgrounds category is listed under * Animation Presets and a preset applies on double-click.
 #[test]
 fn background_presets_are_listed_and_apply() {