From d91a7b3122913167d1050a2fd4bf987a5388bf27 Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Tue, 13 Oct 2026 08:11:35 +1000 Subject: [PATCH] ui: keep Metadata and Info on the selected source item Co-authored-by: Codex --- crates/ui-egui/src/dock.rs | 10 +- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/panels/metadata.rs | 29 ++-- crates/ui-egui/src/panels/misc.rs | 8 +- crates/ui-egui/src/panels/panel_state.rs | 12 ++ crates/ui-egui/tests/panels_ui.rs | 173 +++++++++++++++++++++++ 6 files changed, 220 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index 48d29b55..d144bd57 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -860,12 +860,18 @@ pub fn draw_group_chrome_details(ui: &mut egui::Ui, g: &Group, seqs: &SeqTabs, t ui.ctx().data_mut(|d| d.remove::(list_id)); } } - // clicking anywhere in the panel focuses it + // Activating presses focus their tab immediately, rather than the group's old active panel. + // Otherwise entering an inspector can briefly reactivate a stale Project/Timeline selection. + // Middle-click closes a tab, so it keeps the active panel as the focus target. if let Some(p) = active_panel && ui.rect_contains_pointer(g.rect) && ui.input(|i| i.pointer.any_pressed()) { - actions.push(DockAction::Focus(p)); + let panel = ui + .input(|i| i.pointer.interact_pos().filter(|_| i.pointer.primary_pressed() || i.pointer.secondary_pressed())) + .and_then(|pos| tab_rects.iter().find(|(_, rect)| rect.contains(pos)).map(|(panel, _)| *panel)) + .unwrap_or(p); + actions.push(DockAction::Focus(panel)); } Chrome { actions, tabs: tab_rects } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9a6393b8..6b5cc488 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1693,6 +1693,7 @@ impl FilmcraftApp { } let ctx = ui.ctx().clone(); self.auto.begin_frame(); + panels::metadata::follow_selection(self); self.frames.set_context(&ctx); self.session.poll_persistence(); panels::trim_monitor::advance(self, &ctx); diff --git a/crates/ui-egui/src/panels/metadata.rs b/crates/ui-egui/src/panels/metadata.rs index c8c7f558..88312f1a 100644 --- a/crates/ui-egui/src/panels/metadata.rs +++ b/crates/ui-egui/src/panels/metadata.rs @@ -1,5 +1,5 @@ //! Metadata panel: the clip and file properties of the selected item (Project panel selection, or -//! the clip selected in the Timeline when it has focus). Name, Label and the log fields +//! the clip selected in the Timeline, retained while an inspector has focus). Name, Label and the log fields //! (Description, Scene, Shot, Log Note, Comment, Tape Name, Client, Camera Angle) are editable; //! each committed edit is one `metadata.set` (undoable, saved in the project). //! @@ -7,7 +7,7 @@ //! `metadata.label`, `metadata.section.`. use egui::{Rect, RichText, Sense, vec2}; -use filmcraft_engine::panels::{Field, metadata_fields, metadata_target}; +use filmcraft_engine::panels::{Field, metadata_fields}; use filmcraft_project::{ItemId, Label}; use serde_json::json; @@ -15,15 +15,28 @@ use crate::FilmcraftApp; use crate::dock::PanelKind; use crate::theme::Tokens; -/// The item the panel shows. +use super::panel_state::InspectorSource; + +/// Remember the selection source even when both item inspectors are hidden. +pub(crate) fn follow_selection(app: &mut FilmcraftApp) { + match app.ui.focused { + PanelKind::Project => app.ui.panels.inspector_source = InspectorSource::Project, + PanelKind::Timeline => app.ui.panels.inspector_source = InspectorSource::Timeline, + _ => {} + } +} + +/// The item Metadata and Info show. Keep the preferred source while entering either inspector; +/// an empty selection falls back to the other panel, as Metadata did before source retention. pub fn target(app: &FilmcraftApp) -> Option { let s = &app.session; - if app.ui.focused == PanelKind::Timeline - && let Some(c) = s.state.selection.first() - { - return metadata_target(s, &json!({"clip": c.0})); + let exists = |id: &ItemId| s.project.item(*id).is_some(); + let project = s.state.project_selection.first().copied().filter(exists); + let timeline = s.state.selection.first().and_then(|clip| s.active_sequence()?.find_item(*clip).map(|(_, item)| item.item)).filter(exists); + match app.ui.panels.inspector_source { + InspectorSource::Project => project.or(timeline), + InspectorSource::Timeline => timeline.or(project), } - metadata_target(s, &json!({})) } pub fn field_id(name: &str) -> String { diff --git a/crates/ui-egui/src/panels/misc.rs b/crates/ui-egui/src/panels/misc.rs index 744e9cfb..f5bc5f1e 100644 --- a/crates/ui-egui/src/panels/misc.rs +++ b/crates/ui-egui/src/panels/misc.rs @@ -167,8 +167,9 @@ pub fn info(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { ui.label(v); }); }; - if let Some(item) = app.session.state.project_selection.first().and_then(|i| app.session.project.item(*i)).cloned() { - ui.label(egui::RichText::new(&item.name).strong()); + if let Some(item) = super::metadata::target(app).and_then(|id| app.session.project.item(id)).cloned() { + let header = ui.label(egui::RichText::new(&item.name).strong()); + app.auto.add_response("info.item", &header, &item.name); line(ui, tl!("Type:"), crate::i18n::t(item.type_label()).to_string()); if let Some(m) = item.as_media() { if let Some(v) = &m.info.video { @@ -184,7 +185,8 @@ pub fn info(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { } if let Some(seq) = app.session.active_sequence() { let name = app.session.state.active_sequence.and_then(|s| app.session.project.item(s)).map(|i| i.name.clone()).unwrap_or_default(); - ui.label(egui::RichText::new(name).strong()); + let header = ui.label(egui::RichText::new(&name).strong()); + app.auto.add_response("info.sequence", &header, &name); line( ui, tl!("Settings:"), diff --git a/crates/ui-egui/src/panels/panel_state.rs b/crates/ui-egui/src/panels/panel_state.rs index 20f47e39..74f56ccd 100644 --- a/crates/ui-egui/src/panels/panel_state.rs +++ b/crates/ui-egui/src/panels/panel_state.rs @@ -14,6 +14,18 @@ pub struct PanelsState { pub reference: ReferenceState, pub events: EventsState, pub progress: ProgressState, + /// Preferred selection for Metadata and Info; inspector focus does not replace it. + pub inspector_source: InspectorSource, +} + +/// The last selection panel used by the item inspectors. Only the source is retained, so +/// project changes, deletion and deselection always resolve against the current selection. +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum InspectorSource { + #[default] + Project, + Timeline, } /// Lumetri Scopes settings (the wrench / right-click menu and the footer). diff --git a/crates/ui-egui/tests/panels_ui.rs b/crates/ui-egui/tests/panels_ui.rs index 504f4f3e..acc672a3 100644 --- a/crates/ui-egui/tests/panels_ui.rs +++ b/crates/ui-egui/tests/panels_ui.rs @@ -10,6 +10,7 @@ use std::sync::Arc; use std::sync::mpsc::{Sender, channel}; use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; use filmcraft_engine::Session; use filmcraft_ui_egui::FilmcraftApp; use filmcraft_ui_egui::control::ControlRequest; @@ -571,3 +572,175 @@ fn focused_panel_outline_is_not_covered_by_child_backgrounds() { } } } + +/// Select two different sources using the same Project-row and Timeline-clip clicks as an editor. +fn inspector_selection_driver() -> (Driver, filmcraft_project::ItemId, filmcraft_project::ItemId, filmcraft_project::ClipId) { + let mut d = Driver::demo(); + let seq = d.app().session.active_sequence().unwrap(); + let a = seq.video_tracks[0].items[0].item; + let clip = seq.video_tracks[0].items.iter().find(|clip| clip.item != a).unwrap(); + let (b, clip) = (clip.item, clip.id); + let bin = d + .app() + .session + .project + .root + .children + .iter() + .find_map(|entry| match entry { + filmcraft_project::BinEntry::Bin(bin) if bin.name == "Footage" => Some(bin.id.0), + _ => None, + }) + .unwrap(); + d.exec("project.view.set", json!({"view":"list"})); + d.ok("ui.menu.invoke", json!({"id":"window.panel.Metadata"})); + d.ok("ui.menu.invoke", json!({"id":"window.panel.Info"})); + d.click("panel.tab.Project"); + d.ok("ui.menu.invoke", json!({"id":"projectPanel.openBin","params":{"bin":bin,"how":"inPlace"}})); + d.click(&format!("project.item.{}", a.0)); + assert_eq!(d.app().session.state.project_selection, vec![a]); + d.click(&format!("timeline.clip.{}", clip.0)); + assert!(d.app().session.state.selection.contains(&clip)); + (d, a, b, clip) +} + +#[test] +fn metadata_keeps_the_timeline_item_when_its_tab_and_field_take_focus() { + let (mut d, a, b, _) = inspector_selection_driver(); + let b_name = d.app().session.project.item(b).unwrap().name.clone(); + assert_eq!(d.label("metadata.header"), format!("Clip: {b_name}")); + d.click("panel.tab.Metadata"); + assert_eq!(d.label("metadata.header"), format!("Clip: {b_name}"), "inspector focus must not reactivate the old Project selection"); + let before = serde_json::to_value(&d.app().session.project).unwrap(); + let undo = d.app().session.history.undo.len(); + d.click("metadata.field.Scene"); + d.ok("ui.type", json!({"text":"42B"})); + d.ok("ui.key", json!({"key":"Enter"})); + d.frames(3); + assert_eq!(d.app().session.project.item(b).unwrap().metadata.get("Scene").map(String::as_str), Some("42B")); + assert!(!d.app().session.project.item(a).unwrap().metadata.contains_key("Scene")); + assert_eq!(d.app().session.history.undo.len(), undo + 1); + let after = serde_json::to_value(&d.app().session.project).unwrap(); + d.exec("edit.undo", json!({})); + assert_eq!(serde_json::to_value(&d.app().session.project).unwrap(), before); + d.exec("edit.redo", json!({})); + assert_eq!(serde_json::to_value(&d.app().session.project).unwrap(), after); + d.snapshot("panels-metadata-timeline-selection", Some("metadata.")); +} + +#[test] +fn info_follows_the_timeline_item_and_keeps_sequence_information() { + let (mut d, _, b, _) = inspector_selection_driver(); + let name = d.app().session.project.item(b).unwrap().name.clone(); + d.click("panel.tab.Info"); + assert!(d.harness.query_by_label(&name).is_some(), "Info must show the selected Timeline item's name"); + assert_eq!(d.label("info.item"), name, "the item header belongs to Info, not another panel"); + assert!(!d.label("info.sequence").is_empty()); + assert!(d.harness.query_by_label("Settings:").is_some(), "Info keeps its lower sequence-information section"); + d.snapshot("panels-info-timeline-selection", None); +} + +#[test] +fn inspectors_follow_reselected_project_items_and_clear_without_stale_targets() { + let (mut d, a, b, clip) = inspector_selection_driver(); + let a_name = d.app().session.project.item(a).unwrap().name.clone(); + let b_name = d.app().session.project.item(b).unwrap().name.clone(); + let timeline_selection = d.app().session.state.selection.clone(); + let undo = d.app().session.history.undo.len(); + let playhead = d.app().session.playhead(); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), b_name); + // Selection ownership also follows actual source clicks while both inspectors are hidden. + d.ok("ui.menu.invoke", json!({"id":"window.panel.Source"})); + d.click("panel.tab.Project"); + d.click(&format!("project.item.{}", a.0)); + d.click(&format!("timeline.clip.{}", clip.0)); + d.ok("ui.menu.invoke", json!({"id":"window.panel.Metadata"})); + d.frames(3); + assert_eq!(d.label("metadata.header"), format!("Clip: {b_name}")); + d.click("panel.tab.Project"); + d.click(&format!("project.item.{}", a.0)); + d.click("panel.tab.Metadata"); + assert_eq!(d.label("metadata.header"), format!("Clip: {a_name}")); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), a_name); + assert_eq!(d.app().session.state.selection, timeline_selection); + assert_eq!(d.app().session.state.project_selection, vec![a]); + assert_eq!(d.app().session.history.undo.len(), undo, "inspector focus does not create edits"); + assert_eq!(d.app().session.playhead(), playhead); + + // Preserve Metadata's existing fallback in both directions when a selection is cleared. + d.click(&format!("timeline.clip.{}", clip.0)); + d.ok("ui.key", json!({"key":"Cmd+Shift+A"})); + assert!(d.app().session.state.selection.is_empty()); + d.click("panel.tab.Metadata"); + assert_eq!(d.label("metadata.header"), format!("Clip: {a_name}")); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), a_name); + d.click(&format!("timeline.clip.{}", clip.0)); + d.click("panel.tab.Project"); + d.ok("ui.key", json!({"key":"Cmd+Shift+A"})); + assert!(d.app().session.state.project_selection.is_empty()); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), b_name); + d.click("panel.tab.Metadata"); + assert_eq!(d.label("metadata.header"), format!("Clip: {b_name}")); + d.ok("ui.set", json!({"theme":"light"})); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), b_name, "theme and inspector changes keep the selection source"); + d.snapshot("panels-info-timeline-selection-light", Some("info.")); + + d.click(&format!("timeline.clip.{}", clip.0)); + d.ok("ui.key", json!({"key":"Cmd+Shift+A"})); + assert!(d.app().session.state.selection.is_empty()); + d.click("panel.tab.Metadata"); + assert!(d.has("metadata.empty")); + d.click("panel.tab.Info"); + assert!(!d.has("info.item")); + assert!(!d.label("info.sequence").is_empty(), "active sequence details remain with no clip selected"); + + d.click("panel.tab.Project"); + d.click(&format!("project.item.{}", a.0)); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), a_name); + d.exec("file.newProject", json!({"name":"Fresh inspector project"})); + d.frames(3); + assert!(!d.has("info.item"), "no item identity is retained from the old project"); + assert!(!d.has("info.sequence")); + d.click("panel.tab.Metadata"); + assert!(d.has("metadata.empty")); +} + +#[test] +fn inspector_source_defaults_when_loading_an_older_ui_state() { + let ui = filmcraft_ui_egui::state::UiState::default(); + let mut saved = serde_json::to_value(&ui).unwrap(); + saved["panels"].as_object_mut().unwrap().remove("inspectorSource"); + let restored: filmcraft_ui_egui::state::UiState = serde_json::from_value(saved).unwrap(); + assert_eq!(restored.panels.inspector_source, filmcraft_ui_egui::panels::panel_state::InspectorSource::Project); + assert_eq!(restored.panels.scopes, ui.panels.scopes); +} + +#[test] +fn middle_closing_an_inactive_project_tab_keeps_the_inspector_target() { + let (mut d, _, b, _) = inspector_selection_driver(); + let name = d.app().session.project.item(b).unwrap().name.clone(); + d.click("panel.tab.Info"); + assert_eq!(d.label("info.item"), name); + assert!(d.has("panel.tab.Project"), "the inactive Project tab must be visible before closing it"); + let project = serde_json::to_value(&d.app().session.project).unwrap(); + let state = d.exec("state.inspect", json!({})); + let history = d.exec("history.list", json!({})); + let revision = d.app().session.revision; + + d.ok("ui.click", json!({"id":"panel.tab.Project", "button":"middle"})); + d.frames(3); + assert!(!d.has("panel.tab.Project"), "middle-click closes the inactive tab"); + assert_eq!(serde_json::to_value(&d.app().session.project).unwrap(), project); + assert_eq!(d.exec("state.inspect", json!({})), state); + assert_eq!(d.exec("history.list", json!({})), history); + assert_eq!(d.app().session.revision, revision); + let focused = d.app().ui.focused; + let source = d.app().ui.panels.inspector_source; + assert_eq!(d.label("info.item"), name, "closing a tab must not select its stale item; focused={focused:?}, source={source:?}"); +}