diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index d2f70b825b4efa3544e916589846e7944a91771a..fff7469199f88b88bb02fcf2d595d5ee76628315 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1280,7 +1280,9 @@ "use_key_equivalents": true, "bindings": { "up": "settings_editor::FocusPreviousNavEntry", + "shift-tab": "settings_editor::FocusPreviousNavEntry", "down": "settings_editor::FocusNextNavEntry", + "tab": "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 75dd84f7ae269f0883809c8ec1b2516b25f024c9..0b4119e95e4bf33d1f19a538fa231cc13ff79419 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1386,7 +1386,9 @@ "use_key_equivalents": true, "bindings": { "up": "settings_editor::FocusPreviousNavEntry", + "shift-tab": "settings_editor::FocusPreviousNavEntry", "down": "settings_editor::FocusNextNavEntry", + "tab": "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 e38bca90e4394ec415df253a7d9668cadefceac1..39c1b672a4105e9565bbdaded7229402831c702d 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -1309,7 +1309,9 @@ "use_key_equivalents": true, "bindings": { "up": "settings_editor::FocusPreviousNavEntry", + "shift-tab": "settings_editor::FocusPreviousNavEntry", "down": "settings_editor::FocusNextNavEntry", + "tab": "settings_editor::FocusNextNavEntry", "right": "settings_editor::ExpandNavEntry", "left": "settings_editor::CollapseNavEntry", "pageup": "settings_editor::FocusPreviousRootNavEntry", diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 6855874bae25db1b0990541902333e4e72a283b3..6d74a0e11f7a7ecde003f48b084f4720bd03230e 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -4313,14 +4313,14 @@ impl Window { } /// Returns a generic handler that invokes the given handler with the view and context associated with the given view handle. - pub fn handler_for) + 'static>( + pub fn handler_for) + 'static>( &self, - view: &Entity, + entity: &Entity, f: Callback, - ) -> impl Fn(&mut Window, &mut App) + use { - let view = view.downgrade(); + ) -> impl Fn(&mut Window, &mut App) + 'static { + let entity = entity.downgrade(); move |window: &mut Window, cx: &mut App| { - view.update(cx, |view, cx| f(view, window, cx)).ok(); + entity.update(cx, |entity, cx| f(entity, window, cx)).ok(); } } diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index c9b1b7ac9e7632bb9741cab396ee5f2d6252603c..ef3049ea20953c032714f4855a1b3da8a60a5434 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -1052,7 +1052,7 @@ impl SettingsWindow { }; // high overdraw value so the list scrollbar len doesn't change too much - let list_state = gpui::ListState::new(0, gpui::ListAlignment::Top, px(100.0)).measure_all(); + let list_state = gpui::ListState::new(0, gpui::ListAlignment::Top, px(0.0)).measure_all(); list_state.set_scroll_handler(|_, _, _| {}); let mut this = Self { @@ -1184,7 +1184,7 @@ impl SettingsWindow { move |this: &mut SettingsWindow, window: &mut Window, cx: &mut Context| { - this.open_and_scroll_to_navbar_entry(entry_index, window, cx, false); + this.open_and_scroll_to_navbar_entry(entry_index, None, false, window, cx); }, ); focus_subscriptions.push(subscription); @@ -1622,7 +1622,7 @@ impl SettingsWindow { .visible_navbar_entries() .any(|(index, _)| index == self.navbar_entry) { - self.open_and_scroll_to_navbar_entry(self.navbar_entry, window, cx, true); + self.open_and_scroll_to_navbar_entry(self.navbar_entry, None, true, window, cx); } else { self.open_first_nav_page(); }; @@ -1921,7 +1921,13 @@ impl SettingsWindow { let Some(next_entry_index) = next_index else { return; }; - this.open_and_scroll_to_navbar_entry(next_entry_index, window, cx, false); + this.open_and_scroll_to_navbar_entry( + next_entry_index, + Some(gpui::ScrollStrategy::Bottom), + false, + window, + cx, + ); })) .on_action(cx.listener(|this, _: &FocusPreviousNavEntry, window, cx| { let entry_index = this @@ -1937,7 +1943,13 @@ impl SettingsWindow { let Some(prev_entry_index) = prev_index else { return; }; - this.open_and_scroll_to_navbar_entry(prev_entry_index, window, cx, false); + this.open_and_scroll_to_navbar_entry( + prev_entry_index, + Some(gpui::ScrollStrategy::Top), + false, + window, + cx, + ); })) .border_color(cx.theme().colors().border) .bg(cx.theme().colors().panel_background) @@ -1957,21 +1969,22 @@ impl SettingsWindow { this.visible_navbar_entries() .skip(range.start.saturating_sub(1)) .take(range.len()) - .map(|(ix, entry)| { + .map(|(entry_index, entry)| { TreeViewItem::new( - ("settings-ui-navbar-entry", ix), + ("settings-ui-navbar-entry", entry_index), entry.title, ) .track_focus(&entry.focus_handle) .root_item(entry.is_root) - .toggle_state(this.is_navbar_entry_selected(ix)) + .toggle_state(this.is_navbar_entry_selected(entry_index)) .when(entry.is_root, |item| { item.expanded(entry.expanded || this.has_query) .on_toggle(cx.listener( move |this, _, window, cx| { - this.toggle_navbar_entry(ix); + this.toggle_navbar_entry(entry_index); window.focus( - &this.navbar_entries[ix].focus_handle, + &this.navbar_entries[entry_index] + .focus_handle, ); cx.notify(); }, @@ -1980,7 +1993,11 @@ impl SettingsWindow { .on_click( cx.listener(move |this, _, window, cx| { this.open_and_scroll_to_navbar_entry( - ix, window, cx, true, + entry_index, + None, + true, + window, + cx, ); }), ) @@ -2017,13 +2034,16 @@ impl SettingsWindow { fn open_and_scroll_to_navbar_entry( &mut self, navbar_entry_index: usize, + scroll_strategy: Option, + focus_content: bool, window: &mut Window, cx: &mut Context, - focus_content: bool, ) { self.open_navbar_entry_page(navbar_entry_index); cx.notify(); + let mut handle_to_focus = None; + if self.navbar_entries[navbar_entry_index].is_root || !self.is_nav_entry_visible(navbar_entry_index) { @@ -2035,9 +2055,17 @@ impl SettingsWindow { else { return; }; - self.focus_content_element(first_item_index, window, cx); + handle_to_focus = Some(self.focus_handle_for_content_element(first_item_index, cx)); + } else if !self.is_nav_entry_visible(navbar_entry_index) { + let Some(first_visible_nav_entry_index) = + self.visible_navbar_entries().next().map(|(index, _)| index) + else { + return; + }; + self.focus_and_scroll_to_nav_entry(first_visible_nav_entry_index, window, cx); } else { - window.focus(&self.navbar_entries[navbar_entry_index].focus_handle); + handle_to_focus = + Some(self.navbar_entries[navbar_entry_index].focus_handle.clone()); } } else { let entry_item_index = self.navbar_entries[navbar_entry_index] @@ -2055,18 +2083,33 @@ impl SettingsWindow { offset_in_item: px(0.), }); if focus_content { - self.focus_content_element(entry_item_index, window, cx); + handle_to_focus = Some(self.focus_handle_for_content_element(entry_item_index, cx)); } else { - window.focus(&self.navbar_entries[navbar_entry_index].focus_handle); + handle_to_focus = + Some(self.navbar_entries[navbar_entry_index].focus_handle.clone()); } } + if let Some(scroll_strategy) = scroll_strategy + && let Some(logical_entry_index) = self + .visible_navbar_entries() + .into_iter() + .position(|(index, _)| index == navbar_entry_index) + { + self.navbar_scroll_handle + .scroll_to_item(logical_entry_index + 1, scroll_strategy); + } + // Page scroll handle updates the active item index // in it's next paint call after using scroll_handle.scroll_to_top_of_item // The call after that updates the offset of the scroll handle. So to // ensure the scroll handle doesn't lag behind we need to render three frames // back to back. - cx.on_next_frame(window, |_, window, cx| { + cx.on_next_frame(window, move |_, window, cx| { + if let Some(handle) = handle_to_focus.as_ref() { + window.focus(handle); + } + cx.on_next_frame(window, |_, _, cx| { cx.notify(); }); @@ -2133,7 +2176,7 @@ impl SettingsWindow { fn render_page_items( &mut self, - page_index: Option, + page_index: usize, _window: &mut Window, cx: &mut Context, ) -> impl IntoElement { @@ -2197,16 +2240,14 @@ impl SettingsWindow { .unwrap_or(false); let is_last = Some(actual_item_index) == last_non_header_index; + let item_focus_handle = + this.content_handles[page_index][actual_item_index].focus_handle(cx); + v_flex() .id(("settings-page-item", actual_item_index)) .w_full() .min_w_0() - .when_some(page_index, |element, page_index| { - element.track_focus( - &this.content_handles[page_index][actual_item_index] - .focus_handle(cx), - ) - }) + .track_focus(&item_focus_handle) .child(item.render( this, actual_item_index, @@ -2327,7 +2368,7 @@ impl SettingsWindow { page_header = self.render_files_header(window, cx).into_any_element(); page_content = self - .render_page_items(Some(self.current_page_index()), window, cx) + .render_page_items(self.current_page_index(), window, cx) .into_any_element(); } else { page_header = h_flex() @@ -2356,6 +2397,65 @@ impl SettingsWindow { .pb_8() .px_8() .bg(cx.theme().colors().editor_background) + .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| { + if !sub_page_stack().is_empty() { + window.focus_next(); + return; + } + for (logical_index, (actual_index, _)) in this.visible_page_items().enumerate() { + let handle = this.content_handles[this.current_page_index()][actual_index] + .focus_handle(cx); + let mut offset = 1; // for page header + + if let Some((_, next_item)) = this.visible_page_items().nth(logical_index + 1) + && matches!(next_item, SettingsPageItem::SectionHeader(_)) + { + offset += 1; + } + if handle.contains_focused(window, cx) { + let next_logical_index = logical_index + offset + 1; + this.list_state.scroll_to_reveal_item(next_logical_index); + // We need to render the next item to ensure it's focus handle is in the element tree + cx.on_next_frame(window, |_, window, cx| { + window.focus_next(); + cx.notify(); + }); + cx.notify(); + return; + } + } + window.focus_next(); + })) + .on_action(cx.listener(|this, _: &menu::SelectPrevious, window, cx| { + if !sub_page_stack().is_empty() { + window.focus_prev(); + return; + } + let mut prev_was_header = false; + for (logical_index, (actual_index, item)) in this.visible_page_items().enumerate() { + let is_header = matches!(item, SettingsPageItem::SectionHeader(_)); + let handle = this.content_handles[this.current_page_index()][actual_index] + .focus_handle(cx); + let mut offset = 1; // for page header + + if prev_was_header { + offset -= 1; + } + if handle.contains_focused(window, cx) { + let next_logical_index = logical_index + offset - 1; + this.list_state.scroll_to_reveal_item(next_logical_index); + // We need to render the next item to ensure it's focus handle is in the element tree + cx.on_next_frame(window, |_, window, cx| { + window.focus_prev(); + cx.notify(); + }); + cx.notify(); + return; + } + prev_was_header = is_header; + } + window.focus_prev(); + })) .child(page_header) .when(sub_page_stack().is_empty(), |this| { this.vertical_scrollbar_for(self.list_state.clone(), window, cx) @@ -2543,12 +2643,13 @@ impl SettingsWindow { 0 } - fn focus_content_element(&self, item_index: usize, window: &mut Window, cx: &mut App) { - if !sub_page_stack().is_empty() { - return; - } + fn focus_handle_for_content_element( + &self, + actual_item_index: usize, + cx: &Context, + ) -> FocusHandle { let page_index = self.current_page_index(); - window.focus(&self.content_handles[page_index][item_index].focus_handle(cx)); + self.content_handles[page_index][actual_item_index].focus_handle(cx) } fn focused_nav_entry(&self, window: &Window, cx: &App) -> Option { @@ -2609,9 +2710,10 @@ impl Render for SettingsWindow { { this.open_and_scroll_to_navbar_entry( this.navbar_entry, + None, + true, window, cx, - true, ); } else { this.focus_and_scroll_to_nav_entry(this.navbar_entry, window, cx);