From bb24c085bef61459fec19a1c53ceb5fa73cfafd6 Mon Sep 17 00:00:00 2001 From: tims <0xtimsb@gmail.com> Date: Fri, 3 Jan 2025 23:49:24 +0530 Subject: [PATCH] linux: Add keyboard shortcuts for menus (#22074) Closes #19837 This PR is a continuation of [linux: Implement Menus](https://github.com/zed-industries/zed/pull/21873) and should only be reviewed once the existing PR is merged. I created this as a separate PR as the existing PR was already reviewed but is yet to merge, and also it was my initial plan to do it in separate parts because of the scope of it. This will also help reviewing code faster. This PR adds two new types of keyboard shortcuts to make menu navigation easier: 1. `Alt + Z` for Zed, `Alt + F` for File, `Alt + S` for Selection, and so on to open a specific menu with this combination. This mimics VSCode and IntelliJ. 2. `Arrow Left/Right` when any menu is open. This will trigger the current menu to close, and the previous/next to open respectively. First and last element cycling is handled. `Arrow Up/Down` to navigate menu entries is already there in existing work. https://github.com/user-attachments/assets/976aea48-4e20-4c19-850d-4d205a4bead2 Release Notes: - Added keyboard navigation for menus on Linux (left/right). If you wish to open menus with keyboard shortcuts add the following to your user keymap: ```json { "context": "Workspace", "bindings": { "alt-z": ["app_menu::OpenApplicationMenu", "Zed"], "alt-f": ["app_menu::OpenApplicationMenu", "File"], "alt-e": ["app_menu::OpenApplicationMenu", "Edit"], "alt-s": ["app_menu::OpenApplicationMenu", "Selection"], "alt-v": ["app_menu::OpenApplicationMenu", "View"], "alt-g": ["app_menu::OpenApplicationMenu", "Go"], "alt-w": ["app_menu::OpenApplicationMenu", "Window"], "alt-h": ["app_menu::OpenApplicationMenu", "Help"] } } ``` --------- Co-authored-by: Peter Tripp --- assets/keymaps/default-linux.json | 7 ++ crates/title_bar/src/application_menu.rs | 107 +++++++++++++++++++++-- crates/title_bar/src/title_bar.rs | 40 ++++++++- 3 files changed, 143 insertions(+), 11 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index c8af60d74ad2bf4caac8800d1abe324319cb947d..dc803f1278ca029df0c2b3b45d539cd52301e7f4 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -435,6 +435,13 @@ // "foo-bar": ["task::Spawn", { "task_name": "MyTask", "reveal_target": "dock" }] } }, + { + "context": "ApplicationMenu", + "bindings": { + "left": ["app_menu::NavigateApplicationMenuInDirection", "Left"], + "right": ["app_menu::NavigateApplicationMenuInDirection", "Right"] + } + }, // Bindings from Sublime Text { "context": "Editor", diff --git a/crates/title_bar/src/application_menu.rs b/crates/title_bar/src/application_menu.rs index 95124300d67e1f6f767be5df5d7ec92a78e5e57a..1c3e67c0958fdda7b0b14f73f6049ffc24ca049a 100644 --- a/crates/title_bar/src/application_menu.rs +++ b/crates/title_bar/src/application_menu.rs @@ -1,7 +1,19 @@ -use gpui::{OwnedMenu, OwnedMenuItem, View}; +use gpui::{impl_actions, OwnedMenu, OwnedMenuItem, View}; +use serde::Deserialize; use smallvec::SmallVec; use ui::{prelude::*, ContextMenu, PopoverMenu, PopoverMenuHandle, Tooltip}; +impl_actions!( + app_menu, + [OpenApplicationMenu, NavigateApplicationMenuInDirection,] +); + +#[derive(Clone, Deserialize, PartialEq, Default)] +pub struct OpenApplicationMenu(String); + +#[derive(Clone, Deserialize, PartialEq, Default)] +pub struct NavigateApplicationMenuInDirection(String); + #[derive(Clone)] struct MenuEntry { menu: OwnedMenu, @@ -10,6 +22,7 @@ struct MenuEntry { pub struct ApplicationMenu { entries: SmallVec<[MenuEntry; 8]>, + pending_menu_open: Option, } impl ApplicationMenu { @@ -23,6 +36,7 @@ impl ApplicationMenu { handle: PopoverMenuHandle::default(), }) .collect(), + pending_menu_open: None, } } @@ -62,6 +76,7 @@ impl ApplicationMenu { fn build_menu_from_items(entry: MenuEntry, cx: &mut WindowContext) -> View { ContextMenu::build(cx, |menu, cx| { + // Grab current focus handle so menu can shown items in context with the focused element let menu = menu.when_some(cx.focused(), |menu, focused| menu.context(focused)); let sanitized_items = Self::sanitize_menu_items(entry.menu.items); @@ -93,7 +108,6 @@ impl ApplicationMenu { let entry = entry.clone(); // Application menu must have same ids as first menu item in standard menu - // Hence, we generate ids based on the menu name div() .id(SharedString::from(format!("{}-menu-item", menu_name))) .occlude() @@ -144,33 +158,108 @@ impl ApplicationMenu { .with_handle(current_handle.clone()), ) .on_hover(move |hover_enter, cx| { - // Skip if menu is already open to avoid focus issue if *hover_enter && !current_handle.is_deployed() { all_handles.iter().for_each(|h| h.hide(cx)); - // Defer to prevent focus race condition with the previously open menu + // We need to defer this so that this menu handle can take focus from the previous menu let handle = current_handle.clone(); cx.defer(move |cx| handle.show(cx)); } }) } - pub fn is_any_deployed(&self) -> bool { + #[cfg(not(target_os = "macos"))] + pub fn open_menu(&mut self, action: &OpenApplicationMenu, _cx: &mut ViewContext) { + self.pending_menu_open = Some(action.0.clone()); + } + + #[cfg(not(target_os = "macos"))] + pub fn navigate_menus_in_direction( + &mut self, + action: &NavigateApplicationMenuInDirection, + cx: &mut ViewContext, + ) { + let current_index = self + .entries + .iter() + .position(|entry| entry.handle.is_deployed()); + let Some(current_index) = current_index else { + return; + }; + + let next_index = match action.0.as_str() { + "Left" => { + if current_index == 0 { + self.entries.len() - 1 + } else { + current_index - 1 + } + } + "Right" => { + if current_index == self.entries.len() - 1 { + 0 + } else { + current_index + 1 + } + } + _ => return, + }; + + self.entries[current_index].handle.hide(cx); + + // We need to defer this so that this menu handle can take focus from the previous menu + let next_handle = self.entries[next_index].handle.clone(); + cx.defer(move |_, cx| next_handle.show(cx)); + } + + pub fn all_menus_shown(&self) -> bool { self.entries.iter().any(|entry| entry.handle.is_deployed()) + || self.pending_menu_open.is_some() } } impl Render for ApplicationMenu { - fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { - let is_any_deployed = self.is_any_deployed(); + fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { + let all_menus_shown = self.all_menus_shown(); + + if let Some(pending_menu_open) = self.pending_menu_open.take() { + if let Some(entry) = self + .entries + .iter() + .find(|entry| entry.menu.name == pending_menu_open && !entry.handle.is_deployed()) + { + let handle_to_show = entry.handle.clone(); + let handles_to_hide: Vec<_> = self + .entries + .iter() + .filter(|e| e.menu.name != pending_menu_open && e.handle.is_deployed()) + .map(|e| e.handle.clone()) + .collect(); + + if handles_to_hide.is_empty() { + // We need to wait for the next frame to show all menus first, + // before we can handle show/hide operations + cx.window_context().on_next_frame(move |cx| { + handles_to_hide.iter().for_each(|handle| handle.hide(cx)); + cx.defer(move |cx| handle_to_show.show(cx)); + }); + } else { + // Since menus are already shown, we can directly handle show/hide operations + handles_to_hide.iter().for_each(|handle| handle.hide(cx)); + cx.defer(move |_, cx| handle_to_show.show(cx)); + } + } + } + div() + .key_context("ApplicationMenu") .flex() .flex_row() .gap_x_1() - .when(!is_any_deployed && !self.entries.is_empty(), |this| { + .when(!all_menus_shown && !self.entries.is_empty(), |this| { this.child(self.render_application_menu(&self.entries[0])) }) - .when(is_any_deployed, |this| { + .when(all_menus_shown, |this| { this.children( self.entries .iter() diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index dac9a98d69e3aefbb6265fdf2264a6365bd60ce2..23449ddeb0740fdbab31aa9c49c487c37eeea196 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -7,6 +7,10 @@ mod window_controls; mod stories; use crate::application_menu::ApplicationMenu; + +#[cfg(not(target_os = "macos"))] +use crate::application_menu::{NavigateApplicationMenuInDirection, OpenApplicationMenu}; + use crate::platforms::{platform_linux, platform_mac, platform_windows}; use auto_update::AutoUpdateStatus; use call::ActiveCall; @@ -53,7 +57,39 @@ actions!( pub fn init(cx: &mut AppContext) { cx.observe_new_views(|workspace: &mut Workspace, cx| { let item = cx.new_view(|cx| TitleBar::new("title-bar", workspace, cx)); - workspace.set_titlebar_item(item.into(), cx) + workspace.set_titlebar_item(item.into(), cx); + + #[cfg(not(target_os = "macos"))] + workspace.register_action(|workspace, action: &OpenApplicationMenu, cx| { + if let Some(titlebar) = workspace + .titlebar_item() + .and_then(|item| item.downcast::().ok()) + { + titlebar.update(cx, |titlebar, cx| { + if let Some(ref menu) = titlebar.application_menu { + menu.update(cx, |menu, cx| menu.open_menu(action, cx)); + } + }); + } + }); + + #[cfg(not(target_os = "macos"))] + workspace.register_action( + |workspace, action: &NavigateApplicationMenuInDirection, cx| { + if let Some(titlebar) = workspace + .titlebar_item() + .and_then(|item| item.downcast::().ok()) + { + titlebar.update(cx, |titlebar, cx| { + if let Some(ref menu) = titlebar.application_menu { + menu.update(cx, |menu, cx| { + menu.navigate_menus_in_direction(action, cx) + }); + } + }); + } + }, + ); }) .detach(); } @@ -138,7 +174,7 @@ impl Render for TitleBar { let mut render_project_items = true; title_bar .when_some(self.application_menu.clone(), |title_bar, menu| { - render_project_items = !menu.read(cx).is_any_deployed(); + render_project_items = !menu.read(cx).all_menus_shown(); title_bar.child(menu) }) .when(render_project_items, |title_bar| {