diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index e094992ad815522ddb86ae1de127dbcfaa9ebf1b..e902232adce83edc0dc7f61374032ba706204752 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -1624,40 +1624,65 @@ pub(crate) fn settings_data() -> Vec { 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() }), }), ], diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 11a520e77f7b92812cb5d7c57f4d632f345e5e1c..5216e8a3adc696e265c0a0f14da881445f2f385a 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/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 { + 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 { + 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, list_handle: UniformListScrollHandle, search_matches: Vec>, + content_handles: Vec>>, scroll_handle: ScrollHandle, focus_handle: FocusHandle, - navbar_focus_handle: FocusHandle, - content_focus_handle: FocusHandle, + navbar_focus_handle: Entity, + content_focus_handle: Entity, files_focus_handle: FocusHandle, } @@ -839,8 +874,8 @@ impl SettingsWindow { }) .detach(); - cx.observe_global_in::(window, move |this, _, cx| { - this.fetch_files(cx); + cx.observe_global_in::(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::>(); } - fn build_ui(&mut self, cx: &mut Context) { + fn build_content_handles(&mut self, window: &mut Window, cx: &mut Context) { + self.content_handles = self + .pages + .iter() + .map(|page| { + std::iter::repeat_with(|| NonFocusableHandle::new(0, false, window, cx)) + .take(page.items.len()) + .collect() + }) + .collect::>(); + } + + fn build_ui(&mut self, window: &mut Window, cx: &mut Context) { 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) { + fn fetch_files(&mut self, window: &mut Window, cx: &mut Context) { self.worktree_root_dirs.clear(); let prev_files = self.files.clone(); let settings_store = cx.global::(); @@ -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) { + 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) { 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, _, 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.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.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 { + fn page_items(&self) -> impl Iterator { 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>( + fn render_page_items<'a, Items: Iterator>( &self, items: Items, + page_index: Option, window: &mut Window, cx: &mut Context, ) -> 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::>(), - other.page_items().collect::>() + other.page_items().map(|(_, item)| item).collect::>() ); } } @@ -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(), };