settings_ui: Focus content controls when opened from nav bar (#39792)

Ben Kunkle created

Closes #ISSUE

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

crates/settings_ui/src/page_data.rs   |  45 +++
crates/settings_ui/src/settings_ui.rs | 311 +++++++++++++++++++---------
2 files changed, 245 insertions(+), 111 deletions(-)

Detailed changes

crates/settings_ui/src/page_data.rs 🔗

@@ -1624,40 +1624,65 @@ pub(crate) fn settings_data() -> Vec<SettingsPage> {
                     title: "JSON",
                     files: USER | LOCAL,
                     render: Arc::new(|this, window, cx| {
-                        this.render_page_items(language_settings_data().iter(), window, cx)
-                            .into_any_element()
+                        this.render_page_items(
+                            language_settings_data().iter().enumerate(),
+                            None,
+                            window,
+                            cx,
+                        )
+                        .into_any_element()
                     }),
                 }),
                 SettingsPageItem::SubPageLink(SubPageLink {
                     title: "JSONC",
                     files: USER | LOCAL,
                     render: Arc::new(|this, window, cx| {
-                        this.render_page_items(language_settings_data().iter(), window, cx)
-                            .into_any_element()
+                        this.render_page_items(
+                            language_settings_data().iter().enumerate(),
+                            None,
+                            window,
+                            cx,
+                        )
+                        .into_any_element()
                     }),
                 }),
                 SettingsPageItem::SubPageLink(SubPageLink {
                     title: "Rust",
                     files: USER | LOCAL,
                     render: Arc::new(|this, window, cx| {
-                        this.render_page_items(language_settings_data().iter(), window, cx)
-                            .into_any_element()
+                        this.render_page_items(
+                            language_settings_data().iter().enumerate(),
+                            None,
+                            window,
+                            cx,
+                        )
+                        .into_any_element()
                     }),
                 }),
                 SettingsPageItem::SubPageLink(SubPageLink {
                     title: "Python",
                     files: USER | LOCAL,
                     render: Arc::new(|this, window, cx| {
-                        this.render_page_items(language_settings_data().iter(), window, cx)
-                            .into_any_element()
+                        this.render_page_items(
+                            language_settings_data().iter().enumerate(),
+                            None,
+                            window,
+                            cx,
+                        )
+                        .into_any_element()
                     }),
                 }),
                 SettingsPageItem::SubPageLink(SubPageLink {
                     title: "TSX",
                     files: USER | LOCAL,
                     render: Arc::new(|this, window, cx| {
-                        this.render_page_items(language_settings_data().iter(), window, cx)
-                            .into_any_element()
+                        this.render_page_items(
+                            language_settings_data().iter().enumerate(),
+                            None,
+                            window,
+                            cx,
+                        )
+                        .into_any_element()
                     }),
                 }),
             ],

crates/settings_ui/src/settings_ui.rs 🔗

@@ -8,8 +8,9 @@ use feature_flags::FeatureFlag;
 use fuzzy::StringMatchCandidate;
 use gpui::{
     Action, App, Div, Entity, FocusHandle, Focusable, FontWeight, Global, ReadGlobal as _,
-    ScrollHandle, Task, TitlebarOptions, UniformListScrollHandle, Window, WindowBounds,
-    WindowHandle, WindowOptions, actions, div, point, prelude::*, px, size, uniform_list,
+    ScrollHandle, Subscription, Task, TitlebarOptions, UniformListScrollHandle, Window,
+    WindowBounds, WindowHandle, WindowOptions, actions, div, point, prelude::*, px, size,
+    uniform_list,
 };
 use heck::ToTitleCase as _;
 use project::WorktreeId;
@@ -195,6 +196,38 @@ impl SettingFieldRenderer {
     }
 }
 
+struct NonFocusableHandle {
+    handle: FocusHandle,
+    _subscription: Subscription,
+}
+
+impl NonFocusableHandle {
+    fn new(tab_index: isize, tab_stop: bool, window: &mut Window, cx: &mut App) -> Entity<Self> {
+        let handle = cx.focus_handle().tab_index(tab_index).tab_stop(tab_stop);
+        Self::from_handle(handle, window, cx)
+    }
+
+    fn from_handle(handle: FocusHandle, window: &mut Window, cx: &mut App) -> Entity<Self> {
+        cx.new(|cx| {
+            let _subscription = cx.on_focus(&handle, window, {
+                move |_, window, _| {
+                    window.focus_next();
+                }
+            });
+            Self {
+                handle,
+                _subscription,
+            }
+        })
+    }
+}
+
+impl Focusable for NonFocusableHandle {
+    fn focus_handle(&self, _: &App) -> FocusHandle {
+        self.handle.clone()
+    }
+}
+
 struct SettingsFieldMetadata {
     placeholder: Option<&'static str>,
 }
@@ -226,6 +259,7 @@ fn init_renderers(cx: &mut App) {
             Button::new("open-in-settings-file", "Edit in settings.json")
                 .size(ButtonSize::Default)
                 .style(ButtonStyle::Outlined)
+                .tab_index(0_isize)
                 .on_click(|_, window, cx| {
                     window.dispatch_action(Box::new(OpenCurrentFile), cx);
                 })
@@ -495,10 +529,11 @@ pub struct SettingsWindow {
     navbar_entries: Vec<NavBarEntry>,
     list_handle: UniformListScrollHandle,
     search_matches: Vec<Vec<bool>>,
+    content_handles: Vec<Vec<Entity<NonFocusableHandle>>>,
     scroll_handle: ScrollHandle,
     focus_handle: FocusHandle,
-    navbar_focus_handle: FocusHandle,
-    content_focus_handle: FocusHandle,
+    navbar_focus_handle: Entity<NonFocusableHandle>,
+    content_focus_handle: Entity<NonFocusableHandle>,
     files_focus_handle: FocusHandle,
 }
 
@@ -839,8 +874,8 @@ impl SettingsWindow {
         })
         .detach();
 
-        cx.observe_global_in::<SettingsStore>(window, move |this, _, cx| {
-            this.fetch_files(cx);
+        cx.observe_global_in::<SettingsStore>(window, move |this, window, cx| {
+            this.fetch_files(window, cx);
             cx.notify();
         })
         .detach();
@@ -857,24 +892,29 @@ impl SettingsWindow {
             search_bar,
             search_task: None,
             search_matches: vec![],
+            content_handles: vec![],
             scroll_handle: ScrollHandle::new(),
             focus_handle: cx.focus_handle(),
-            navbar_focus_handle: cx
-                .focus_handle()
-                .tab_index(NAVBAR_CONTAINER_TAB_INDEX)
-                .tab_stop(false),
-            content_focus_handle: cx
-                .focus_handle()
-                .tab_index(CONTENT_CONTAINER_TAB_INDEX)
-                .tab_stop(false),
+            navbar_focus_handle: NonFocusableHandle::new(
+                NAVBAR_CONTAINER_TAB_INDEX,
+                false,
+                window,
+                cx,
+            ),
+            content_focus_handle: NonFocusableHandle::new(
+                CONTENT_CONTAINER_TAB_INDEX,
+                false,
+                window,
+                cx,
+            ),
             files_focus_handle: cx
                 .focus_handle()
                 .tab_index(HEADER_CONTAINER_TAB_INDEX)
                 .tab_stop(false),
         };
 
-        this.fetch_files(cx);
-        this.build_ui(cx);
+        this.fetch_files(window, cx);
+        this.build_ui(window, cx);
 
         this.search_bar.update(cx, |editor, cx| {
             editor.focus_handle(cx).focus(window);
@@ -898,6 +938,7 @@ impl SettingsWindow {
         // set the current page to the parent page
         if !*expanded && selected_page_index == toggle_page_index {
             self.navbar_entry = ix;
+            // note: not opening page. Toggling does not change content just selected page
         }
     }
 
@@ -956,13 +997,13 @@ impl SettingsWindow {
             };
             let key = (root_entry, sub_entry_title);
             if Some(key) == prev_selected_entry {
-                self.navbar_entry = index;
+                self.open_nav_page(index);
                 found_nav_entry = true;
             }
             entry.expanded = *prev_navbar_state.get(&key).unwrap_or(&false);
         }
         if !found_nav_entry {
-            self.navbar_entry = 0;
+            self.open_first_nav_page();
         }
         self.navbar_entries = navbar_entries;
     }
@@ -1118,12 +1159,7 @@ impl SettingsWindow {
                     page[item_index] = true;
                 }
                 this.filter_matches_to_file();
-                let first_navbar_entry_index = this
-                    .visible_navbar_entries()
-                    .next()
-                    .map(|e| e.0)
-                    .unwrap_or(0);
-                this.navbar_entry = first_navbar_entry_index;
+                this.open_first_nav_page();
                 cx.notify();
             })
             .ok();
@@ -1138,10 +1174,24 @@ impl SettingsWindow {
             .collect::<Vec<_>>();
     }
 
-    fn build_ui(&mut self, cx: &mut Context<SettingsWindow>) {
+    fn build_content_handles(&mut self, window: &mut Window, cx: &mut Context<SettingsWindow>) {
+        self.content_handles = self
+            .pages
+            .iter()
+            .map(|page| {
+                std::iter::repeat_with(|| NonFocusableHandle::new(0, false, window, cx))
+                    .take(page.items.len())
+                    .collect()
+            })
+            .collect::<Vec<_>>();
+    }
+
+    fn build_ui(&mut self, window: &mut Window, cx: &mut Context<SettingsWindow>) {
         if self.pages.is_empty() {
             self.pages = page_data::settings_data();
         }
+        sub_page_stack_mut().clear();
+        self.build_content_handles(window, cx);
         self.build_search_matches();
         self.build_navbar();
 
@@ -1150,7 +1200,7 @@ impl SettingsWindow {
         cx.notify();
     }
 
-    fn fetch_files(&mut self, cx: &mut Context<SettingsWindow>) {
+    fn fetch_files(&mut self, window: &mut Window, cx: &mut Context<SettingsWindow>) {
         self.worktree_root_dirs.clear();
         let prev_files = self.files.clone();
         let settings_store = cx.global::<SettingsStore>();
@@ -1200,29 +1250,38 @@ impl SettingsWindow {
             .iter()
             .any(|(file, _)| file == &self.current_file);
         if !current_file_still_exists {
-            self.change_file(0, cx);
+            self.change_file(0, window, cx);
         }
     }
 
-    fn change_file(&mut self, ix: usize, cx: &mut Context<SettingsWindow>) {
+    fn open_nav_page(&mut self, navbar_entry: usize) {
+        self.navbar_entry = navbar_entry;
+        sub_page_stack_mut().clear();
+    }
+
+    fn open_first_nav_page(&mut self) {
+        let first_navbar_entry_index = self
+            .visible_navbar_entries()
+            .next()
+            .map(|e| e.0)
+            .unwrap_or(0);
+        self.open_nav_page(first_navbar_entry_index);
+    }
+
+    fn change_file(&mut self, ix: usize, window: &mut Window, cx: &mut Context<SettingsWindow>) {
         if ix >= self.files.len() {
             self.current_file = SettingsUiFile::User;
-            self.build_ui(cx);
+            self.build_ui(window, cx);
             return;
         }
         if self.files[ix].0 == self.current_file {
             return;
         }
         self.current_file = self.files[ix].0.clone();
-        self.navbar_entry = 0;
-        self.build_ui(cx);
+        self.open_nav_page(0);
+        self.build_ui(window, cx);
 
-        let first_navbar_entry_index = self
-            .visible_navbar_entries()
-            .next()
-            .map(|e| e.0)
-            .unwrap_or(0);
-        self.navbar_entry = first_navbar_entry_index;
+        self.open_first_nav_page();
     }
 
     fn render_files_header(
@@ -1259,7 +1318,7 @@ impl SettingsWindow {
                                 .on_click(cx.listener({
                                     let focus_handle = focus_handle.clone();
                                     move |this, _: &gpui::ClickEvent, window, cx| {
-                                        this.change_file(ix, cx);
+                                        this.change_file(ix, window, cx);
                                         focus_handle.focus(window);
                                     }
                                 }))
@@ -1360,8 +1419,8 @@ impl SettingsWindow {
             .child(self.render_search(window, cx))
             .child(
                 v_flex()
-                    .size_full()
-                    .track_focus(&self.navbar_focus_handle)
+                    .flex_grow()
+                    .track_focus(&self.navbar_focus_handle.focus_handle(cx))
                     .tab_group()
                     .tab_index(NAVBAR_GROUP_TAB_INDEX)
                     .child(
@@ -1369,9 +1428,9 @@ impl SettingsWindow {
                             "settings-ui-nav-bar",
                             visible_count,
                             cx.processor(move |this, range: Range<usize>, _, cx| {
-                                let entries: Vec<_> = this.visible_navbar_entries().collect();
-                                range
-                                    .filter_map(|ix| entries.get(ix).copied())
+                                this.visible_navbar_entries()
+                                    .skip(range.start.saturating_sub(1))
+                                    .take(range.len())
                                     .map(|(ix, entry)| {
                                         TreeViewItem::new(
                                             ("settings-ui-navbar-entry", ix),
@@ -1388,40 +1447,51 @@ impl SettingsWindow {
                                                 },
                                             ))
                                         })
-                                        .on_click(cx.listener(move |this, _, _, cx| {
-                                            this.navbar_entry = ix;
-
-                                            if !this.navbar_entries[ix].is_root {
-                                                let mut selected_page_ix = ix;
-
-                                                while !this.navbar_entries[selected_page_ix].is_root
-                                                {
-                                                    selected_page_ix -= 1;
-                                                }
-
-                                                let section_header = ix - selected_page_ix;
-
-                                                if let Some(section_index) = this
-                                                    .page_items()
-                                                    .enumerate()
-                                                    .filter(|item| {
-                                                        matches!(
-                                                            item.1,
-                                                            SettingsPageItem::SectionHeader(_)
-                                                        )
-                                                    })
-                                                    .take(section_header)
-                                                    .last()
-                                                    .map(|pair| pair.0)
-                                                {
-                                                    this.scroll_handle
-                                                        .scroll_to_top_of_item(section_index);
-                                                }
-                                            }
-
-                                            cx.notify();
-                                        }))
-                                        .into_any_element()
+                                        .on_click(
+                                            cx.listener(
+                                                move |this, evt: &gpui::ClickEvent, window, cx| {
+                                                    if !this.navbar_entries[ix].is_root {
+                                                        this.open_nav_page(ix);
+                                                        let mut selected_page_ix = ix;
+
+                                                        while !this.navbar_entries[selected_page_ix]
+                                                            .is_root
+                                                        {
+                                                            selected_page_ix -= 1;
+                                                        }
+
+                                                        let section_header = ix - selected_page_ix;
+
+                                                        if let Some(section_index) =
+                                                            this.page_items()
+                                                                .enumerate()
+                                                                .filter(|(_, (_, item))| {
+                                                                    matches!(
+                                                                 item,
+                                                                 SettingsPageItem::SectionHeader(_)
+                                                             )
+                                                                })
+                                                                .take(section_header)
+                                                                .last()
+                                                                .map(|(index, _)| index)
+                                                        {
+                                                            this.scroll_handle
+                                                                .scroll_to_top_of_item(
+                                                                    section_index,
+                                                                );
+                                                            this.focus_content_element(
+                                                                section_index,
+                                                                window,
+                                                                cx,
+                                                            );
+                                                        }
+                                                    } else if !evt.is_keyboard() {
+                                                        this.open_nav_page(ix);
+                                                    }
+                                                    cx.notify();
+                                                },
+                                            ),
+                                        )
                                     })
                                     .collect()
                             }),
@@ -1453,18 +1523,18 @@ impl SettingsWindow {
     }
 
     fn focus_first_nav_item(&self, window: &mut Window, cx: &mut Context<Self>) {
-        self.navbar_focus_handle.focus(window);
+        self.navbar_focus_handle.focus_handle(cx).focus(window);
         window.focus_next();
         cx.notify();
     }
 
     fn focus_first_content_item(&self, window: &mut Window, cx: &mut Context<Self>) {
-        self.content_focus_handle.focus(window);
+        self.content_focus_handle.focus_handle(cx).focus(window);
         window.focus_next();
         cx.notify();
     }
 
-    fn page_items(&self) -> impl Iterator<Item = &SettingsPageItem> {
+    fn page_items(&self) -> impl Iterator<Item = (usize, &SettingsPageItem)> {
         let page_idx = self.current_page_index();
 
         self.current_page()
@@ -1472,7 +1542,7 @@ impl SettingsWindow {
             .iter()
             .enumerate()
             .filter_map(move |(item_index, item)| {
-                self.search_matches[page_idx][item_index].then_some(item)
+                self.search_matches[page_idx][item_index].then_some((item_index, item))
             })
     }
 
@@ -1497,9 +1567,10 @@ impl SettingsWindow {
             .child(Label::new(last))
     }
 
-    fn render_page_items<'a, Items: Iterator<Item = &'a SettingsPageItem>>(
+    fn render_page_items<'a, Items: Iterator<Item = (usize, &'a SettingsPageItem)>>(
         &self,
         items: Items,
+        page_index: Option<usize>,
         window: &mut Window,
         cx: &mut Context<SettingsWindow>,
     ) -> impl IntoElement {
@@ -1538,28 +1609,38 @@ impl SettingsWindow {
                 .iter()
                 .enumerate()
                 .rev()
-                .find(|(_, item)| !matches!(item, SettingsPageItem::SectionHeader(_)))
+                .find(|(_, (_, item))| !matches!(item, SettingsPageItem::SectionHeader(_)))
                 .map(|(index, _)| index);
 
-            page_content =
-                page_content.children(items.clone().into_iter().enumerate().map(|(index, item)| {
+            page_content = page_content.children(items.clone().into_iter().enumerate().map(
+                |(index, (actual_item_index, item))| {
                     let no_bottom_border = items
                         .get(index + 1)
-                        .map(|next_item| matches!(next_item, SettingsPageItem::SectionHeader(_)))
+                        .map(|(_, next_item)| {
+                            matches!(next_item, SettingsPageItem::SectionHeader(_))
+                        })
                         .unwrap_or(false);
                     let is_last = Some(index) == last_non_header_index;
 
                     if let SettingsPageItem::SectionHeader(header) = item {
                         section_header = Some(*header);
                     }
-                    item.render(
-                        self,
-                        section_header.expect("All items rendered after a section header"),
-                        no_bottom_border || is_last,
-                        window,
-                        cx,
-                    )
-                }))
+                    div()
+                        .when_some(page_index, |element, page_index| {
+                            element.track_focus(
+                                &self.content_handles[page_index][actual_item_index]
+                                    .focus_handle(cx),
+                            )
+                        })
+                        .child(item.render(
+                            self,
+                            section_header.expect("All items rendered after a section header"),
+                            no_bottom_border || is_last,
+                            window,
+                            cx,
+                        ))
+                },
+            ))
         }
         page_content
     }
@@ -1576,7 +1657,12 @@ impl SettingsWindow {
             page_header = self.render_files_header(window, cx).into_any_element();
 
             page_content = self
-                .render_page_items(self.page_items(), window, cx)
+                .render_page_items(
+                    self.page_items(),
+                    Some(self.current_page_index()),
+                    window,
+                    cx,
+                )
                 .into_any_element();
         } else {
             page_header = h_flex()
@@ -1603,14 +1689,14 @@ impl SettingsWindow {
             .pb_6()
             .px_6()
             .gap_4()
-            .track_focus(&self.content_focus_handle)
+            .track_focus(&self.content_focus_handle.focus_handle(cx))
             .bg(cx.theme().colors().editor_background)
             .vertical_scrollbar_for(self.scroll_handle.clone(), window, cx)
             .child(page_header)
             .child(
                 div()
                     .size_full()
-                    .track_focus(&self.content_focus_handle)
+                    .track_focus(&self.content_focus_handle.focus_handle(cx))
                     .tab_group()
                     .tab_index(CONTENT_GROUP_TAB_INDEX)
                     .child(page_content),
@@ -1786,6 +1872,14 @@ impl SettingsWindow {
         }
         0
     }
+
+    fn focus_content_element(&self, item_index: usize, window: &mut Window, cx: &mut App) {
+        if !sub_page_stack().is_empty() {
+            return;
+        }
+        let page_index = self.current_page_index();
+        window.focus(&self.content_handles[page_index][item_index].focus_handle(cx));
+    }
 }
 
 impl Render for SettingsWindow {
@@ -1806,7 +1900,11 @@ impl Render for SettingsWindow {
                 this.search_bar.focus_handle(cx).focus(window);
             }))
             .on_action(cx.listener(|this, _: &ToggleFocusNav, window, cx| {
-                if this.navbar_focus_handle.contains_focused(window, cx) {
+                if this
+                    .navbar_focus_handle
+                    .focus_handle(cx)
+                    .contains_focused(window, cx)
+                {
                     this.focus_first_content_item(window, cx);
                 } else {
                     this.focus_first_nav_item(window, cx);
@@ -2143,7 +2241,7 @@ mod test {
             );
             assert_eq!(
                 self.current_page().items.iter().collect::<Vec<_>>(),
-                other.page_items().collect::<Vec<_>>()
+                other.page_items().map(|(_, item)| item).collect::<Vec<_>>()
             );
         }
     }
@@ -2245,11 +2343,22 @@ mod test {
             navbar_entries: Vec::default(),
             list_handle: UniformListScrollHandle::default(),
             search_matches: vec![],
+            content_handles: vec![],
             search_task: None,
             scroll_handle: ScrollHandle::new(),
             focus_handle: cx.focus_handle(),
-            navbar_focus_handle: cx.focus_handle(),
-            content_focus_handle: cx.focus_handle(),
+            navbar_focus_handle: NonFocusableHandle::new(
+                NAVBAR_CONTAINER_TAB_INDEX,
+                false,
+                window,
+                cx,
+            ),
+            content_focus_handle: NonFocusableHandle::new(
+                CONTENT_CONTAINER_TAB_INDEX,
+                false,
+                window,
+                cx,
+            ),
             files_focus_handle: cx.focus_handle(),
         };