settings ui: Add more UX improvements (#39700)

Danilo Leal created

Release Notes:

- N/A

Change summary

assets/keymaps/default-linux.json              |   1 
assets/keymaps/default-macos.json              |   1 
assets/keymaps/default-windows.json            |   1 
crates/settings_ui/src/settings_ui.rs          | 180 +++++++++++--------
crates/ui/src/components/button/button.rs      |   1 
crates/ui/src/components/button/button_like.rs |   8 
crates/ui/src/components/keybinding_hint.rs    |  17 +
crates/ui/src/components/tree_view_item.rs     |  11 
crates/ui_input/src/font_picker.rs             |  11 
crates/ui_input/src/numeric_stepper.rs         |  58 +++---
crates/zed/src/zed/app_menus.rs                |  18 +
11 files changed, 182 insertions(+), 125 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -1249,6 +1249,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "ctrl-w": "workspace::CloseWindow",
+      "ctrl-m": "settings_editor::Minimize",
       "ctrl-f": "search::FocusSearch",
       "ctrl-shift-e": "settings_editor::ToggleFocusNav",
       // todo(settings_ui): cut this down based on the max files and overflow UI

assets/keymaps/default-macos.json 🔗

@@ -1354,6 +1354,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "cmd-w": "workspace::CloseWindow",
+      "cmd-m": "settings_editor::Minimize",
       "cmd-f": "search::FocusSearch",
       "cmd-shift-e": "settings_editor::ToggleFocusNav",
       // todo(settings_ui): cut this down based on the max files and overflow UI

assets/keymaps/default-windows.json 🔗

@@ -1270,6 +1270,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "ctrl-w": "workspace::CloseWindow",
+      "ctrl-m": "settings_editor::Minimize",
       "ctrl-f": "search::FocusSearch",
       "ctrl-shift-e": "settings_editor::ToggleFocusNav",
       // todo(settings_ui): cut this down based on the max files and overflow UI

crates/settings_ui/src/settings_ui.rs 🔗

@@ -29,8 +29,8 @@ use std::{
     sync::{Arc, LazyLock, RwLock, atomic::AtomicBool},
 };
 use ui::{
-    ButtonLike, ContextMenu, Divider, DropdownMenu, DropdownStyle, IconButtonShape,
-    KeybindingPosition, PopoverMenu, Switch, SwitchColor, TreeViewItem, WithScrollbar, prelude::*,
+    ContextMenu, Divider, DropdownMenu, DropdownStyle, IconButtonShape, KeyBinding, KeybindingHint,
+    PopoverMenu, Switch, SwitchColor, TreeViewItem, WithScrollbar, prelude::*,
 };
 use ui_input::{NumericStepper, NumericStepperStyle, NumericStepperType};
 use util::{ResultExt as _, paths::PathStyle, rel_path::RelPath};
@@ -46,6 +46,8 @@ const CONTENT_GROUP_TAB_INDEX: isize = 3;
 actions!(
     settings_editor,
     [
+        /// Minimizes the settings UI window.
+        Minimize,
         /// Toggles focus between the navbar and the main content.
         ToggleFocusNav,
         /// Focuses the next file in the file list.
@@ -430,6 +432,20 @@ fn init_renderers(cx: &mut App) {
 }
 
 pub fn open_settings_editor(cx: &mut App) -> anyhow::Result<WindowHandle<SettingsWindow>> {
+    let existing_window = cx
+        .windows()
+        .into_iter()
+        .find_map(|window| window.downcast::<SettingsWindow>());
+
+    if let Some(existing_window) = existing_window {
+        existing_window
+            .update(cx, |_, window, _| {
+                window.activate_window();
+            })
+            .ok();
+        return Ok(existing_window);
+    }
+
     cx.open_window(
         WindowOptions {
             titlebar: Some(TitlebarOptions {
@@ -479,6 +495,7 @@ pub struct SettingsWindow {
     list_handle: UniformListScrollHandle,
     search_matches: Vec<Vec<bool>>,
     scroll_handle: ScrollHandle,
+    focus_handle: FocusHandle,
     navbar_focus_handle: FocusHandle,
     content_focus_handle: FocusHandle,
     files_focus_handle: FocusHandle,
@@ -778,6 +795,7 @@ impl SettingsWindow {
             search_task: None,
             search_matches: vec![],
             scroll_handle: ScrollHandle::new(),
+            focus_handle: cx.focus_handle(),
             navbar_focus_handle: cx
                 .focus_handle()
                 .tab_index(NAVBAR_CONTAINER_TAB_INDEX)
@@ -1089,23 +1107,42 @@ impl SettingsWindow {
         self.build_ui(cx);
     }
 
-    fn render_files(&self, _window: &mut Window, cx: &mut Context<SettingsWindow>) -> Div {
-        h_flex().gap_1().children(self.files.iter().enumerate().map(
-            |(ix, (file, focus_handle))| {
-                Button::new(ix, file.name())
-                    .toggle_state(file == &self.current_file)
-                    .selected_style(ButtonStyle::Tinted(ui::TintColor::Accent))
-                    .track_focus(focus_handle)
-                    .on_click(
-                        cx.listener(move |this, evt: &gpui::ClickEvent, window, cx| {
-                            this.change_file(ix, cx);
-                            if evt.is_keyboard() {
-                                this.focus_first_nav_item(window, cx);
-                            }
-                        }),
-                    )
-            },
-        ))
+    fn render_files(
+        &self,
+        _window: &mut Window,
+        cx: &mut Context<SettingsWindow>,
+    ) -> impl IntoElement {
+        h_flex()
+            .w_full()
+            .gap_1()
+            .justify_between()
+            .child(
+                h_flex()
+                    .id("file_buttons_container")
+                    .w_64() // Temporary fix until long-term solution is a fixed set of buttons representing a file location (User, Project, and Remote)
+                    .gap_1()
+                    .overflow_x_scroll()
+                    .children(
+                        self.files
+                            .iter()
+                            .enumerate()
+                            .map(|(ix, (file, focus_handle))| {
+                                Button::new(ix, file.name())
+                                    .toggle_state(file == &self.current_file)
+                                    .selected_style(ButtonStyle::Tinted(ui::TintColor::Accent))
+                                    .track_focus(focus_handle)
+                                    .on_click(cx.listener(
+                                        move |this, evt: &gpui::ClickEvent, window, cx| {
+                                            this.change_file(ix, cx);
+                                            if evt.is_keyboard() {
+                                                this.focus_first_nav_item(window, cx);
+                                            }
+                                        },
+                                    ))
+                            }),
+                    ),
+            )
+            .child(Button::new("temp", "Edit in settings.json").style(ButtonStyle::Outlined)) // This should be replaced by the actual, functioning button
     }
 
     fn render_search(&self, _window: &mut Window, cx: &mut App) -> Div {
@@ -1128,6 +1165,11 @@ impl SettingsWindow {
     ) -> impl IntoElement {
         let visible_count = self.visible_navbar_entries().count();
         let nav_background = cx.theme().colors().panel_background;
+        let focus_keybind_label = if self.navbar_focus_handle.contains_focused(window, cx) {
+            "Focus Content"
+        } else {
+            "Focus Navbar"
+        };
 
         v_flex()
             .w_64()
@@ -1220,23 +1262,21 @@ impl SettingsWindow {
                     .vertical_scrollbar_for(self.list_handle.clone(), window, cx),
             )
             .child(
-                h_flex().w_full().justify_center().bg(nav_background).child(
-                    Button::new(
-                        "nav-key-hint",
-                        if self.navbar_focus_handle.contains_focused(window, cx) {
-                            "Focus Content"
-                        } else {
-                            "Focus Navbar"
-                        },
-                    )
-                    .key_binding(ui::KeyBinding::for_action_in(
-                        &ToggleFocusNav,
-                        &self.navbar_focus_handle,
-                        window,
-                        cx,
-                    ))
-                    .key_binding_position(KeybindingPosition::Start),
-                ),
+                h_flex()
+                    .w_full()
+                    .p_2()
+                    .pb_0p5()
+                    .border_t_1()
+                    .border_color(cx.theme().colors().border_variant)
+                    .children(
+                        KeyBinding::for_action(&ToggleFocusNav, window, cx).map(|this| {
+                            KeybindingHint::new(
+                                this,
+                                cx.theme().colors().surface_background.opacity(0.5),
+                            )
+                            .suffix(focus_keybind_label)
+                        }),
+                    ),
             )
     }
 
@@ -1361,7 +1401,7 @@ impl SettingsWindow {
         let page_content;
 
         if sub_page_stack().len() == 0 {
-            page_header = self.render_files(window, cx);
+            page_header = self.render_files(window, cx).into_any_element();
             page_content = self
                 .render_page_items(self.page_items(), window, cx)
                 .into_any_element();
@@ -1377,7 +1417,8 @@ impl SettingsWindow {
                             this.pop_sub_page(cx);
                         })),
                 )
-                .child(self.render_sub_page_breadcrumbs());
+                .child(self.render_sub_page_breadcrumbs())
+                .into_any_element();
 
             let active_page_render_fn = sub_page_stack().last().unwrap().link.render.clone();
             page_content = (active_page_render_fn)(self, window, cx);
@@ -1475,12 +1516,10 @@ impl Render for SettingsWindow {
         div()
             .id("settings-window")
             .key_context("SettingsWindow")
-            .flex()
-            .flex_row()
-            .size_full()
-            .font(ui_font)
-            .bg(cx.theme().colors().background)
-            .text_color(cx.theme().colors().text)
+            .track_focus(&self.focus_handle)
+            .on_action(|_: &Minimize, window, _cx| {
+                window.minimize_window();
+            })
             .on_action(cx.listener(|this, _: &search::FocusSearch, window, cx| {
                 this.search_bar.focus_handle(cx).focus(window);
             }))
@@ -1513,6 +1552,12 @@ impl Render for SettingsWindow {
             .on_action(|_: &menu::SelectPrevious, window, _| {
                 window.focus_prev();
             })
+            .flex()
+            .flex_row()
+            .size_full()
+            .font(ui_font)
+            .bg(cx.theme().colors().background)
+            .text_color(cx.theme().colors().text)
             .child(self.render_nav(window, cx))
             .child(self.render_page(window, cx))
     }
@@ -1652,36 +1697,24 @@ fn render_font_picker(
         )
     });
 
-    div()
-        .child(
-            PopoverMenu::new("font-picker")
-                .menu(move |_window, _cx| Some(font_picker.clone()))
-                .trigger(
-                    ButtonLike::new("font-family-button")
-                        .style(ButtonStyle::Outlined)
-                        .size(ButtonSize::Medium)
-                        .full_width()
-                        .tab_index(0_isize)
-                        .child(
-                            h_flex()
-                                .w_full()
-                                .justify_between()
-                                .child(Label::new(current_value))
-                                .child(
-                                    Icon::new(IconName::ChevronUpDown)
-                                        .color(Color::Muted)
-                                        .size(IconSize::XSmall),
-                                ),
-                        ),
-                )
-                .full_width(true)
-                .anchor(gpui::Corner::TopLeft)
-                .offset(gpui::Point {
-                    x: px(0.0),
-                    y: px(4.0),
-                })
-                .with_handle(ui::PopoverMenuHandle::default()),
+    PopoverMenu::new("font-picker")
+        .menu(move |_window, _cx| Some(font_picker.clone()))
+        .trigger(
+            Button::new("font-family-button", current_value)
+                .tab_index(0_isize)
+                .style(ButtonStyle::Outlined)
+                .size(ButtonSize::Medium)
+                .icon(IconName::ChevronUpDown)
+                .icon_color(Color::Muted)
+                .icon_size(IconSize::Small)
+                .icon_position(IconPosition::End),
         )
+        .anchor(gpui::Corner::TopLeft)
+        .offset(gpui::Point {
+            x: px(0.0),
+            y: px(2.0),
+        })
+        .with_handle(ui::PopoverMenuHandle::default())
         .into_any_element()
 }
 
@@ -1932,6 +1965,7 @@ mod test {
             search_matches: 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(),
             files_focus_handle: cx.focus_handle(),

crates/ui/src/components/button/button.rs 🔗

@@ -474,7 +474,6 @@ impl RenderOnce for Button {
     }
 }
 
-// View this component preview using `workspace: open component-preview`
 impl Component for Button {
     fn scope() -> ComponentScope {
         ComponentScope::Input

crates/ui/src/components/button/button_like.rs 🔗

@@ -625,7 +625,13 @@ impl RenderOnce for ButtonLike {
                     |refinement: StyleRefinement| refinement.bg(hovered_style.background);
                 this.cursor(self.cursor_style)
                     .hover(focus_color)
-                    .focus(focus_color)
+                    .map(|this| {
+                        if matches!(self.style, ButtonStyle::Outlined) {
+                            this.focus(|s| s.border_color(cx.theme().colors().border_focused))
+                        } else {
+                            this.focus(focus_color)
+                        }
+                    })
                     .active(|active| active.bg(style.active(cx).background))
             })
             .when_some(

crates/ui/src/components/keybinding_hint.rs 🔗

@@ -1,5 +1,5 @@
 use crate::KeyBinding;
-use crate::{h_flex, prelude::*};
+use crate::prelude::*;
 use gpui::{AnyElement, App, BoxShadow, FontStyle, Hsla, IntoElement, Window, point};
 use theme::Appearance;
 
@@ -206,20 +206,23 @@ impl KeybindingHint {
 
 impl RenderOnce for KeybindingHint {
     fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
-        let colors = cx.theme().colors().clone();
+        let colors = cx.theme().colors();
         let is_light = cx.theme().appearance() == Appearance::Light;
 
         let border_color =
             self.background_color
                 .blend(colors.text.alpha(if is_light { 0.08 } else { 0.16 }));
-        let bg_color =
-            self.background_color
-                .blend(colors.text.alpha(if is_light { 0.06 } else { 0.12 }));
+
+        let bg_color = self
+            .background_color
+            .blend(colors.text_accent.alpha(if is_light { 0.05 } else { 0.1 }));
+
         let shadow_color = colors.text.alpha(if is_light { 0.04 } else { 0.08 });
 
         let size = self
             .size
             .unwrap_or(TextSize::Small.rems(cx).to_pixels(window.rem_size()));
+
         let kb_size = size - px(2.0);
 
         let mut base = h_flex();
@@ -228,15 +231,13 @@ impl RenderOnce for KeybindingHint {
             .get_or_insert_with(Default::default)
             .font_style = Some(FontStyle::Italic);
 
-        base.items_center()
-            .gap_0p5()
+        base.gap_1()
             .font_buffer(cx)
             .text_size(size)
             .text_color(colors.text_disabled)
             .children(self.prefix)
             .child(
                 h_flex()
-                    .items_center()
                     .rounded_sm()
                     .px_0p5()
                     .mr_0p5()

crates/ui/src/components/tree_view_item.rs 🔗

@@ -152,6 +152,7 @@ impl RenderOnce for TreeViewItem {
                     .when_some(self.tab_index, |this, index| this.tab_index(index))
                     .map(|this| {
                         let label = self.label;
+
                         if self.root_item {
                             this.h(item_size)
                                 .px_1()
@@ -159,11 +160,11 @@ impl RenderOnce for TreeViewItem {
                                 .gap_2p5()
                                 .rounded_sm()
                                 .border_1()
-                                .focus(|s| s.border_color(focused_border))
                                 .border_color(transparent_border)
                                 .when(self.selected, |this| {
                                     this.border_color(selected_border).bg(selected_bg)
                                 })
+                                .focus(|s| s.border_color(focused_border))
                                 .hover(|s| s.bg(cx.theme().colors().element_hover))
                                 .child(
                                     Disclosure::new("toggle", self.expanded)
@@ -190,11 +191,11 @@ impl RenderOnce for TreeViewItem {
                                     .rounded_sm()
                                     .border_1()
                                     .focusable()
-                                    .in_focus(|s| s.border_color(focused_border))
                                     .border_color(transparent_border)
                                     .when(self.selected, |this| {
                                         this.border_color(selected_border).bg(selected_bg)
                                     })
+                                    .in_focus(|s| s.border_color(focused_border))
                                     .hover(|s| s.bg(cx.theme().colors().element_hover))
                                     .child(
                                         Label::new(label)
@@ -211,10 +212,12 @@ impl RenderOnce for TreeViewItem {
                                 && let Some(on_toggle) = self.on_toggle.clone()
                             {
                                 this.on_click(move |event, window, cx| {
-                                    if !event.is_keyboard() {
+                                    if event.is_keyboard() {
+                                        on_click(event, window, cx);
+                                        on_toggle(event, window, cx);
+                                    } else {
                                         on_click(event, window, cx);
                                     }
-                                    on_toggle(event, window, cx);
                                 })
                             } else {
                                 this.on_click(on_click)

crates/ui_input/src/font_picker.rs 🔗

@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use fuzzy::{StringMatch, StringMatchCandidate};
-use gpui::{AnyElement, App, Context, SharedString, Task, Window};
+use gpui::{AnyElement, App, Context, DismissEvent, SharedString, Task, Window};
 use picker::{Picker, PickerDelegate};
 use theme::FontFamilyCache;
 use ui::{ListItem, ListItemSpacing, prelude::*};
@@ -139,7 +139,12 @@ impl PickerDelegate for FontPickerDelegate {
         }
     }
 
-    fn dismissed(&mut self, _window: &mut Window, _cx: &mut Context<FontPicker>) {}
+    fn dismissed(&mut self, window: &mut Window, cx: &mut Context<FontPicker>) {
+        cx.defer_in(window, |picker, window, cx| {
+            picker.set_query("", window, cx);
+        });
+        cx.emit(DismissEvent);
+    }
 
     fn render_match(
         &self,
@@ -172,5 +177,5 @@ pub fn font_picker(
     Picker::uniform_list(delegate, window, cx)
         .show_scrollbar(true)
         .width(rems_from_px(210.))
-        .max_height(Some(rems(20.).into()))
+        .max_height(Some(rems(18.).into()))
 }

crates/ui_input/src/numeric_stepper.rs 🔗

@@ -382,18 +382,7 @@ impl<T: NumericStepperType> RenderOnce for NumericStepper<T> {
             })
             .child(
                 h_flex()
-                    .gap_1()
-                    .rounded_sm()
-                    .map(|this| {
-                        if is_outlined {
-                            this.overflow_hidden()
-                                .bg(cx.theme().colors().surface_background)
-                                .border_1()
-                                .border_color(cx.theme().colors().border_variant)
-                        } else {
-                            this.px_1().bg(cx.theme().colors().editor_background)
-                        }
-                    })
+                    .when(!is_outlined, |this| this.gap_1())
                     .map(|decrement| {
                         let decrement_handler = {
                             let value = self.value;
@@ -412,20 +401,24 @@ impl<T: NumericStepperType> RenderOnce for NumericStepper<T> {
                             decrement.child(
                                 h_flex()
                                     .id("decrement_button")
+                                    .cursor(gpui::CursorStyle::PointingHand)
                                     .p_1p5()
                                     .size_full()
                                     .justify_center()
-                                    .hover(|s| {
-                                        s.bg(cx.theme().colors().element_hover)
-                                            .cursor(gpui::CursorStyle::PointingHand)
-                                    })
-                                    .border_r_1()
+                                    .overflow_hidden()
+                                    .rounded_tl_sm()
+                                    .rounded_bl_sm()
+                                    .border_1()
                                     .border_color(cx.theme().colors().border_variant)
+                                    .bg(cx.theme().colors().surface_background)
+                                    .hover(|s| s.bg(cx.theme().colors().element_hover))
                                     .child(Icon::new(IconName::Dash).size(IconSize::Small))
                                     .when_some(tab_index.as_mut(), |this, tab_index| {
                                         *tab_index += 1;
                                         this.tab_index(*tab_index - 1).focus(|style| {
-                                            style.bg(cx.theme().colors().element_hover)
+                                            style
+                                                .border_color(cx.theme().colors().border_focused)
+                                                .bg(cx.theme().colors().element_hover)
                                         })
                                     })
                                     .on_click(decrement_handler),
@@ -446,9 +439,12 @@ impl<T: NumericStepperType> RenderOnce for NumericStepper<T> {
                     .child(
                         h_flex()
                             .min_w_16()
-                            .w_full()
-                            .border_1()
-                            .border_color(cx.theme().colors().border_transparent)
+                            .size_full()
+                            .when(is_outlined, |this| {
+                                this.border_y_1()
+                                    .border_color(cx.theme().colors().border_variant)
+                                    .bg(cx.theme().colors().surface_background)
+                            })
                             .in_focus(|this| this.border_color(cx.theme().colors().border_focused))
                             .child(match *self.mode.read(cx) {
                                 NumericStepperMode::Read => h_flex()
@@ -460,7 +456,9 @@ impl<T: NumericStepperType> RenderOnce for NumericStepper<T> {
                                     .when_some(tab_index.as_mut(), |this, tab_index| {
                                         *tab_index += 1;
                                         this.tab_index(*tab_index - 1).focus(|style| {
-                                            style.bg(cx.theme().colors().element_hover)
+                                            style
+                                                .border_color(cx.theme().colors().border_focused)
+                                                .bg(cx.theme().colors().element_hover)
                                         })
                                     })
                                     .on_click({
@@ -543,20 +541,24 @@ impl<T: NumericStepperType> RenderOnce for NumericStepper<T> {
                             increment.child(
                                 h_flex()
                                     .id("increment_button")
+                                    .cursor(gpui::CursorStyle::PointingHand)
                                     .p_1p5()
                                     .size_full()
                                     .justify_center()
-                                    .hover(|s| {
-                                        s.bg(cx.theme().colors().element_hover)
-                                            .cursor(gpui::CursorStyle::PointingHand)
-                                    })
-                                    .border_l_1()
+                                    .overflow_hidden()
+                                    .rounded_tr_sm()
+                                    .rounded_br_sm()
+                                    .border_1()
                                     .border_color(cx.theme().colors().border_variant)
+                                    .bg(cx.theme().colors().surface_background)
+                                    .hover(|s| s.bg(cx.theme().colors().element_hover))
                                     .child(Icon::new(IconName::Plus).size(IconSize::Small))
                                     .when_some(tab_index.as_mut(), |this, tab_index| {
                                         *tab_index += 1;
                                         this.tab_index(*tab_index - 1).focus(|style| {
-                                            style.bg(cx.theme().colors().element_hover)
+                                            style
+                                                .border_color(cx.theme().colors().border_focused)
+                                                .bg(cx.theme().colors().element_hover)
                                         })
                                     })
                                     .on_click(increment_handler),

crates/zed/src/zed/app_menus.rs 🔗

@@ -63,22 +63,26 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
                 MenuItem::submenu(Menu {
                     name: "Settings".into(),
                     items: vec![
-                        MenuItem::action("Open Settings", super::OpenSettings),
-                        MenuItem::action("Open Key Bindings", zed_actions::OpenKeymapEditor),
+                        MenuItem::action("Open Settings", zed_actions::OpenSettingsEditor),
+                        MenuItem::action("Open Settings JSON", super::OpenSettings),
+                        MenuItem::action("Open Project Settings", super::OpenProjectSettings),
                         MenuItem::action("Open Default Settings", super::OpenDefaultSettings),
+                        MenuItem::separator(),
+                        MenuItem::action("Open Keymap Editor", zed_actions::OpenKeymapEditor),
+                        MenuItem::action("Open Keymap JSON", zed_actions::OpenKeymap),
                         MenuItem::action(
                             "Open Default Key Bindings",
                             zed_actions::OpenDefaultKeymap,
                         ),
-                        MenuItem::action("Open Project Settings", super::OpenProjectSettings),
-                        MenuItem::action(
-                            "Select Settings Profile...",
-                            zed_actions::settings_profile_selector::Toggle,
-                        ),
+                        MenuItem::separator(),
                         MenuItem::action(
                             "Select Theme...",
                             zed_actions::theme_selector::Toggle::default(),
                         ),
+                        MenuItem::action(
+                            "Select Icon Theme...",
+                            zed_actions::icon_theme_selector::Toggle::default(),
+                        ),
                     ],
                 }),
                 MenuItem::separator(),