diff --git a/crates/oak-app/src/actions.rs b/crates/oak-app/src/actions.rs index 3a3644ff6..dbba2c27c 100644 --- a/crates/oak-app/src/actions.rs +++ b/crates/oak-app/src/actions.rs @@ -38,9 +38,10 @@ //! up in `OakApp::dispatch_action_id`. use std::collections::HashMap; +use std::rc::Rc; use std::sync::{Mutex, OnceLock}; -use gpui::{Action, KeyBinding}; +use gpui::{Action, KeyBinding, KeyBindingContextPredicate}; // One macro call generates all three views of the registry, so they can // never drift apart: the gpui action structs, the `ActionId` enum and the @@ -326,18 +327,31 @@ pub fn entry_for_menu_id(id: usize) -> Option<&'static ActionEntry> { REGISTRY.iter().find(|entry| entry.action.menu_id() == id) } -/// The global key bindings for every action's *effective* key (context -/// `None`: the shell dispatches them wherever the focus is, and the modal -/// guard in the shell's action listeners suppresses them while a dialog is -/// open). An unbound action contributes nothing. +/// The global key bindings for every action's *effective* key. The shell +/// dispatches them wherever the focus is (the modal guard in the shell's +/// action listeners suppresses them while a dialog is open). An unbound +/// action contributes nothing. +/// +/// The multicam source-switch keys (`1..9` / `⌘1..⌘9`) are the exception: +/// they are scoped to the multicam panel's key context. Bound globally +/// they would swallow the digit keys everywhere — the matched action +/// bubbles through the focused panel and stops the keystroke before any +/// value box / hex field / text input can type `1..9`. pub fn key_bindings() -> Vec { let mut bindings = Vec::new(); for entry in REGISTRY { + // The stable C++ ids of the 18 source-switch actions all start + // with `multicamswitch` (1..9 and 1nosplit..9nosplit); nothing + // else shares the prefix. + let context = entry + .cpp_id + .starts_with("multicamswitch") + .then(|| Rc::new(KeyBindingContextPredicate::parse("MulticamPanel").unwrap())); for key in effective_keys(entry) { let binding = KeyBinding::load( &key, (entry.build)(), - None, + context.clone(), false, None, &gpui::DummyKeyboardMapper, @@ -870,6 +884,47 @@ mod tests { } } + /// The multicam source-switch hotkeys are panel-context: their + /// bindings carry the `MulticamPanel` key context so the digit keys + /// stay free for text entry outside the panel, and every other binding + /// stays global. (Regression: bound globally, `1..9` matched in every + /// focused panel and swallowed the digits before any value box / hex + /// field / text input could type them.) + #[test] + fn multicam_bindings_are_scoped_to_the_panel_context() { + let panel_context = Some(Rc::new( + KeyBindingContextPredicate::parse("MulticamPanel").unwrap(), + )); + let multicam_actions: &[&dyn Action] = &[ + &MulticamSwitch1, &MulticamSwitch2, &MulticamSwitch3, &MulticamSwitch4, + &MulticamSwitch5, &MulticamSwitch6, &MulticamSwitch7, &MulticamSwitch8, + &MulticamSwitch9, &MulticamSwitchNoSplit1, &MulticamSwitchNoSplit2, + &MulticamSwitchNoSplit3, &MulticamSwitchNoSplit4, &MulticamSwitchNoSplit5, + &MulticamSwitchNoSplit6, &MulticamSwitchNoSplit7, &MulticamSwitchNoSplit8, + &MulticamSwitchNoSplit9, + ]; + let mut scoped = 0usize; + for binding in key_bindings() { + let is_multicam = multicam_actions + .iter() + .any(|known| binding.action().partial_eq(*known)); + if is_multicam { + scoped += 1; + assert_eq!( + binding.predicate(), + panel_context, + "the source-switch binding must be gated on the MulticamPanel key context" + ); + } else { + assert!( + binding.predicate().is_none(), + "non-multicam bindings must stay global" + ); + } + } + assert_eq!(scoped, 18, "all 18 source-switch bindings are panel-scoped"); + } + /// Every registry action appears somewhere in the menu tree (an action /// without a menu entry is unreachable with the mouse, and the registry /// is meant to drive the menus). diff --git a/crates/oak-app/src/panels/multicam.rs b/crates/oak-app/src/panels/multicam.rs index 0f287daf8..7cb10782f 100644 --- a/crates/oak-app/src/panels/multicam.rs +++ b/crates/oak-app/src/panels/multicam.rs @@ -46,7 +46,7 @@ use gpui::colors::{Colors, DefaultColors}; use gpui::dock::{DockPanel, PanelEvent}; use gpui::{ canvas, div, img, prelude::*, px, AnyElement, App, Bounds, Context, Entity, EventEmitter, - MouseButton, ObjectFit, Pixels, Point, Rgba, Render, SharedString, Window, + FocusHandle, MouseButton, ObjectFit, Pixels, Point, Rgba, Render, SharedString, Window, }; use gpui_widgets::viewer::PlaybackClock; @@ -97,6 +97,12 @@ pub struct MulticamPanel { /// Set when a switch cleared the frames: the next refresh pass covers /// every source immediately. full_refresh: bool, + /// The panel's own focus handle: clicking the panel focuses it and puts + /// `MulticamPanel` on the key context path, which is what gates the + /// `1..9` / `⌘1..⌘9` source-switch bindings (see + /// [`crate::actions::key_bindings`]) — outside the panel those digit + /// keys stay free for text entry. + focus: FocusHandle, } impl MulticamPanel { @@ -117,6 +123,7 @@ impl MulticamPanel { refresh_cursor: 0, last_playhead: i64::MIN, full_refresh: true, + focus: _cx.focus_handle(), } } @@ -348,11 +355,17 @@ impl Render for MulticamPanel { .flex() .flex_col() .overflow_hidden() + // The panel owns the key context that gates the `1..9` / + // `⌘1..⌘9` source-switch bindings: focusing the panel (click) + // makes the keys switch angles, anywhere else they type digits. + .key_context("MulticamPanel") + .track_focus(&self.focus) // Any click inside the panel makes it the focused panel (the // dock re-emits this as `DockEvent::PanelFocused`, which the // shell uses to route the focused-panel hotkeys). .on_mouse_down(MouseButton::Left, { - cx.listener(|_this, _event: &gpui::MouseDownEvent, _window, cx| { + cx.listener(|this, _event: &gpui::MouseDownEvent, window, cx| { + window.focus(&this.focus, cx); cx.emit(PanelEvent::Focused); }) }) @@ -444,7 +457,10 @@ impl DockPanel for MulticamPanel { #[cfg(test)] mod tests { use super::*; - use gpui::{point, size, TestAppContext, VisualTestContext}; + use crate::actions::MulticamSwitch5; + use gpui::{point, size, Focusable, TestAppContext, VisualTestContext}; + use std::cell::Cell; + use std::rc::Rc; /// The grid conversion helpers used by the panel (rows/cols from the /// node, index round-trips) plus the panel's click math. @@ -522,4 +538,109 @@ mod tests { assert!(state.source_count >= 1, "demo sources: {}", state.source_count); assert_eq!(frame_count, state.source_count as usize, "every angle frame is cached"); } + + /// A minimal window root that mirrors the multicam panel's key-context + /// wiring: a `MulticamPanel`-scoped interactive region next to a text + /// field, with the source-switch action bound the way the shell binds + /// it (through the app keymap). + struct KeyContextHarness { + panel_focus: FocusHandle, + switched: Rc>, + editable: Entity, + } + + impl KeyContextHarness { + fn new(window: &mut Window, cx: &mut Context) -> Self { + let _ = window; + let editable = cx.new(|cx| { + gpui_elements::editable_text::EditableTextState::new( + gpui_elements::editable_text::StringStorage::from("0"), + cx, + ) + }); + Self { + panel_focus: cx.focus_handle(), + switched: Rc::new(Cell::new(false)), + editable, + } + } + } + + impl EventEmitter<()> for KeyContextHarness {} + + impl Render for KeyContextHarness { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let switched = self.switched.clone(); + div() + .size_full() + .flex() + .gap_4() + .child( + div() + .key_context("MulticamPanel") + .track_focus(&self.panel_focus) + .w_1_2() + .bg(gpui::rgb(0x202020)) + .on_action( + move |_: &MulticamSwitch5, _: &mut Window, _: &mut App| { + switched.set(true); + }, + ), + ) + .child( + crate::oakui::component::text_input("value-box", cx) + .state(self.editable.downgrade()), + ) + } + } + + /// The digit keys must type into a focused text field and only switch + /// sources while the multicam panel itself is focused. (Regression: + /// the `1..9` / `⌘1..⌘9` switch bindings were global, so a digit in + /// any value box / hex field / text input dispatched the switch action + /// instead of reaching the input handler — every field could only ever + /// type `0`.) + #[gpui::test] + async fn digit_keys_type_in_text_fields_but_switch_inside_the_panel( + cx: &mut TestAppContext, + ) { + cx.update(|cx| cx.init_colors()); + let window = cx.open_window(size(px(640.0), px(360.0)), |window, cx| { + cx.bind_keys(crate::actions::key_bindings()); + KeyContextHarness::new(window, cx) + }); + cx.run_until_parked(); + let harness = window.root(cx).expect("key-context harness root"); + let cx = VisualTestContext::from_window(window.into(), cx).into_mut(); + + // Focus the text field: `5` must type into it — no binding matches + // on a focus path that lacks the `MulticamPanel` context. (The + // field's caret starts at position 0, so the digit lands at the + // front of the initial "0".) + let field_focus = cx.read(|app| harness.read(app).editable.focus_handle(app)); + cx.update(|window, app| window.focus(&field_focus, app)); + cx.run_until_parked(); + cx.simulate_keystrokes("5"); + let text = cx.read(|app| harness.read(app).editable.read(app).as_str().to_string()); + assert_eq!(text, "50", "a digit reaches the focused text field"); + + // Focus the panel: the same key now dispatches the switch action + // and never reaches the (still registered) input handler. + let panel_focus = cx.read(|app| harness.read(app).panel_focus.clone()); + cx.update(|window, app| window.focus(&panel_focus, app)); + cx.run_until_parked(); + cx.simulate_keystrokes("5"); + let (switched, text) = cx.read(|app| { + let harness = harness.read(app); + (harness.switched.get(), harness.editable.read(app).as_str().to_string()) + }); + assert!( + switched, + "the panel-scoped binding fires while the panel is focused" + ); + assert_eq!( + text, "50", + "the same key no longer types into the field" + ); + } }