Cargo.lock 🔗
@@ -14551,6 +14551,7 @@ dependencies = [
"menu",
"node_runtime",
"paths",
+ "pretty_assertions",
"project",
"serde",
"session",
settings_ui: Dynamic navbar filtering (#39494)
Ben Kunkle created
Closes #ISSUE
Release Notes:
- N/A *or* Added/Fixed/Improved ...Change summary
Cargo.lock | 1
crates/settings_ui/Cargo.toml | 1
crates/settings_ui/src/settings_ui.rs | 328 ++++++++++-----------
crates/ui/src/components/disclosure.rs | 12
crates/ui/src/components/list/list_header.rs | 3
crates/ui/src/components/list/list_item.rs | 5
crates/ui/src/components/tree_view_item.rs | 14
7 files changed, 183 insertions(+), 181 deletions(-)
Detailed changes
crates/settings_ui/Cargo.toml 🔗
@@ -46,6 +46,7 @@ paths.workspace = true
session.workspace = true
settings.workspace = true
zlog.workspace = true
+pretty_assertions.workspace = true
[[example]]
name = "ui"
crates/settings_ui/src/settings_ui.rs 🔗
@@ -144,7 +144,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
vec![
SettingsPage {
title: "General Page",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("General"),
SettingsPageItem::SettingItem(SettingItem {
@@ -303,7 +302,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Appearance & Behavior",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Theme"),
// todo(settings_ui): Figure out how we want to add these
@@ -663,7 +661,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Editor",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Indentation"),
// todo(settings_ui): Needs numeric stepper
@@ -1360,7 +1357,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Languages & Frameworks",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("General"),
SettingsPageItem::SettingItem(SettingItem {
@@ -1393,7 +1389,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Workbench & Window",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Workbench"),
SettingsPageItem::SettingItem(SettingItem {
@@ -1495,7 +1490,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Panels & Tools",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Project Panel"),
SettingsPageItem::SettingItem(SettingItem {
@@ -1700,7 +1694,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Version Control",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Git"),
SettingsPageItem::SettingItem(SettingItem {
@@ -1863,7 +1856,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "System & Network",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Network"),
// todo(settings_ui): Proxy needs a default
@@ -1903,7 +1895,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Diagnostics & Errors",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Display"),
SettingsPageItem::SettingItem(SettingItem {
@@ -2121,7 +2112,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Collaboration",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Calls"),
SettingsPageItem::SettingItem(SettingItem {
@@ -2201,7 +2191,6 @@ fn user_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "AI",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("General"),
SettingsPageItem::SettingItem(SettingItem {
@@ -2224,7 +2213,6 @@ fn project_settings_data() -> Vec<SettingsPage> {
vec![
SettingsPage {
title: "Project",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Worktree Settings Content"),
SettingsPageItem::SettingItem(SettingItem {
@@ -2244,7 +2232,6 @@ fn project_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Appearance & Behavior",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Guides"),
SettingsPageItem::SettingItem(SettingItem {
@@ -2316,7 +2303,6 @@ fn project_settings_data() -> Vec<SettingsPage> {
},
SettingsPage {
title: "Editing",
- expanded: false,
items: vec![
SettingsPageItem::SectionHeader("Indentation"),
// todo(settings_ui): Needs numeric stepper
@@ -2788,12 +2774,13 @@ struct SubPage {
struct NavBarEntry {
title: &'static str,
is_root: bool,
+ expanded: bool,
page_index: usize,
+ item_index: Option<usize>,
}
struct SettingsPage {
title: &'static str,
- expanded: bool,
items: Vec<SettingsPageItem>,
}
@@ -3058,66 +3045,80 @@ impl SettingsWindow {
let toggle_page_index = self.page_index_from_navbar_index(ix);
let selected_page_index = self.page_index_from_navbar_index(self.navbar_entry);
- let expanded = &mut self.page_for_navbar_index(ix).expanded;
+ let expanded = &mut self.navbar_entries[ix].expanded;
*expanded = !*expanded;
- let expanded = *expanded;
// if currently selected page is a child of the parent page we are folding,
// set the current page to the parent page
- if selected_page_index == toggle_page_index {
+ if !*expanded && selected_page_index == toggle_page_index {
self.navbar_entry = ix;
- } else if selected_page_index > toggle_page_index {
- let sub_items_count = self.pages[toggle_page_index]
- .items
- .iter()
- .filter(|item| matches!(item, SettingsPageItem::SectionHeader(_)))
- .count();
- if expanded {
- self.navbar_entry += sub_items_count;
- } else {
- self.navbar_entry -= sub_items_count;
- }
}
-
- self.build_navbar();
}
fn build_navbar(&mut self) {
let mut navbar_entries = Vec::with_capacity(self.navbar_entries.len());
for (page_index, page) in self.pages.iter().enumerate() {
- if !self.search_matches[page_index]
- .iter()
- .any(|is_match| *is_match)
- && !self.search_matches[page_index].is_empty()
- {
- continue;
- }
navbar_entries.push(NavBarEntry {
title: page.title,
is_root: true,
+ expanded: false,
page_index,
+ item_index: None,
});
- if !page.expanded {
- continue;
- }
for (item_index, item) in page.items.iter().enumerate() {
let SettingsPageItem::SectionHeader(title) = item else {
continue;
};
- if !self.search_matches[page_index][item_index] {
- continue;
- }
-
navbar_entries.push(NavBarEntry {
title,
is_root: false,
+ expanded: false,
page_index,
+ item_index: Some(item_index),
});
}
}
self.navbar_entries = navbar_entries;
}
+ fn visible_navbar_entries(&self) -> impl Iterator<Item = (usize, &NavBarEntry)> {
+ let mut index = 0;
+ let entries = &self.navbar_entries;
+ let search_matches = &self.search_matches;
+ std::iter::from_fn(move || {
+ while index < entries.len() {
+ let entry = &entries[index];
+ let included_in_search = if let Some(item_index) = entry.item_index {
+ search_matches[entry.page_index][item_index]
+ } else {
+ search_matches[entry.page_index].iter().any(|b| *b)
+ || search_matches[entry.page_index].is_empty()
+ };
+ if included_in_search {
+ break;
+ }
+ index += 1;
+ }
+ if index >= self.navbar_entries.len() {
+ return None;
+ }
+ let entry = &entries[index];
+ let entry_index = index;
+
+ index += 1;
+ if entry.is_root && !entry.expanded {
+ while index < entries.len() {
+ if entries[index].is_root {
+ break;
+ }
+ index += 1;
+ }
+ }
+
+ return Some((entry_index, entry));
+ })
+ }
+
fn update_matches(&mut self, cx: &mut Context<SettingsWindow>) {
self.search_task.take();
let query = self.search_bar.read(cx).text(cx);
@@ -3125,7 +3126,6 @@ impl SettingsWindow {
for page in &mut self.search_matches {
page.fill(true);
}
- self.build_navbar();
cx.notify();
return;
}
@@ -3169,7 +3169,7 @@ impl SettingsWindow {
candidates.as_slice(),
&query,
false,
- false,
+ true,
candidates.len(),
&atomic_bool,
cx.background_executor().clone(),
@@ -3191,21 +3191,29 @@ impl SettingsWindow {
page[header_index] = true;
page[item_index] = true;
}
- this.build_navbar();
- this.navbar_entry = 0;
+ let first_navbar_entry_index = this
+ .visible_navbar_entries()
+ .next()
+ .map(|e| e.0)
+ .unwrap_or(0);
+ this.navbar_entry = first_navbar_entry_index;
cx.notify();
})
.ok();
}));
}
- fn build_ui(&mut self, cx: &mut Context<SettingsWindow>) {
- self.pages = self.current_file.pages();
+ fn build_search_matches(&mut self) {
self.search_matches = self
.pages
.iter()
.map(|page| vec![true; page.items.len()])
.collect::<Vec<_>>();
+ }
+
+ fn build_ui(&mut self, cx: &mut Context<SettingsWindow>) {
+ self.pages = self.current_file.pages();
+ self.build_search_matches();
self.build_navbar();
if !self.search_bar.read(cx).is_empty(cx) {
@@ -3283,25 +3291,20 @@ impl SettingsWindow {
"settings-ui-nav-bar",
self.navbar_entries.len(),
cx.processor(|this, range: Range<usize>, _, cx| {
- range
- .into_iter()
- .map(|ix| {
- let entry = &this.navbar_entries[ix];
-
+ this.visible_navbar_entries()
+ .skip(range.start.saturating_sub(1))
+ .take(range.len())
+ .map(|(ix, entry)| {
TreeViewItem::new(("settings-ui-navbar-entry", ix), entry.title)
.root_item(entry.is_root)
.toggle_state(this.is_navbar_entry_selected(ix))
.when(entry.is_root, |item| {
- item.toggle(
- this.pages[this.page_index_from_navbar_index(ix)]
- .expanded,
- )
- .on_toggle(
- cx.listener(move |this, _, _, cx| {
+ item.expanded(entry.expanded).on_toggle(cx.listener(
+ move |this, _, _, cx| {
this.toggle_navbar_entry(ix);
cx.notify();
- }),
- )
+ },
+ ))
})
.on_click(cx.listener(move |this, _, _, cx| {
this.navbar_entry = ix;
@@ -3423,11 +3426,6 @@ impl SettingsWindow {
self.navbar_entries[index].page_index
}
- fn page_for_navbar_index(&mut self, index: usize) -> &mut SettingsPage {
- let index = self.page_index_from_navbar_index(index);
- &mut self.pages[index]
- }
-
fn is_navbar_entry_selected(&self, ix: usize) -> bool {
ix == self.navbar_entry
}
@@ -3626,10 +3624,6 @@ mod test {
use super::*;
impl SettingsWindow {
- fn navbar(&self) -> &[NavBarEntry] {
- self.navbar_entries.as_slice()
- }
-
fn navbar_entry(&self) -> usize {
self.navbar_entry
}
@@ -3642,6 +3636,7 @@ mod test {
}
fn build(mut self) -> Self {
+ self.build_search_matches();
self.build_navbar();
self
}
@@ -3653,7 +3648,6 @@ mod test {
) -> Self {
let page = SettingsPage {
title,
- expanded: false,
items: Vec::default(),
};
@@ -3671,13 +3665,25 @@ mod test {
fn assert_search_results(&self, other: &Self) {
// page index could be different because of filtered out pages
- assert!(
- self.navbar_entries
- .iter()
- .zip(other.navbar_entries.iter())
- .all(|(entry, other)| {
- entry.is_root == other.is_root && entry.title == other.title
+ #[derive(Debug, PartialEq)]
+ struct EntryMinimal {
+ is_root: bool,
+ title: &'static str,
+ }
+ pretty_assertions::assert_eq!(
+ other
+ .visible_navbar_entries()
+ .map(|(_, entry)| EntryMinimal {
+ is_root: entry.is_root,
+ title: entry.title,
+ })
+ .collect::<Vec<_>>(),
+ self.visible_navbar_entries()
+ .map(|(_, entry)| EntryMinimal {
+ is_root: entry.is_root,
+ title: entry.title,
})
+ .collect::<Vec<_>>(),
);
assert_eq!(
self.current_page().items.iter().collect::<Vec<_>>(),
@@ -3719,55 +3725,50 @@ mod test {
fn parse(input: &'static str, window: &mut Window, cx: &mut App) -> SettingsWindow {
let mut pages: Vec<SettingsPage> = Vec::new();
- let mut current_page = None;
+ let mut expanded_pages = Vec::new();
let mut selected_idx = None;
- let mut ix = 0;
- let mut in_closed_subentry = false;
+ let mut index = 0;
+ let mut in_expanded_section = false;
for mut line in input
.lines()
.map(|line| line.trim())
.filter(|line| !line.is_empty())
{
- let mut is_selected = false;
- if line.ends_with("*") {
- assert!(
- selected_idx.is_none(),
- "Can only have one selected navbar entry at a time"
- );
- selected_idx = Some(ix);
- line = &line[..line.len() - 1];
- is_selected = true;
+ if let Some(pre) = line.strip_suffix('*') {
+ assert!(selected_idx.is_none(), "Only one selected entry allowed");
+ selected_idx = Some(index);
+ line = pre;
}
-
- if line.starts_with("v") || line.starts_with(">") {
- if let Some(current_page) = current_page.take() {
- pages.push(current_page);
- }
-
- let expanded = line.starts_with("v");
- in_closed_subentry = !expanded;
- ix += 1;
-
- current_page = Some(SettingsPage {
- title: line.split_once(" ").unwrap().1,
- expanded,
- items: Vec::default(),
+ let (kind, title) = line.split_once(" ").unwrap();
+ assert_eq!(kind.len(), 1);
+ let kind = kind.chars().next().unwrap();
+ if kind == 'v' {
+ let page_idx = pages.len();
+ expanded_pages.push(page_idx);
+ pages.push(SettingsPage {
+ title,
+ items: vec![],
+ });
+ index += 1;
+ in_expanded_section = true;
+ } else if kind == '>' {
+ pages.push(SettingsPage {
+ title,
+ items: vec![],
});
- } else if line.starts_with("-") {
- if !in_closed_subentry {
- ix += 1;
- } else if is_selected && in_closed_subentry {
- panic!("Can't select sub entry if it's parent is closed");
+ index += 1;
+ in_expanded_section = false;
+ } else if kind == '-' {
+ pages
+ .last_mut()
+ .unwrap()
+ .items
+ .push(SettingsPageItem::SectionHeader(title));
+ if selected_idx == Some(index) && !in_expanded_section {
+ panic!("Items in unexpanded sections cannot be selected");
}
-
- let Some(current_page) = current_page.as_mut() else {
- panic!("Sub entries must be within a page");
- };
-
- current_page.items.push(SettingsPageItem::SectionHeader(
- line.split_once(" ").unwrap().1,
- ));
+ index += 1;
} else {
panic!(
"Entries must start with one of 'v', '>', or '-'\n line: {}",
@@ -3776,15 +3777,6 @@ mod test {
}
}
- if let Some(current_page) = current_page.take() {
- pages.push(current_page);
- }
-
- let search_matches = pages
- .iter()
- .map(|page| vec![true; page.items.len()])
- .collect::<Vec<_>>();
-
let mut settings_window = SettingsWindow {
files: Vec::default(),
current_file: crate::SettingsUiFile::User,
@@ -3793,43 +3785,70 @@ mod test {
navbar_entry: selected_idx.expect("Must have a selected navbar entry"),
navbar_entries: Vec::default(),
list_handle: UniformListScrollHandle::default(),
- search_matches,
+ search_matches: vec![],
search_task: None,
sub_page_stack: vec![],
};
+ settings_window.build_search_matches();
settings_window.build_navbar();
+ for expanded_page_index in expanded_pages {
+ for entry in &mut settings_window.navbar_entries {
+ if entry.page_index == expanded_page_index && entry.is_root {
+ entry.expanded = true;
+ }
+ }
+ }
settings_window
}
#[track_caller]
fn check_navbar_toggle(
before: &'static str,
- toggle_idx: usize,
+ toggle_page: &'static str,
after: &'static str,
window: &mut Window,
cx: &mut App,
) {
let mut settings_window = parse(before, window, cx);
+ let toggle_page_idx = settings_window
+ .pages
+ .iter()
+ .position(|page| page.title == toggle_page)
+ .expect("page not found");
+ let toggle_idx = settings_window
+ .navbar_entries
+ .iter()
+ .position(|entry| entry.page_index == toggle_page_idx)
+ .expect("page not found");
settings_window.toggle_navbar_entry(toggle_idx);
let expected_settings_window = parse(after, window, cx);
- assert_eq!(settings_window.navbar(), expected_settings_window.navbar());
- assert_eq!(
- settings_window.navbar_entry(),
- expected_settings_window.navbar_entry()
+ pretty_assertions::assert_eq!(
+ settings_window
+ .visible_navbar_entries()
+ .map(|(_, entry)| entry)
+ .collect::<Vec<_>>(),
+ expected_settings_window
+ .visible_navbar_entries()
+ .map(|(_, entry)| entry)
+ .collect::<Vec<_>>(),
+ );
+ pretty_assertions::assert_eq!(
+ settings_window.navbar_entries[settings_window.navbar_entry()],
+ expected_settings_window.navbar_entries[expected_settings_window.navbar_entry()],
);
}
macro_rules! check_navbar_toggle {
- ($name:ident, before: $before:expr, toggle_idx: $toggle_idx:expr, after: $after:expr) => {
+ ($name:ident, before: $before:expr, toggle_page: $toggle_page:expr, after: $after:expr) => {
#[gpui::test]
fn $name(cx: &mut gpui::TestAppContext) {
let window = cx.add_empty_window();
window.update(|window, cx| {
register_settings(cx);
- check_navbar_toggle($before, $toggle_idx, $after, window, cx);
+ check_navbar_toggle($before, $toggle_page, $after, window, cx);
});
}
};
@@ -3844,7 +3863,7 @@ mod test {
v Project
- Project Settings
",
- toggle_idx: 0,
+ toggle_page: "General",
after: r"
> General*
v Project
@@ -3861,7 +3880,7 @@ mod test {
v Project
- Project Settings
",
- toggle_idx: 0,
+ toggle_page: "General",
after: r"
v General*
- General
@@ -3880,7 +3899,7 @@ mod test {
v Project
- Project Settings*
",
- toggle_idx: 1,
+ toggle_page: "Project",
after: r"
> General
> Project*
@@ -3899,7 +3918,7 @@ mod test {
- General
> Appearance & Behavior
",
- toggle_idx: 3,
+ toggle_page: "Project",
after: r"
v General Page
- General
@@ -3923,7 +3942,7 @@ mod test {
- General*
> Appearance & Behavior
",
- toggle_idx: 0,
+ toggle_page: "General Page",
after: r"
> General Page
v Project
@@ -3946,7 +3965,7 @@ mod test {
- General*
> Appearance & Behavior
",
- toggle_idx: 0,
+ toggle_page: "General Page",
after: r"
v General Page
- General
@@ -3959,31 +3978,6 @@ mod test {
"
);
- check_navbar_toggle!(
- navbar_toggle_sub_entry_does_nothing,
- before: r"
- > General Page
- - General
- - Privacy
- v Project
- - Worktree Settings Content
- v AI
- - General*
- > Appearance & Behavior
- ",
- toggle_idx: 4,
- after: r"
- > General Page
- - General
- - Privacy
- v Project
- - Worktree Settings Content
- v AI
- - General*
- > Appearance & Behavior
- "
- );
-
#[gpui::test]
fn test_basic_search(cx: &mut gpui::TestAppContext) {
let cx = cx.add_empty_window();
crates/ui/src/components/disclosure.rs 🔗
@@ -10,7 +10,7 @@ pub struct Disclosure {
is_open: bool,
selected: bool,
disabled: bool,
- on_toggle: Option<Arc<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>,
+ on_toggle_expanded: Option<Arc<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>,
cursor_style: CursorStyle,
opened_icon: IconName,
closed_icon: IconName,
@@ -24,7 +24,7 @@ impl Disclosure {
is_open,
selected: false,
disabled: false,
- on_toggle: None,
+ on_toggle_expanded: None,
cursor_style: CursorStyle::PointingHand,
opened_icon: IconName::ChevronDown,
closed_icon: IconName::ChevronRight,
@@ -32,11 +32,11 @@ impl Disclosure {
}
}
- pub fn on_toggle(
+ pub fn on_toggle_expanded(
mut self,
handler: impl Into<Option<Arc<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>>,
) -> Self {
- self.on_toggle = handler.into();
+ self.on_toggle_expanded = handler.into();
self
}
@@ -65,7 +65,7 @@ impl Toggleable for Disclosure {
impl Clickable for Disclosure {
fn on_click(mut self, handler: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static) -> Self {
- self.on_toggle = Some(Arc::new(handler));
+ self.on_toggle_expanded = Some(Arc::new(handler));
self
}
@@ -99,7 +99,7 @@ impl RenderOnce for Disclosure {
.when_some(self.visible_on_hover.clone(), |this, group_name| {
this.visible_on_hover(group_name)
})
- .when_some(self.on_toggle, move |this, on_toggle| {
+ .when_some(self.on_toggle_expanded, move |this, on_toggle| {
this.on_click(move |event, window, cx| on_toggle(event, window, cx))
})
}
crates/ui/src/components/list/list_header.rs 🔗
@@ -107,7 +107,8 @@ impl RenderOnce for ListHeader {
h_flex()
.gap(DynamicSpacing::Base04.rems(cx))
.children(self.toggle.map(|is_open| {
- Disclosure::new("toggle", is_open).on_toggle(self.on_toggle.clone())
+ Disclosure::new("toggle", is_open)
+ .on_toggle_expanded(self.on_toggle.clone())
}))
.child(
div()
crates/ui/src/components/list/list_item.rs 🔗
@@ -308,7 +308,10 @@ impl RenderOnce for ListItem {
.when(is_open && !self.always_show_disclosure_icon, |this| {
this.visible_on_hover("")
})
- .child(Disclosure::new("toggle", is_open).on_toggle(self.on_toggle))
+ .child(
+ Disclosure::new("toggle", is_open)
+ .on_toggle_expanded(self.on_toggle),
+ )
}))
.child(
h_flex()
crates/ui/src/components/tree_view_item.rs 🔗
@@ -9,7 +9,7 @@ pub struct TreeViewItem {
id: ElementId,
group_name: Option<SharedString>,
label: SharedString,
- toggle: bool,
+ expanded: bool,
selected: bool,
disabled: bool,
focused: bool,
@@ -28,7 +28,7 @@ impl TreeViewItem {
id: id.into(),
group_name: None,
label: label.into(),
- toggle: false,
+ expanded: false,
selected: false,
disabled: false,
focused: false,
@@ -73,8 +73,8 @@ impl TreeViewItem {
self
}
- pub fn toggle(mut self, toggle: bool) -> Self {
- self.toggle = toggle;
+ pub fn expanded(mut self, toggle: bool) -> Self {
+ self.expanded = toggle;
self
}
@@ -161,10 +161,12 @@ impl RenderOnce for TreeViewItem {
})
.hover(|s| s.bg(cx.theme().colors().element_hover))
.child(
- Disclosure::new("toggle", self.toggle)
+ Disclosure::new("toggle", self.expanded)
.when_some(
self.on_toggle.clone(),
- |disclosure, on_toggle| disclosure.on_toggle(on_toggle),
+ |disclosure, on_toggle| {
+ disclosure.on_toggle_expanded(on_toggle)
+ },
)
.opened_icon(IconName::ChevronDown)
.closed_icon(IconName::ChevronRight),