settings_ui: Fix focus bugs (#40806)

Ben Kunkle created 1 week ago

Closes #40608


Release Notes:

- settings_ui: Fixed an issue where tabbing to the nav bar from the
search bar while the nav bar was scrolled would result in the first
_visible_ nav entry being selected, instead of the literal first nav
entry
- settings_ui: Fixed an issue where scrolling the selected nav entry off
screen would cause the keyboard shortcut hint for the focus nav / focus
content binding to dissapear
- settings_ui: Fixed an issue where text input controls could not be
focused via the keyboard

Change summary

crates/settings_ui/src/components.rs  |  9 ++++---
crates/settings_ui/src/settings_ui.rs | 36 +++++++++++++++++++++++++---
2 files changed, 37 insertions(+), 8 deletions(-)

Detailed changes

crates/settings_ui/src/components.rs

@@ -62,10 +62,6 @@ impl RenderOnce for SettingsEditor {
             }
         });
 
-        if let Some(tab_index) = self.tab_index {
-            editor.focus_handle(cx).tab_index(tab_index);
-        }
-
         let weak_editor = editor.downgrade();
 
         let theme_colors = cx.theme().colors();
@@ -78,6 +74,11 @@ impl RenderOnce for SettingsEditor {
             .border_1()
             .border_color(theme_colors.border)
             .bg(theme_colors.editor_background)
+            .when_some(self.tab_index, |this, tab_index| {
+                let focus_handle = editor.focus_handle(cx).tab_index(tab_index).tab_stop(true);
+                this.track_focus(&focus_handle)
+                    .focus(|s| s.border_color(theme_colors.border_focused))
+            })
             .child(editor)
             .when_some(self.confirm, |this, confirm| {
                 this.on_action::<menu::Confirm>({

crates/settings_ui/src/settings_ui.rs

@@ -1821,6 +1821,9 @@ impl SettingsWindow {
             .read(cx)
             .handle
             .contains_focused(window, cx)
+            || self
+                .visible_navbar_entries()
+                .any(|(_, entry)| entry.focus_handle.is_focused(window))
         {
             "Focus Content"
         } else {
@@ -2020,7 +2023,13 @@ impl SettingsWindow {
                     .border_t_1()
                     .border_color(cx.theme().colors().border_variant)
                     .children(
-                        KeyBinding::for_action(&ToggleFocusNav, window, cx).map(|this| {
+                        KeyBinding::for_action_in(
+                            &ToggleFocusNav,
+                            &self.navbar_focus_handle.focus_handle(cx),
+                            window,
+                            cx,
+                        )
+                        .map(|this| {
                             KeybindingHint::new(
                                 this,
                                 cx.theme().colors().surface_background.opacity(0.5),
@@ -2123,6 +2132,17 @@ impl SettingsWindow {
             .any(|(index, _)| index == nav_entry_index)
     }
 
+    fn focus_and_scroll_to_first_visible_nav_entry(
+        &self,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        if let Some(nav_entry_index) = self.visible_navbar_entries().next().map(|(index, _)| index)
+        {
+            self.focus_and_scroll_to_nav_entry(nav_entry_index, window, cx);
+        }
+    }
+
     fn focus_and_scroll_to_nav_entry(
         &self,
         nav_entry_index: usize,
@@ -2735,9 +2755,17 @@ impl Render for SettingsWindow {
                             let prev_index = this.focused_file_index(window, cx).saturating_sub(1);
                             this.focus_file_at_index(prev_index, window);
                         }))
-                        .on_action(|_: &menu::SelectNext, window, _| {
-                            window.focus_next();
-                        })
+                        .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| {
+                            if this
+                                .search_bar
+                                .focus_handle(cx)
+                                .contains_focused(window, cx)
+                            {
+                                this.focus_and_scroll_to_first_visible_nav_entry(window, cx);
+                            } else {
+                                window.focus_next();
+                            }
+                        }))
                         .on_action(|_: &menu::SelectPrevious, window, _| {
                             window.focus_prev();
                         })