From ce8d5e41a5ad5f2845ac89c264f80068b3ceffce Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Mon, 13 Oct 2025 12:19:05 -0300 Subject: [PATCH] settings_ui: Make arrow keys up and down activate page content (#40106) Release Notes: - settings ui: Navigating the settings navbar with arrow keys up and down now also activates the page, allowing users to more quickly see the content for a given page before moving focus to the page itself. --- assets/keymaps/default-linux.json | 3 + assets/keymaps/default-macos.json | 3 + assets/keymaps/default-windows.json | 3 + crates/settings_ui/src/settings_ui.rs | 106 +++++++++++++++++++++++--- 4 files changed, 104 insertions(+), 11 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index ae15492b804274bc3ceacaae156a19093344a9b7..d85aec85d9c15442222b2cd1c934be20d440fe1a 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1249,6 +1249,7 @@ "escape": "workspace::CloseWindow", "ctrl-m": "settings_editor::Minimize", "ctrl-f": "search::FocusSearch", + "left": "settings_editor::ToggleFocusNav", "ctrl-shift-e": "settings_editor::ToggleFocusNav", // todo(settings_ui): cut this down based on the max files and overflow UI "ctrl-1": ["settings_editor::FocusFile", 0], @@ -1269,6 +1270,8 @@ "context": "SettingsWindow > NavigationMenu", "use_key_equivalents": true, "bindings": { + "up": "settings_editor::FocusPreviousNavEntry", + "down": "settings_editor::FocusNextNavEntry", "right": "settings_editor::ExpandNavEntry", "left": "settings_editor::CollapseNavEntry", "pageup": "settings_editor::FocusPreviousRootNavEntry", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 50528dca6f97e8b287333d4011ebc97bb1ed27a2..95c959b086a306e37b2069cf869e9cb6dcd98e89 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1354,6 +1354,7 @@ "escape": "workspace::CloseWindow", "cmd-m": "settings_editor::Minimize", "cmd-f": "search::FocusSearch", + "left": "settings_editor::ToggleFocusNav", "cmd-shift-e": "settings_editor::ToggleFocusNav", // todo(settings_ui): cut this down based on the max files and overflow UI "ctrl-1": ["settings_editor::FocusFile", 0], @@ -1374,6 +1375,8 @@ "context": "SettingsWindow > NavigationMenu", "use_key_equivalents": true, "bindings": { + "up": "settings_editor::FocusPreviousNavEntry", + "down": "settings_editor::FocusNextNavEntry", "right": "settings_editor::ExpandNavEntry", "left": "settings_editor::CollapseNavEntry", "pageup": "settings_editor::FocusPreviousRootNavEntry", diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 570766e92ce8adddb6913fcc20acd71bf7ed240b..ae754f2e60b8c7fbfaa84aef996f72bc00cace21 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -1270,6 +1270,7 @@ "escape": "workspace::CloseWindow", "ctrl-m": "settings_editor::Minimize", "ctrl-f": "search::FocusSearch", + "left": "settings_editor::ToggleFocusNav", "ctrl-shift-e": "settings_editor::ToggleFocusNav", // todo(settings_ui): cut this down based on the max files and overflow UI "ctrl-1": ["settings_editor::FocusFile", 0], @@ -1290,6 +1291,8 @@ "context": "SettingsWindow > NavigationMenu", "use_key_equivalents": true, "bindings": { + "up": "settings_editor::FocusPreviousNavEntry", + "down": "settings_editor::FocusNextNavEntry", "right": "settings_editor::ExpandNavEntry", "left": "settings_editor::CollapseNavEntry", "pageup": "settings_editor::FocusPreviousRootNavEntry", diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 0ad5cb647e58f70242c593d5cd85fe18bc22a820..7282bdbf593e6d074d2f39b849fbfa525df37596 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -75,7 +75,11 @@ actions!( /// Focuses the first navigation entry. FocusFirstNavEntry, /// Focuses the last navigation entry. - FocusLastNavEntry + FocusLastNavEntry, + /// Focuses and opens the next navigation entry without moving focus to content. + FocusNextNavEntry, + /// Focuses and opens the previous navigation entry without moving focus to content. + FocusPreviousNavEntry ] ); @@ -459,6 +463,7 @@ pub struct SettingsWindow { /// [page_index][page_item_index] will be false /// when the item is filtered out either by searches /// or by the current file + navbar_focus_subscriptions: Vec, filter_table: Vec>, has_query: bool, content_handles: Vec>>, @@ -889,6 +894,7 @@ impl SettingsWindow { window, cx, ), + navbar_focus_subscriptions: vec![], content_focus_handle: NonFocusableHandle::new( CONTENT_CONTAINER_TAB_INDEX, false, @@ -934,7 +940,8 @@ impl SettingsWindow { } fn build_navbar(&mut self, cx: &App) { - let mut navbar_entries = Vec::with_capacity(self.navbar_entries.len()); + let mut navbar_entries = Vec::new(); + for (page_index, page) in self.pages.iter().enumerate() { navbar_entries.push(NavBarEntry { title: page.title, @@ -963,6 +970,30 @@ impl SettingsWindow { self.navbar_entries = navbar_entries; } + fn setup_navbar_focus_subscriptions( + &mut self, + window: &mut Window, + cx: &mut Context, + ) { + let mut focus_subscriptions = Vec::new(); + + for entry_index in 0..self.navbar_entries.len() { + let focus_handle = self.navbar_entries[entry_index].focus_handle.clone(); + + let subscription = cx.on_focus( + &focus_handle, + window, + move |this: &mut SettingsWindow, + window: &mut Window, + cx: &mut Context| { + this.open_and_scroll_to_navbar_entry(entry_index, window, cx, false); + }, + ); + focus_subscriptions.push(subscription); + } + self.navbar_focus_subscriptions = focus_subscriptions; + } + fn visible_navbar_entries(&self) -> impl Iterator { let mut index = 0; let entries = &self.navbar_entries; @@ -1258,6 +1289,7 @@ impl SettingsWindow { if self.pages.is_empty() { self.pages = page_data::settings_data(); self.build_navbar(cx); + self.setup_navbar_focus_subscriptions(window, cx); self.build_content_handles(window, cx); } sub_page_stack_mut().clear(); @@ -1355,7 +1387,7 @@ impl SettingsWindow { .visible_navbar_entries() .any(|(index, _)| index == self.navbar_entry) { - self.open_and_scroll_to_navbar_entry(self.navbar_entry, window, cx); + self.open_and_scroll_to_navbar_entry(self.navbar_entry, window, cx, true); } else { self.open_first_nav_page(); }; @@ -1571,6 +1603,38 @@ impl SettingsWindow { this.focus_and_scroll_to_nav_entry(last_entry_index, window, cx); } })) + .on_action(cx.listener(|this, _: &FocusNextNavEntry, window, cx| { + let entry_index = this + .focused_nav_entry(window, cx) + .unwrap_or(this.navbar_entry); + let mut next_index = None; + for (index, _) in this.visible_navbar_entries() { + if index > entry_index { + next_index = Some(index); + break; + } + } + let Some(next_entry_index) = next_index else { + return; + }; + this.open_and_scroll_to_navbar_entry(next_entry_index, window, cx, false); + })) + .on_action(cx.listener(|this, _: &FocusPreviousNavEntry, window, cx| { + let entry_index = this + .focused_nav_entry(window, cx) + .unwrap_or(this.navbar_entry); + let mut prev_index = None; + for (index, _) in this.visible_navbar_entries() { + if index >= entry_index { + break; + } + prev_index = Some(index); + } + let Some(prev_entry_index) = prev_index else { + return; + }; + this.open_and_scroll_to_navbar_entry(prev_entry_index, window, cx, false); + })) .border_color(cx.theme().colors().border) .bg(cx.theme().colors().panel_background) .child(self.render_search(window, cx)) @@ -1602,6 +1666,9 @@ impl SettingsWindow { .on_toggle(cx.listener( move |this, _, window, cx| { this.toggle_navbar_entry(ix); + // Update selection state immediately before cx.notify + // to prevent double selection flash + this.navbar_entry = ix; window.focus( &this.navbar_entries[ix].focus_handle, ); @@ -1612,7 +1679,7 @@ impl SettingsWindow { .on_click( cx.listener(move |this, _, window, cx| { this.open_and_scroll_to_navbar_entry( - ix, window, cx, + ix, window, cx, true, ); }), ) @@ -1651,6 +1718,7 @@ impl SettingsWindow { navbar_entry_index: usize, window: &mut Window, cx: &mut Context, + focus_content: bool, ) { self.open_navbar_entry_page(navbar_entry_index); cx.notify(); @@ -1658,12 +1726,17 @@ impl SettingsWindow { if self.navbar_entries[navbar_entry_index].is_root || !self.is_nav_entry_visible(navbar_entry_index) { - let Some(first_item_index) = self.visible_page_items().next().map(|(index, _)| index) - else { - return; - }; - self.focus_content_element(first_item_index, window, cx); self.page_scroll_handle.set_offset(point(px(0.), px(0.))); + if focus_content { + let Some(first_item_index) = + self.visible_page_items().next().map(|(index, _)| index) + else { + return; + }; + self.focus_content_element(first_item_index, window, cx); + } else { + window.focus(&self.navbar_entries[navbar_entry_index].focus_handle); + } } else { let entry_item_index = self.navbar_entries[navbar_entry_index] .item_index @@ -1676,7 +1749,12 @@ impl SettingsWindow { }; self.page_scroll_handle .scroll_to_top_of_item(selected_item_index); - self.focus_content_element(entry_item_index, window, cx); + + if focus_content { + self.focus_content_element(entry_item_index, window, cx); + } else { + window.focus(&self.navbar_entries[navbar_entry_index].focus_handle); + } } // Page scroll handle updates the active item index @@ -2132,7 +2210,12 @@ impl Render for SettingsWindow { .focus_handle(cx) .contains_focused(window, cx) { - this.open_and_scroll_to_navbar_entry(this.navbar_entry, window, cx); + this.open_and_scroll_to_navbar_entry( + this.navbar_entry, + window, + cx, + true, + ); } else { this.focus_and_scroll_to_nav_entry(this.navbar_entry, window, cx); } @@ -2507,6 +2590,7 @@ mod test { navbar_entry: selected_idx.expect("Must have a selected navbar entry"), navbar_entries: Vec::default(), navbar_scroll_handle: UniformListScrollHandle::default(), + navbar_focus_subscriptions: vec![], filter_table: vec![], has_query: false, content_handles: vec![],