From c12cf38ce11d1eb2576f5dfe8de318136940803f Mon Sep 17 00:00:00 2001 From: Frantisek Bohac Date: Thu, 12 Nov 2026 18:25:44 +0100 Subject: [PATCH 1/4] Show native keyboard shortcuts in terminal action tooltips --- crates/okena-app/src/keybindings/mod.rs | 17 +- crates/okena-ui/src/header_buttons.rs | 15 +- crates/okena-ui/src/lib.rs | 1 + crates/okena-ui/src/shortcuts.rs | 267 ++++++++++++++++++ .../src/layout/tabs/mod.rs | 8 +- .../src/layout/terminal_pane/zoom.rs | 45 +-- 6 files changed, 307 insertions(+), 46 deletions(-) create mode 100644 crates/okena-ui/src/shortcuts.rs diff --git a/crates/okena-app/src/keybindings/mod.rs b/crates/okena-app/src/keybindings/mod.rs index 6811f39b1..340fe4ce2 100644 --- a/crates/okena-app/src/keybindings/mod.rs +++ b/crates/okena-app/src/keybindings/mod.rs @@ -501,22 +501,7 @@ fn create_keybinding(action: &str, keystroke: &str, context: Option<&str>) -> Op } } -/// Format a keystroke for display (convert to human-readable format) -pub fn format_keystroke(keystroke: &str) -> String { - keystroke - .replace("cmd", "⌘") - .replace("ctrl", "Ctrl") - .replace("alt", "Alt") - .replace("shift", "⇧") - .replace("-", "+") - .replace("pageup", "PgUp") - .replace("pagedown", "PgDn") - .replace("escape", "Esc") - .replace("left", "←") - .replace("right", "→") - .replace("up", "↑") - .replace("down", "↓") -} +pub use okena_ui::shortcuts::format_keystroke; /// The active shortcut for an action, formatted for the current platform. pub fn shortcut_for_action(action: &str) -> Option { diff --git a/crates/okena-ui/src/header_buttons.rs b/crates/okena-ui/src/header_buttons.rs index 7e13a7ee3..2878c53d9 100644 --- a/crates/okena-ui/src/header_buttons.rs +++ b/crates/okena-ui/src/header_buttons.rs @@ -5,7 +5,6 @@ use crate::theme::ThemeColors; use gpui::*; -use gpui_component::tooltip::Tooltip; /// All available header actions for terminal panes. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -127,12 +126,14 @@ pub fn header_button_base( .size(px(size.icon)) .text_color(rgb(t.text_secondary)), ) - .tooltip(move |_window, cx| { - let mut tooltip = Tooltip::new(tooltip_text); - if let Some(ref action) = gpui_action { - tooltip = tooltip.action(action.as_ref(), None); - } - tooltip.build(_window, cx) + .tooltip(move |window, cx| { + crate::shortcuts::action_tooltip( + tooltip_text, + gpui_action.as_deref(), + Some("TerminalPane"), + window, + cx, + ) }); // Apply hover style - red for close, normal for others diff --git a/crates/okena-ui/src/lib.rs b/crates/okena-ui/src/lib.rs index 471e1fb08..400de3443 100644 --- a/crates/okena-ui/src/lib.rs +++ b/crates/okena-ui/src/lib.rs @@ -36,6 +36,7 @@ pub mod resize_handle; pub mod scrollbar; pub mod selectable_list; pub mod settings; +pub mod shortcuts; pub mod simple_input; pub mod submenu; pub mod text_utils; diff --git a/crates/okena-ui/src/shortcuts.rs b/crates/okena-ui/src/shortcuts.rs new file mode 100644 index 000000000..e130bb44d --- /dev/null +++ b/crates/okena-ui/src/shortcuts.rs @@ -0,0 +1,267 @@ +//! Shared shortcut labels for tooltips, keybinding editors and command lists. +use gpui::{Action, AnyView, App, AsKeystroke, KeyBinding, KeyContext, Keystroke, Window}; +use gpui_component::tooltip::Tooltip; + +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum ShortcutPlatform { + Mac, + Windows, + Linux, +} + +impl ShortcutPlatform { + pub const fn current() -> Self { + if cfg!(target_os = "macos") { + Self::Mac + } else if cfg!(target_os = "windows") { + Self::Windows + } else { + Self::Linux + } + } +} + +/// Formats every stroke, preserving punctuation keys such as `ctrl--`. +/// Invalid/in-progress editor input is left intact. +pub fn format_keystroke(source: &str) -> String { + format_keystroke_for_platform(source, ShortcutPlatform::current()) +} + +pub fn format_keystroke_for_platform(source: &str, platform: ShortcutPlatform) -> String { + source + .split_whitespace() + .map(|source| { + Keystroke::parse(source) + .map(|key| format_stroke(&key, platform)) + .unwrap_or_else(|_| source.to_owned()) + }) + .collect::>() + .join(" ") +} + +fn format_stroke(key: &Keystroke, platform: ShortcutPlatform) -> String { + let mac = platform == ShortcutPlatform::Mac; + let mut parts = Vec::new(); + if key.modifiers.control { + parts.push(if mac { "⌃" } else { "Ctrl" }); + } + if key.modifiers.alt { + parts.push(if mac { "⌥" } else { "Alt" }); + } + if key.modifiers.shift { + parts.push(if mac { "⇧" } else { "Shift" }); + } + if key.modifiers.platform { + parts.push(match platform { + ShortcutPlatform::Mac => "⌘", + ShortcutPlatform::Windows => "Win", + ShortcutPlatform::Linux => "Super", + }); + } + if key.modifiers.function { + parts.push("Fn"); + } + let name = match key.key.as_str() { + "escape" => if mac { "⎋" } else { "Esc" }.into(), + "enter" => if mac { "↩" } else { "Enter" }.into(), + "backspace" => if mac { "⌫" } else { "Backspace" }.into(), + "delete" => if mac { "⌦" } else { "Delete" }.into(), + "pageup" => "PgUp".into(), + "pagedown" => "PgDn".into(), + "left" => "←".into(), + "right" => "→".into(), + "up" => "↑".into(), + "down" => "↓".into(), + "space" => "Space".into(), + "tab" => "Tab".into(), + "home" => "Home".into(), + "end" => "End".into(), + name if name.len() == 1 || name.starts_with('f') && name[1..].parse::().is_ok() => { + name.to_uppercase() + } + name => { + let mut chars = name.chars(); + chars + .next() + .map(|first| first.to_uppercase().collect::() + chars.as_str()) + .unwrap_or_default() + } + }; + parts.push(&name); + parts.join(if mac { "" } else { "+" }) +} + +fn preferred_binding(bindings: &[KeyBinding], platform: ShortcutPlatform) -> Option<&KeyBinding> { + // Defaults contain both cmd and ctrl variants. Choose a usable native variant + // among effective bindings, keeping registration precedence within that set. + bindings + .iter() + .rev() + .find(|binding| { + binding.keystrokes().iter().all(|key| { + let modifiers = key.as_keystroke().modifiers; + platform == ShortcutPlatform::Mac || !modifiers.platform + }) && (platform != ShortcutPlatform::Mac + || binding + .keystrokes() + .iter() + .any(|key| key.as_keystroke().modifiers.platform)) + }) + .or_else(|| bindings.last()) +} + +/// Resolves the live keymap, including disabled/conflicting bindings, for a UI +/// action's context. No settings cache or hard-coded default shortcuts are used. +pub fn shortcut_for_action( + action: &dyn Action, + context: Option<&str>, + window: &Window, +) -> Option { + let bindings = match context { + Some(context) => { + window.bindings_for_action_in_context(action, KeyContext::parse(context).ok()?) + } + None => window.bindings_for_action(action), + }; + let platform = ShortcutPlatform::current(); + let binding = preferred_binding(&bindings, platform)?; + Some( + binding + .keystrokes() + .iter() + .map(|key| format_stroke(key.as_keystroke(), platform)) + .collect::>() + .join(" "), + ) +} + +pub fn action_tooltip( + label: &str, + action: Option<&dyn Action>, + context: Option<&str>, + window: &mut Window, + cx: &mut App, +) -> AnyView { + let text = match action.and_then(|action| shortcut_for_action(action, context, window)) { + Some(shortcut) => format!("{label} ({shortcut})"), + None => label.to_owned(), + }; + Tooltip::new(text).build(window, cx) +} + +#[cfg(test)] +mod tests { + use super::*; + use gpui::{Context, IntoElement, Render, TestAppContext, div, px, size}; + gpui::actions!(shortcut_tests, [SampleAction, OtherAction]); + + struct TestView; + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + } + } + + #[gpui::test] + fn resolves_live_remapped_contextual_and_shadowed_bindings(cx: &mut TestAppContext) { + let window = cx.open_window(size(px(300.), px(200.)), |_, _| TestView); + cx.update(|cx| { + cx.bind_keys([ + KeyBinding::new("ctrl-d", SampleAction, Some("TerminalPane")), + KeyBinding::new("ctrl-p", SampleAction, Some("Sidebar")), + ]) + }); + window + .update(cx, |_, window, _| { + assert_eq!( + shortcut_for_action(&SampleAction, Some("TerminalPane"), window), + Some(format_keystroke("ctrl-d")) + ); + assert_eq!( + shortcut_for_action(&SampleAction, Some("Sidebar"), window), + Some(format_keystroke("ctrl-p")) + ); + assert_eq!( + shortcut_for_action(&SampleAction, Some("SearchBar"), window), + None + ); + }) + .unwrap(); + cx.update(|cx| { + cx.bind_keys([ + KeyBinding::new("ctrl-d", OtherAction, Some("TerminalPane")), + KeyBinding::new("ctrl-k ctrl-c", SampleAction, Some("TerminalPane")), + ]) + }); + window + .update(cx, |_, window, _| { + assert_eq!( + shortcut_for_action(&SampleAction, Some("TerminalPane"), window), + Some(format_keystroke("ctrl-k ctrl-c")) + ); + }) + .unwrap(); + cx.update(|cx| cx.clear_key_bindings()); + window + .update(cx, |_, window, _| { + assert_eq!( + shortcut_for_action(&SampleAction, Some("TerminalPane"), window), + None + ); + }) + .unwrap(); + } + + #[test] + fn prefers_native_effective_binding_and_preserves_custom_fallback() { + let bindings = [ + KeyBinding::new("cmd-d", SampleAction, None), + KeyBinding::new("ctrl-d", SampleAction, None), + ]; + assert_eq!( + preferred_binding(&bindings, ShortcutPlatform::Mac) + .unwrap() + .keystrokes(), + bindings[0].keystrokes() + ); + assert_eq!( + preferred_binding(&bindings, ShortcutPlatform::Windows) + .unwrap() + .keystrokes(), + bindings[1].keystrokes() + ); + assert_eq!( + preferred_binding(&bindings[1..], ShortcutPlatform::Mac) + .unwrap() + .keystrokes(), + bindings[1].keystrokes() + ); + } + #[test] + fn formats_platform_modifiers_chords_and_punctuation() { + assert_eq!( + format_keystroke_for_platform("cmd-shift-p", ShortcutPlatform::Mac), + "⇧⌘P" + ); + assert_eq!( + format_keystroke_for_platform("ctrl-shift-p", ShortcutPlatform::Windows), + "Ctrl+Shift+P" + ); + assert_eq!( + format_keystroke_for_platform("cmd-p", ShortcutPlatform::Windows), + "Win+P" + ); + assert_eq!( + format_keystroke_for_platform("cmd-p", ShortcutPlatform::Linux), + "Super+P" + ); + assert_eq!( + format_keystroke_for_platform("ctrl-k ctrl--", ShortcutPlatform::Windows), + "Ctrl+K Ctrl+-" + ); + assert_eq!( + format_keystroke_for_platform("cmd-k cmd-c", ShortcutPlatform::Mac), + "⌘K ⌘C" + ); + } +} diff --git a/crates/okena-views-terminal/src/layout/tabs/mod.rs b/crates/okena-views-terminal/src/layout/tabs/mod.rs index cddeee572..ba4354b85 100644 --- a/crates/okena-views-terminal/src/layout/tabs/mod.rs +++ b/crates/okena-views-terminal/src/layout/tabs/mod.rs @@ -116,7 +116,7 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - None, + Some(Box::new(crate::actions::SplitVertical)), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_split_v.action_dispatcher { @@ -138,7 +138,7 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - None, + Some(Box::new(crate::actions::SplitHorizontal)), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_split_h.action_dispatcher { @@ -160,7 +160,7 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - None, + Some(Box::new(crate::actions::AddTab)), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_add_tab.action_dispatcher { @@ -181,7 +181,7 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, Some(if standalone { "Close" } else { "Close Tab" }), - None, + Some(Box::new(crate::actions::CloseTerminal)), ) .on_click(move |_, _window, cx| { if let Some(ref tid) = terminal_id_for_close diff --git a/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs b/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs index 7016c8bed..c4e83c7c5 100644 --- a/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs +++ b/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs @@ -167,7 +167,7 @@ impl TerminalPane { size, &t, None, - None, + Some(Box::new(crate::actions::FullscreenPrevTerminal)), ) .on_click({ let workspace = workspace.clone(); @@ -211,7 +211,7 @@ impl TerminalPane { size, &t, None, - None, + Some(Box::new(crate::actions::FullscreenNextTerminal)), ) .on_click({ let workspace = workspace.clone(); @@ -246,24 +246,31 @@ impl TerminalPane { ) }) .child( - header_button_base(HeaderAction::ExitZoom, id_suffix, size, &t, None, None) - .on_click({ - let project_id = self.project_id.clone(); - let dispatcher = dispatcher.clone(); - move |_, _window, cx| { - cx.stop_propagation(); - if let Some(ref dispatcher) = dispatcher { - dispatcher.dispatch( - ActionRequest::SetFullscreen { - project_id: project_id.clone(), - terminal_id: None, - window: None, - }, - cx, - ); - } + header_button_base( + HeaderAction::ExitZoom, + id_suffix, + size, + &t, + None, + Some(Box::new(crate::actions::ToggleFullscreen)), + ) + .on_click({ + let project_id = self.project_id.clone(); + let dispatcher = dispatcher.clone(); + move |_, _window, cx| { + cx.stop_propagation(); + if let Some(ref dispatcher) = dispatcher { + dispatcher.dispatch( + ActionRequest::SetFullscreen { + project_id: project_id.clone(), + terminal_id: None, + window: None, + }, + cx, + ); } - }), + } + }), ), ) } From 16ed5e5cc55143505558ab4b868d70d9c108be21 Mon Sep 17 00:00:00 2001 From: Frantisek Bohac Date: Thu, 12 Nov 2026 20:00:11 +0100 Subject: [PATCH 2/4] Show assigned shortcuts across desktop action tooltips --- .../src/views/panels/project_column.rs | 54 ++- crates/okena-files/src/file_viewer/render.rs | 47 ++- crates/okena-ui/src/header_buttons.rs | 13 +- crates/okena-ui/src/shortcuts.rs | 342 ++++++++++++------ .../okena-views-git/src/diff_viewer/render.rs | 24 +- crates/okena-views-sidebar/src/folder_list.rs | 1 + .../okena-views-sidebar/src/item_widgets.rs | 22 +- .../okena-views-sidebar/src/project_list.rs | 77 +++- crates/okena-views-sidebar/src/sidebar/mod.rs | 20 +- .../src/layout/tabs/mod.rs | 50 ++- .../src/layout/terminal_pane/zoom.rs | 24 +- 11 files changed, 526 insertions(+), 148 deletions(-) diff --git a/crates/okena-app/src/views/panels/project_column.rs b/crates/okena-app/src/views/panels/project_column.rs index 8926d7ca7..d622b7f2e 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -892,7 +892,27 @@ impl ProjectColumn { .size(px(14.0)) .text_color(rgb(t.text_secondary)), ) - .tooltip(move |_window, cx| Tooltip::new(vis_tooltip).build(_window, cx)), + .tooltip({ + let focus_manager = self.focus_manager.clone(); + let project_id = self.project_id.clone(); + move |window, cx| { + let fm = focus_manager.read(cx); + let matches = fm + .focused_terminal_state() + .map(|state| state.project_id) + .or_else(|| fm.focused_project_id().cloned()) + .as_ref() + == Some(&project_id); + okena_ui::shortcuts::action_tooltip( + vis_tooltip, + matches.then_some( + &crate::keybindings::ToggleProjectVisibility as &dyn Action, + ), + window, + cx, + ) + } + }), ) .child( div() @@ -927,7 +947,37 @@ impl ProjectColumn { .size(px(14.0)) .text_color(rgb(t.text_secondary)), ) - .tooltip(move |_window, cx| Tooltip::new(focus_tooltip).build(_window, cx)), + .tooltip({ + let focus_manager = self.focus_manager.clone(); + let workspace = self.workspace.clone(); + let project_id = self.project_id.clone(); + let window_id = self.window_id; + move |window, cx| { + let fm = focus_manager.read(cx); + // The keyboard exit also clears the folder filter; + // the button does not. Show it only for an equal result. + let matches = if is_focused_view { + fm.focused_project_id() == Some(&project_id) + && workspace + .read(cx) + .active_folder_filter(window_id) + .is_none() + } else { + fm.focused_project_id().is_none() + && fm + .focused_terminal_state() + .is_some_and(|state| state.project_id == project_id) + }; + okena_ui::shortcuts::action_tooltip( + focus_tooltip, + matches.then_some( + &crate::keybindings::FocusActiveProject as &dyn Action, + ), + window, + cx, + ) + } + }), ) .into_any_element() }); diff --git a/crates/okena-files/src/file_viewer/render.rs b/crates/okena-files/src/file_viewer/render.rs index 15bfa7915..b79d200a7 100644 --- a/crates/okena-files/src/file_viewer/render.rs +++ b/crates/okena-files/src/file_viewer/render.rs @@ -217,6 +217,43 @@ fn loading_row(depth: usize, t: &ThemeColors, cx: &App) -> Div { } impl FileViewer { + /// Cancel first dismisses transient UI or clears selection. Only advertise it + /// for navigation when it reaches the same Back/Close event as the button. + fn cancel_navigates(&self, window: &Window) -> bool { + let tab = self.active_tab(); + self.focus_handle.is_focused(window) + && self.selection_context_menu.is_none() + && self.tab_context_menu.is_none() + && self.context_menu.is_none() + && self.rename_state.is_none() + && self.delete_confirm.is_none() + && self.search_state.is_none() + && (tab.display_mode != DisplayMode::Preview + || tab.markdown_selection.normalized_non_empty().is_none()) + && tab.selection.normalized_non_empty().is_none() + } + + fn navigation_tooltip( + &self, + label: &'static str, + back: bool, + cx: &Context, + ) -> impl Fn(&mut Window, &mut App) -> AnyView + use<> { + let viewer = cx.entity().downgrade(); + move |window, cx| { + let matches = viewer.upgrade().is_some_and(|viewer| { + let viewer = viewer.read(cx); + viewer.can_go_back == back && viewer.cancel_navigates(window) + }); + okena_ui::shortcuts::action_tooltip( + label, + matches.then_some(&Cancel as &dyn Action), + window, + cx, + ) + } + } + fn render_selectable_markdown_unit( &self, id: ElementId, @@ -1674,10 +1711,7 @@ impl Render for FileViewer { .bg(rgb(t.bg_hover)) .text_color(rgb(t.text_primary)) }) - .tooltip(|window, cx| { - gpui_component::tooltip::Tooltip::new("Back") - .build(window, cx) - }) + .tooltip(self.navigation_tooltip("Back", true, cx)) .on_click(cx.listener(|_this, _, _window, cx| { cx.emit(super::FileViewerEvent::Back); })) @@ -2068,10 +2102,7 @@ impl Render for FileViewer { .justify_center() .rounded(px(6.0)) .hover(|s| s.bg(rgb(t.bg_hover))) - .tooltip(|window, cx| { - gpui_component::tooltip::Tooltip::new("Close") - .build(window, cx) - }) + .tooltip(self.navigation_tooltip("Close", false, cx)) .on_click(cx.listener(|this, _, _window, cx| this.close(cx))) .child( svg() diff --git a/crates/okena-ui/src/header_buttons.rs b/crates/okena-ui/src/header_buttons.rs index 2878c53d9..587f3497b 100644 --- a/crates/okena-ui/src/header_buttons.rs +++ b/crates/okena-ui/src/header_buttons.rs @@ -103,7 +103,7 @@ pub fn header_button_base( size: ButtonSize, t: &ThemeColors, tooltip_override: Option<&'static str>, - gpui_action: Option>, + gpui_action: Option, ) -> Stateful
{ let tooltip_text = tooltip_override.unwrap_or_else(|| action.tooltip()); let bg_hover = t.bg_hover; @@ -126,14 +126,9 @@ pub fn header_button_base( .size(px(size.icon)) .text_color(rgb(t.text_secondary)), ) - .tooltip(move |window, cx| { - crate::shortcuts::action_tooltip( - tooltip_text, - gpui_action.as_deref(), - Some("TerminalPane"), - window, - cx, - ) + .tooltip(move |window, cx| match &gpui_action { + Some(shortcut) => shortcut.tooltip(tooltip_text, window, cx), + None => crate::shortcuts::action_tooltip(tooltip_text, None, window, cx), }); // Apply hover style - red for close, normal for others diff --git a/crates/okena-ui/src/shortcuts.rs b/crates/okena-ui/src/shortcuts.rs index e130bb44d..7e929d3cf 100644 --- a/crates/okena-ui/src/shortcuts.rs +++ b/crates/okena-ui/src/shortcuts.rs @@ -1,5 +1,5 @@ //! Shared shortcut labels for tooltips, keybinding editors and command lists. -use gpui::{Action, AnyView, App, AsKeystroke, KeyBinding, KeyContext, Keystroke, Window}; +use gpui::{Action, AnyView, App, AsKeystroke, KeyBinding, Keystroke, Window}; use gpui_component::tooltip::Tooltip; #[derive(Clone, Copy, PartialEq, Eq)] @@ -32,7 +32,13 @@ pub fn format_keystroke_for_platform(source: &str, platform: ShortcutPlatform) - .split_whitespace() .map(|source| { Keystroke::parse(source) - .map(|key| format_stroke(&key, platform)) + .map(|key| { + if key.key.is_empty() { + source.to_owned() + } else { + format_stroke(&key, platform) + } + }) .unwrap_or_else(|_| source.to_owned()) }) .collect::>() @@ -91,152 +97,282 @@ fn format_stroke(key: &Keystroke, platform: ShortcutPlatform) -> String { parts.join(if mac { "" } else { "+" }) } -fn preferred_binding(bindings: &[KeyBinding], platform: ShortcutPlatform) -> Option<&KeyBinding> { - // Defaults contain both cmd and ctrl variants. Choose a usable native variant - // among effective bindings, keeping registration precedence within that set. - bindings +/// Formats a binding with the same stroke formatter as the keybindings editor. +pub fn format_binding(binding: &KeyBinding) -> String { + let platform = ShortcutPlatform::current(); + binding + .keystrokes() .iter() - .rev() - .find(|binding| { - binding.keystrokes().iter().all(|key| { - let modifiers = key.as_keystroke().modifiers; - platform == ShortcutPlatform::Mac || !modifiers.platform - }) && (platform != ShortcutPlatform::Mac - || binding - .keystrokes() - .iter() - .any(|key| key.as_keystroke().modifiers.platform)) + .map(|key| format_stroke(key.as_keystroke(), platform)) + .collect::>() + .join(" ") +} + +/// Returns all effective bindings on the actual focused dispatch path, retaining +/// parameters, nested contexts and registration order for other UI consumers. +pub fn bindings_for_action(action: &dyn Action, window: &Window, cx: &App) -> Vec { + let contexts = window.context_stack(); + let keymap = cx.key_bindings(); + let keymap = keymap.borrow(); + // Use the same keymap input resolver as GPUI dispatch, with the actual path. + // A missing root context is an empty stack, not a synthetic empty context: + // negated predicates (e.g. !TerminalPane) distinguish those two cases. + keymap + .bindings_for_action(action) + .filter(|binding| { + keymap + .bindings_for_input(binding.keystrokes(), &contexts) + .0 + .first() + .is_some_and(|effective| effective.action().partial_eq(action)) }) - .or_else(|| bindings.last()) + .cloned() + .collect() } -/// Resolves the live keymap, including disabled/conflicting bindings, for a UI -/// action's context. No settings cache or hard-coded default shortcuts are used. -pub fn shortcut_for_action( - action: &dyn Action, - context: Option<&str>, - window: &Window, -) -> Option { - let bindings = match context { - Some(context) => { - window.bindings_for_action_in_context(action, KeyContext::parse(context).ok()?) - } - None => window.bindings_for_action(action), - }; - let platform = ShortcutPlatform::current(); - let binding = preferred_binding(&bindings, platform)?; - Some( - binding - .keystrokes() - .iter() - .map(|key| format_stroke(key.as_keystroke(), platform)) - .collect::>() - .join(" "), - ) +/// Chooses one display binding without removing alternatives. GPUI returns effective +/// bindings in registration order; the last one wins, on every platform. Modifiers +/// do not determine eligibility: custom Ctrl on macOS and Win/Super elsewhere work. +pub fn shortcut_for_action(action: &dyn Action, window: &Window, cx: &App) -> Option { + bindings_for_action(action, window, cx) + .last() + .map(format_binding) } pub fn action_tooltip( label: &str, action: Option<&dyn Action>, - context: Option<&str>, window: &mut Window, cx: &mut App, ) -> AnyView { - let text = match action.and_then(|action| shortcut_for_action(action, context, window)) { + let text = match action.and_then(|action| shortcut_for_action(action, window, cx)) { Some(shortcut) => format!("{label} ({shortcut})"), None => label.to_owned(), }; Tooltip::new(text).build(window, cx) } +/// Resolve an explicitly named application action without reversing crate dependencies. +/// Missing/unregistered actions keep their text-only tooltip. +pub fn named_action_tooltip( + label: &str, + action_name: &str, + window: &mut Window, + cx: &mut App, +) -> AnyView { + let action = cx.build_action(action_name, None).ok(); + action_tooltip(label, action.as_deref(), window, cx) +} + +/// A reusable button's shortcut and its live operation/target eligibility check. +/// The check runs when the tooltip opens, not when the button was last rendered. +pub struct ActionShortcut { + action: Box, + is_applicable: Box bool>, +} + +impl ActionShortcut { + pub fn new( + action: impl Action, + is_applicable: impl Fn(&Window, &App) -> bool + 'static, + ) -> Self { + Self { + action: Box::new(action), + is_applicable: Box::new(is_applicable), + } + } + + pub fn tooltip(&self, label: &str, window: &mut Window, cx: &mut App) -> AnyView { + let action = (self.is_applicable)(window, cx).then_some(self.action.as_ref()); + action_tooltip(label, action, window, cx) + } +} + #[cfg(test)] mod tests { use super::*; - use gpui::{Context, IntoElement, Render, TestAppContext, div, px, size}; + use gpui::{ + Context, FocusHandle, InteractiveElement, IntoElement, ParentElement, Render, + TestAppContext, div, + }; gpui::actions!(shortcut_tests, [SampleAction, OtherAction]); - struct TestView; + #[derive(Clone, Debug, PartialEq, gpui::Action)] + #[action(namespace = shortcut_tests, no_json)] + struct ParameterizedAction { + target: usize, + } + + struct TestView { + focus: FocusHandle, + other_focus: FocusHandle, + } impl Render for TestView { fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { div() + .key_context("Workspace") + .child( + div() + .key_context("TerminalPane") + .track_focus(&self.focus) + .child(gpui::deferred(div())), + ) + .child(div().key_context("Sidebar").track_focus(&self.other_focus)) } } #[gpui::test] - fn resolves_live_remapped_contextual_and_shadowed_bindings(cx: &mut TestAppContext) { - let window = cx.open_window(size(px(300.), px(200.)), |_, _| TestView); - cx.update(|cx| { + fn resolves_live_nested_contexts_remapping_removal_and_shadowing(cx: &mut TestAppContext) { + let (view, cx) = cx.add_window_view(|window, cx| { + let focus = cx.focus_handle(); + focus.focus(window, cx); + TestView { + focus, + other_focus: cx.focus_handle(), + } + }); + cx.update(|window, cx| { cx.bind_keys([ - KeyBinding::new("ctrl-d", SampleAction, Some("TerminalPane")), + KeyBinding::new("ctrl-d", SampleAction, Some("Workspace > TerminalPane")), KeyBinding::new("ctrl-p", SampleAction, Some("Sidebar")), - ]) + ]); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("ctrl-d")) + ); + cx.bind_keys([KeyBinding::new("ctrl-d", OtherAction, Some("TerminalPane"))]); + assert_eq!(shortcut_for_action(&SampleAction, window, cx), None); + cx.bind_keys([KeyBinding::new( + "ctrl-k ctrl--", + SampleAction, + Some("Workspace > TerminalPane"), + )]); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("ctrl-k ctrl--")) + ); + cx.bind_keys([KeyBinding::new( + "ctrl-k ctrl--", + gpui::NoAction, + Some("TerminalPane"), + )]); + assert_eq!(shortcut_for_action(&SampleAction, window, cx), None); + // Simulate a configuration reload, including removal of old assignments. + cx.clear_key_bindings(); + assert_eq!(shortcut_for_action(&SampleAction, window, cx), None); + cx.bind_keys([KeyBinding::new( + "cmd-j", + SampleAction, + Some("Workspace > TerminalPane"), + )]); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("cmd-j")) + ); + let other_focus = view.read(cx).other_focus.clone(); + other_focus.focus(window, cx); + assert_eq!(shortcut_for_action(&SampleAction, window, cx), None); + }); + } + + #[gpui::test] + fn preserves_alternatives_parameters_and_custom_platform_modifiers(cx: &mut TestAppContext) { + let (_view, cx) = cx.add_window_view(|window, cx| { + let focus = cx.focus_handle(); + focus.focus(window, cx); + TestView { + focus, + other_focus: cx.focus_handle(), + } + }); + cx.update(|window, cx| { + cx.bind_keys([ + KeyBinding::new("ctrl-d", SampleAction, Some("TerminalPane")), + KeyBinding::new("cmd-d", SampleAction, Some("TerminalPane")), + KeyBinding::new( + "ctrl-1", + ParameterizedAction { target: 1 }, + Some("TerminalPane"), + ), + ]); + assert_eq!(bindings_for_action(&SampleAction, window, cx).len(), 2); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("cmd-d")) + ); + assert_eq!( + shortcut_for_action(&ParameterizedAction { target: 1 }, window, cx), + Some(format_keystroke("ctrl-1")) + ); + assert_eq!( + shortcut_for_action(&ParameterizedAction { target: 2 }, window, cx), + None + ); + cx.bind_keys([KeyBinding::new( + "ctrl-j", + SampleAction, + Some("TerminalPane"), + )]); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("ctrl-j")) + ); + }); + } + + #[gpui::test] + fn resolves_root_bindings_after_deferred_draw_without_focus(cx: &mut TestAppContext) { + let (_, cx) = cx.add_window_view(|_, cx| TestView { + focus: cx.focus_handle(), + other_focus: cx.focus_handle(), }); - window - .update(cx, |_, window, _| { - assert_eq!( - shortcut_for_action(&SampleAction, Some("TerminalPane"), window), - Some(format_keystroke("ctrl-d")) - ); - assert_eq!( - shortcut_for_action(&SampleAction, Some("Sidebar"), window), - Some(format_keystroke("ctrl-p")) - ); - assert_eq!( - shortcut_for_action(&SampleAction, Some("SearchBar"), window), - None - ); - }) - .unwrap(); - cx.update(|cx| { + cx.update(|window, cx| { + window.blur(); + assert!(window.context_stack().is_empty()); cx.bind_keys([ - KeyBinding::new("ctrl-d", OtherAction, Some("TerminalPane")), - KeyBinding::new("ctrl-k ctrl-c", SampleAction, Some("TerminalPane")), - ]) + KeyBinding::new("ctrl-g", SampleAction, None), + KeyBinding::new("ctrl-d", SampleAction, Some("TerminalPane")), + KeyBinding::new("ctrl-n", SampleAction, Some("!TerminalPane")), + KeyBinding::new("ctrl-g", OtherAction, Some("!TerminalPane")), + ]); + assert_eq!( + shortcut_for_action(&SampleAction, window, cx), + Some(format_keystroke("ctrl-g")) + ); + assert_eq!(bindings_for_action(&SampleAction, window, cx).len(), 1); }); - window - .update(cx, |_, window, _| { - assert_eq!( - shortcut_for_action(&SampleAction, Some("TerminalPane"), window), - Some(format_keystroke("ctrl-k ctrl-c")) - ); - }) - .unwrap(); - cx.update(|cx| cx.clear_key_bindings()); - window - .update(cx, |_, window, _| { - assert_eq!( - shortcut_for_action(&SampleAction, Some("TerminalPane"), window), - None - ); - }) - .unwrap(); } #[test] - fn prefers_native_effective_binding_and_preserves_custom_fallback() { - let bindings = [ - KeyBinding::new("cmd-d", SampleAction, None), - KeyBinding::new("ctrl-d", SampleAction, None), - ]; + fn formats_named_keys_all_modifiers_and_preserves_invalid_input() { assert_eq!( - preferred_binding(&bindings, ShortcutPlatform::Mac) - .unwrap() - .keystrokes(), - bindings[0].keystrokes() + format_keystroke_for_platform( + "ctrl-alt-shift-cmd-fn-delete", + ShortcutPlatform::Windows + ), + "Ctrl+Alt+Shift+Win+Fn+Delete" ); assert_eq!( - preferred_binding(&bindings, ShortcutPlatform::Windows) - .unwrap() - .keystrokes(), - bindings[1].keystrokes() + format_keystroke_for_platform("ctrl-alt-shift-cmd-delete", ShortcutPlatform::Mac), + "⌃⌥⇧⌘⌦" ); assert_eq!( - preferred_binding(&bindings[1..], ShortcutPlatform::Mac) - .unwrap() - .keystrokes(), - bindings[1].keystrokes() + format_keystroke_for_platform( + "escape pageup pagedown f12 tab space", + ShortcutPlatform::Linux + ), + "Esc PgUp PgDn F12 Tab Space" + ); + assert_eq!( + format_keystroke_for_platform("ctrl-", ShortcutPlatform::Windows), + "ctrl-" + ); + assert_eq!( + format_keystroke_for_platform("", ShortcutPlatform::Windows), + "" ); } + #[test] fn formats_platform_modifiers_chords_and_punctuation() { assert_eq!( diff --git a/crates/okena-views-git/src/diff_viewer/render.rs b/crates/okena-views-git/src/diff_viewer/render.rs index c3d5aa1cb..dc340cbe3 100644 --- a/crates/okena-views-git/src/diff_viewer/render.rs +++ b/crates/okena-views-git/src/diff_viewer/render.rs @@ -91,8 +91,28 @@ impl DiffViewer { .text_size(ui_text_md(cx)) .text_color(rgb(t.text_secondary)) .hover(|s| s.bg(rgb(t.bg_hover)).text_color(rgb(t.text_primary))) - .tooltip(|window, cx| { - gpui_component::tooltip::Tooltip::new("Back").build(window, cx) + .tooltip({ + let viewer = cx.entity().downgrade(); + move |window, cx| { + let matches = viewer.upgrade().is_some_and(|viewer| { + let viewer = viewer.read(cx); + viewer.focus_handle.is_focused(window) + && viewer.can_go_back + && viewer.search.is_none() + && viewer.delete_confirm.is_none() + && viewer.discard_confirm.is_none() + && viewer.context_menu.is_none() + && viewer.commit_hash_menu.is_none() + && viewer.selection_context_menu.is_none() + && viewer.selection.normalized_non_empty().is_none() + }); + okena_ui::shortcuts::action_tooltip( + "Back", + matches.then_some(&Cancel as &dyn Action), + window, + cx, + ) + } }) .on_click(cx.listener(|this, _, _window, cx| this.back(cx))) .child( diff --git a/crates/okena-views-sidebar/src/folder_list.rs b/crates/okena-views-sidebar/src/folder_list.rs index 778795c88..7b6e95c3d 100644 --- a/crates/okena-views-sidebar/src/folder_list.rs +++ b/crates/okena-views-sidebar/src/folder_list.rs @@ -446,6 +446,7 @@ impl Sidebar { "Show Project" }, &t, + Some((self.focus_manager.clone(), project_id.clone())), ) .on_click(cx.listener({ let project_id = project_id.clone(); diff --git a/crates/okena-views-sidebar/src/item_widgets.rs b/crates/okena-views-sidebar/src/item_widgets.rs index 6e323178c..b1d7ed44d 100644 --- a/crates/okena-views-sidebar/src/item_widgets.rs +++ b/crates/okena-views-sidebar/src/item_widgets.rs @@ -231,12 +231,32 @@ pub fn sidebar_visibility_button( group_name: &'static str, tooltip_text: &'static str, t: &ThemeColors, + shortcut_target: Option<(Entity, String)>, ) -> Stateful
{ icon_button(id, "icons/eye.svg", t) .opacity(0.0) .when(show_in_overview, |d| d.opacity(1.0)) .group_hover(group_name, |s| s.opacity(1.0)) - .tooltip(move |_window, cx| Tooltip::new(tooltip_text).build(_window, cx)) + .tooltip(move |window, cx| { + let matches = shortcut_target.as_ref().is_some_and(|(focus, project_id)| { + let fm = focus.read(cx); + fm.focused_terminal_state() + .map(|state| state.project_id) + .or_else(|| fm.focused_project_id().cloned()) + .as_ref() + == Some(project_id) + }); + if matches { + okena_ui::shortcuts::named_action_tooltip( + tooltip_text, + "okena::ToggleProjectVisibility", + window, + cx, + ) + } else { + Tooltip::new(tooltip_text).build(window, cx) + } + }) } /// Project/worktree name with optional terminal count badge. diff --git a/crates/okena-views-sidebar/src/project_list.rs b/crates/okena-views-sidebar/src/project_list.rs index 06f78de07..ad66a213e 100644 --- a/crates/okena-views-sidebar/src/project_list.rs +++ b/crates/okena-views-sidebar/src/project_list.rs @@ -2,7 +2,6 @@ use gpui::prelude::*; use gpui::*; -use gpui_component::tooltip::Tooltip; use okena_core::api::ActionRequest; use okena_core::types::WorktreeMarker; use okena_ui::color_dot::color_dot; @@ -46,6 +45,32 @@ pub enum ProjectRowStyle { } impl Sidebar { + /// Sidebar rows address explicit terminals; keyboard commands address focus. + fn is_shortcut_terminal( + &self, + project_id: &str, + terminal_id: &str, + window: &Window, + cx: &App, + ) -> bool { + if !window + .context_stack() + .iter() + .any(|context| context.contains("TerminalPane")) + { + return false; + } + let fm = self.focus_manager.read(cx); + let Some(focused) = fm.focused_terminal_state() else { + return false; + }; + focused.project_id == project_id + && self.workspace.read(cx).project(project_id) + .and_then(|project| project.layout.as_ref()) + .and_then(|layout| layout.get_at_path(&focused.layout_path)) + .is_some_and(|node| matches!(node, okena_workspace::state::LayoutNode::Terminal { terminal_id: Some(id), .. } if id == terminal_id)) + } + /// Appends standard project row content (expand arrow, color dot, name, badges, visibility) /// to a pre-configured container div. Each caller sets up its own container with drag/drop. pub fn append_project_row_content( @@ -287,6 +312,8 @@ impl Sidebar { vis_tooltip_show }, &t, + (!is_worktree_style) + .then(|| (self.focus_manager.clone(), project_id.clone())), ) .on_click(cx.listener({ let project_id = project_id.clone(); @@ -790,10 +817,24 @@ impl Sidebar { })) .tooltip({ let tooltip_text = if is_minimized { "Restore" } else { "Minimize" }; - move |_window, cx| { - Tooltip::new(tooltip_text) - .action(&MinimizeTerminal as &dyn Action, None) - .build(_window, cx) + let sidebar = cx.entity().downgrade(); + let project_id = project_id.clone(); + let terminal_id = terminal_id.clone(); + move |window, cx| { + let matches = sidebar.upgrade().is_some_and(|sidebar| { + sidebar.read(cx).is_shortcut_terminal( + &project_id, + &terminal_id, + window, + cx, + ) + }); + okena_ui::shortcuts::action_tooltip( + tooltip_text, + matches.then_some(&MinimizeTerminal as &dyn Action), + window, + cx, + ) } }), ) @@ -828,10 +869,28 @@ impl Sidebar { ); } })) - .tooltip(|_window, cx| { - Tooltip::new("Fullscreen") - .action(&ToggleFullscreen as &dyn Action, None) - .build(_window, cx) + .tooltip({ + let sidebar = cx.entity().downgrade(); + let project_id = project_id.clone(); + let terminal_id = terminal_id.clone(); + move |window, cx| { + let matches = sidebar.upgrade().is_some_and(|sidebar| { + let sidebar = sidebar.read(cx); + !sidebar.focus_manager.read(cx).has_fullscreen() + && sidebar.is_shortcut_terminal( + &project_id, + &terminal_id, + window, + cx, + ) + }); + okena_ui::shortcuts::action_tooltip( + "Fullscreen", + matches.then_some(&ToggleFullscreen as &dyn Action), + window, + cx, + ) + } }), ), ) diff --git a/crates/okena-views-sidebar/src/sidebar/mod.rs b/crates/okena-views-sidebar/src/sidebar/mod.rs index 974ffcc7c..2dcaecb41 100644 --- a/crates/okena-views-sidebar/src/sidebar/mod.rs +++ b/crates/okena-views-sidebar/src/sidebar/mod.rs @@ -678,12 +678,20 @@ impl Sidebar { .text_color(rgb(t.text_secondary)), ) .tooltip(move |window, cx| { - Tooltip::new(if expand { - "Expand active projects (Terminals)" - } else { - "Collapse active projects (Terminals)" - }) - .build(window, cx) + okena_ui::shortcuts::named_action_tooltip( + if expand { + "Expand active projects (Terminals)" + } else { + "Collapse active projects (Terminals)" + }, + if expand { + "okena::ExpandActiveProjects" + } else { + "okena::CollapseActiveProjects" + }, + window, + cx, + ) }) .on_click(cx.listener(move |this, _, _, cx| { cx.stop_propagation(); diff --git a/crates/okena-views-terminal/src/layout/tabs/mod.rs b/crates/okena-views-terminal/src/layout/tabs/mod.rs index ba4354b85..da6a56866 100644 --- a/crates/okena-views-terminal/src/layout/tabs/mod.rs +++ b/crates/okena-views-terminal/src/layout/tabs/mod.rs @@ -91,12 +91,32 @@ impl LayoutContainer { path.push(ctx.active_tab); path }; + // Group split/add-tab callbacks address the group, while keyboard actions + // address its focused leaf. Only standalone targets are equivalent. + let shortcut_guard = || { + let focus_manager = self.focus_manager.clone(); + let workspace = self.workspace.clone(); + let project_id = ctx.project_id.clone(); + let path = terminal_layout_path.clone(); + let terminal_id = terminal_id.clone(); + move |window: &Window, cx: &App| { + window + .context_stack() + .iter() + .any(|context| context.contains("TerminalPane")) + && focus_manager.read(cx).is_focused(&project_id, &path) + && workspace.read(cx).project(&project_id) + .and_then(|project| project.layout.as_ref()) + .and_then(|layout| layout.get_at_path(&path)) + .is_some_and(|node| matches!(node, LayoutNode::Terminal { terminal_id: Some(id), .. } if Some(id) == terminal_id.as_ref())) + } + }; let more_button = crate::layout::layout_container::terminal_actions_button( HeaderAction::More, &id_suffix, ctx.project_id.clone(), self.request_broker.clone(), - terminal_layout_path, + terminal_layout_path.clone(), terminal_id.clone(), self.backend.supports_buffer_capture(), false, @@ -116,7 +136,12 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - Some(Box::new(crate::actions::SplitVertical)), + standalone.then(|| { + okena_ui::shortcuts::ActionShortcut::new( + crate::actions::SplitVertical, + shortcut_guard(), + ) + }), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_split_v.action_dispatcher { @@ -138,7 +163,12 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - Some(Box::new(crate::actions::SplitHorizontal)), + standalone.then(|| { + okena_ui::shortcuts::ActionShortcut::new( + crate::actions::SplitHorizontal, + shortcut_guard(), + ) + }), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_split_h.action_dispatcher { @@ -160,7 +190,12 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, None, - Some(Box::new(crate::actions::AddTab)), + standalone.then(|| { + okena_ui::shortcuts::ActionShortcut::new( + crate::actions::AddTab, + shortcut_guard(), + ) + }), ) .on_click(move |_, _window, cx| { if let Some(ref dispatcher) = ctx_add_tab.action_dispatcher { @@ -181,7 +216,12 @@ impl LayoutContainer { ButtonSize::COMPACT, &t, Some(if standalone { "Close" } else { "Close Tab" }), - Some(Box::new(crate::actions::CloseTerminal)), + terminal_id.as_ref().map(|_| { + okena_ui::shortcuts::ActionShortcut::new( + crate::actions::CloseTerminal, + shortcut_guard(), + ) + }), ) .on_click(move |_, _window, cx| { if let Some(ref tid) = terminal_id_for_close diff --git a/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs b/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs index c4e83c7c5..a9c4d1024 100644 --- a/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs +++ b/crates/okena-views-terminal/src/layout/terminal_pane/zoom.rs @@ -167,7 +167,13 @@ impl TerminalPane { size, &t, None, - Some(Box::new(crate::actions::FullscreenPrevTerminal)), + Some(okena_ui::shortcuts::ActionShortcut::new( + crate::actions::FullscreenPrevTerminal, + { + let focus = self.focus_handle.clone(); + move |window, _| focus.is_focused(window) + }, + )), ) .on_click({ let workspace = workspace.clone(); @@ -211,7 +217,13 @@ impl TerminalPane { size, &t, None, - Some(Box::new(crate::actions::FullscreenNextTerminal)), + Some(okena_ui::shortcuts::ActionShortcut::new( + crate::actions::FullscreenNextTerminal, + { + let focus = self.focus_handle.clone(); + move |window, _| focus.is_focused(window) + }, + )), ) .on_click({ let workspace = workspace.clone(); @@ -252,7 +264,13 @@ impl TerminalPane { size, &t, None, - Some(Box::new(crate::actions::ToggleFullscreen)), + Some(okena_ui::shortcuts::ActionShortcut::new( + crate::actions::ToggleFullscreen, + { + let focus = self.focus_handle.clone(); + move |window, _| focus.is_focused(window) + }, + )), ) .on_click({ let project_id = self.project_id.clone(); From 46b4083c9cf0b3de8d5ca934a56e30ea971cd5d2 Mon Sep 17 00:00:00 2001 From: Frantisek Bohac Date: Thu, 12 Nov 2026 21:26:30 +0100 Subject: [PATCH 3/4] Name shortcut applicability callback type for Clippy --- crates/okena-ui/src/shortcuts.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/okena-ui/src/shortcuts.rs b/crates/okena-ui/src/shortcuts.rs index 7e929d3cf..029bb0483 100644 --- a/crates/okena-ui/src/shortcuts.rs +++ b/crates/okena-ui/src/shortcuts.rs @@ -164,11 +164,13 @@ pub fn named_action_tooltip( action_tooltip(label, action.as_deref(), window, cx) } +type ShortcutApplicability = Box bool>; + /// A reusable button's shortcut and its live operation/target eligibility check. /// The check runs when the tooltip opens, not when the button was last rendered. pub struct ActionShortcut { action: Box, - is_applicable: Box bool>, + is_applicable: ShortcutApplicability, } impl ActionShortcut { From 84caca9fad48f5bddd929f4a40e26830b84ed73d Mon Sep 17 00:00:00 2001 From: David Matejka Date: Sat, 14 Nov 2026 14:33:25 +0100 Subject: [PATCH 4/4] Prefer the native binding in action tooltips The default keymap registers a cmd-* and a ctrl-shift-* variant of most terminal actions on every platform, ctrl last. Picking the last effective binding showed the Ctrl variant on macOS. Prefer bindings whose first stroke uses Cmd on macOS and avoids Win/Super elsewhere; among those the last registered still wins, and other bindings remain the fallback. Co-Authored-By: Claude Opus 5.5 --- crates/okena-ui/src/shortcuts.rs | 68 ++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/crates/okena-ui/src/shortcuts.rs b/crates/okena-ui/src/shortcuts.rs index 029bb0483..0ed8c7a48 100644 --- a/crates/okena-ui/src/shortcuts.rs +++ b/crates/okena-ui/src/shortcuts.rs @@ -131,12 +131,27 @@ pub fn bindings_for_action(action: &dyn Action, window: &Window, cx: &App) -> Ve } /// Chooses one display binding without removing alternatives. GPUI returns effective -/// bindings in registration order; the last one wins, on every platform. Modifiers -/// do not determine eligibility: custom Ctrl on macOS and Win/Super elsewhere work. +/// bindings in registration order; the last one wins, on every platform. pub fn shortcut_for_action(action: &dyn Action, window: &Window, cx: &App) -> Option { - bindings_for_action(action, window, cx) - .last() - .map(format_binding) + let bindings = bindings_for_action(action, window, cx); + preferred_binding(&bindings, ShortcutPlatform::current()).map(format_binding) +} + +/// Defaults register both a `cmd-…` and a Ctrl variant on every platform, so prefer +/// the native one (Cmd on macOS, no Win/Super elsewhere); among equals the last wins. +/// Other modifiers stay eligible as a fallback: custom Ctrl on macOS and Win/Super work. +fn preferred_binding(bindings: &[KeyBinding], platform: ShortcutPlatform) -> Option<&KeyBinding> { + let is_native = |binding: &&KeyBinding| { + binding.keystrokes().first().is_some_and(|stroke| { + let uses_platform_key = stroke.as_keystroke().modifiers.platform; + uses_platform_key == (platform == ShortcutPlatform::Mac) + }) + }; + bindings + .iter() + .rev() + .find(is_native) + .or_else(|| bindings.last()) } pub fn action_tooltip( @@ -298,9 +313,14 @@ mod tests { ), ]); assert_eq!(bindings_for_action(&SampleAction, window, cx).len(), 2); + let native = if ShortcutPlatform::current() == ShortcutPlatform::Mac { + "cmd-d" + } else { + "ctrl-d" + }; assert_eq!( shortcut_for_action(&SampleAction, window, cx), - Some(format_keystroke("cmd-d")) + Some(format_keystroke(native)) ); assert_eq!( shortcut_for_action(&ParameterizedAction { target: 1 }, window, cx), @@ -310,18 +330,50 @@ mod tests { shortcut_for_action(&ParameterizedAction { target: 2 }, window, cx), None ); + let remapped = if ShortcutPlatform::current() == ShortcutPlatform::Mac { + "cmd-j" + } else { + "ctrl-j" + }; cx.bind_keys([KeyBinding::new( - "ctrl-j", + remapped, SampleAction, Some("TerminalPane"), )]); assert_eq!( shortcut_for_action(&SampleAction, window, cx), - Some(format_keystroke("ctrl-j")) + Some(format_keystroke(remapped)) ); }); } + #[test] + fn prefers_the_native_default_and_falls_back_to_the_last_binding() { + let defaults = [ + KeyBinding::new("cmd-d", SampleAction, None), + KeyBinding::new("ctrl-shift-d", SampleAction, None), + ]; + let chosen = |bindings: &[KeyBinding], platform| { + preferred_binding(bindings, platform).map(format_binding) + }; + assert_eq!( + chosen(&defaults, ShortcutPlatform::Mac), + Some(format_binding(&defaults[0])) + ); + for platform in [ShortcutPlatform::Linux, ShortcutPlatform::Windows] { + assert_eq!( + chosen(&defaults, platform), + Some(format_binding(&defaults[1])) + ); + } + let custom_only = [KeyBinding::new("ctrl-j", SampleAction, None)]; + assert_eq!( + chosen(&custom_only, ShortcutPlatform::Mac), + Some(format_binding(&custom_only[0])) + ); + assert_eq!(chosen(&[], ShortcutPlatform::Linux), None); + } + #[gpui::test] fn resolves_root_bindings_after_deferred_draw_without_focus(cx: &mut TestAppContext) { let (_, cx) = cx.add_window_view(|_, cx| TestView {