Refine toolbar and buffer search styling (#3743)

Marshall Bowers created

This PR refines the toolbar styling, specifically around the buffer
search.

Spacing has been adjusted to feel less claustrophobic.

Release Notes:

- N/A

Change summary

crates/search2/src/buffer_search.rs | 91 ++++++++++++++++++++----------
crates/search2/src/search.rs        | 33 ----------
crates/workspace2/src/toolbar.rs    | 11 ++-
3 files changed, 68 insertions(+), 67 deletions(-)

Detailed changes

crates/search2/src/buffer_search.rs 🔗

@@ -189,22 +189,6 @@ impl Render for BufferSearchBar {
                 Some(ui::Label::new(message))
             });
         let should_show_replace_input = self.replace_enabled && supported_options.replacement;
-        let replace_all = should_show_replace_input.then(|| {
-            super::render_replace_button(
-                ReplaceAll,
-                ui::Icon::ReplaceAll,
-                "Replace all",
-                cx.listener(|this, _, cx| this.replace_all(&ReplaceAll, cx)),
-            )
-        });
-        let replace_next = should_show_replace_input.then(|| {
-            super::render_replace_button(
-                ReplaceNext,
-                ui::Icon::ReplaceNext,
-                "Replace next",
-                cx.listener(|this, _, cx| this.replace_next(&ReplaceNext, cx)),
-            )
-        });
         let in_replace = self.replacement_editor.focus_handle(cx).is_focused(cx);
 
         let mut key_context = KeyContext::default();
@@ -214,6 +198,8 @@ impl Render for BufferSearchBar {
         }
 
         h_stack()
+            .w_full()
+            .gap_2()
             .key_context(key_context)
             .on_action(cx.listener(Self::previous_history_query))
             .on_action(cx.listener(Self::next_history_query))
@@ -239,7 +225,6 @@ impl Render for BufferSearchBar {
             .when(self.supported_options().word, |this| {
                 this.on_action(cx.listener(Self::toggle_whole_word))
             })
-            .w_full()
             .child(
                 h_stack()
                     .flex_1()
@@ -268,6 +253,7 @@ impl Render for BufferSearchBar {
             )
             .child(
                 h_stack()
+                    .gap_2()
                     .flex_none()
                     .child(
                         h_stack()
@@ -275,12 +261,22 @@ impl Render for BufferSearchBar {
                             .child(search_button_for_mode(SearchMode::Regex)),
                     )
                     .when(supported_options.replacement, |this| {
-                        this.child(super::toggle_replace_button(
-                            self.replace_enabled,
-                            cx.listener(|this, _: &ClickEvent, cx| {
+                        this.child(
+                            IconButton::new(
+                                "buffer-search-bar-toggle-replace-button",
+                                Icon::Replace,
+                            )
+                            .style(ButtonStyle::Subtle)
+                            .when(self.replace_enabled, |button| {
+                                button.style(ButtonStyle::Filled)
+                            })
+                            .on_click(cx.listener(|this, _: &ClickEvent, cx| {
                                 this.toggle_replace(&ToggleReplace, cx);
+                            }))
+                            .tooltip(|cx| {
+                                Tooltip::for_action("Toggle replace", &ToggleReplace, cx)
                             }),
-                        ))
+                        )
                     }),
             )
             .child(
@@ -288,16 +284,55 @@ impl Render for BufferSearchBar {
                     .gap_0p5()
                     .flex_1()
                     .when(self.replace_enabled, |this| {
-                        this.child(self.replacement_editor.clone())
-                            .children(replace_next)
-                            .children(replace_all)
+                        this.child(
+                            h_stack()
+                                .flex_1()
+                                // We're giving this a fixed height to match the height of the search input,
+                                // which has an icon inside that is increasing its height.
+                                .h_8()
+                                .px_2()
+                                .py_1()
+                                .gap_2()
+                                .border_1()
+                                .border_color(cx.theme().colors().border)
+                                .rounded_lg()
+                                .child(self.render_text_input(&self.replacement_editor, cx)),
+                        )
+                        .when(should_show_replace_input, |this| {
+                            this.child(
+                                IconButton::new("search-replace-next", ui::Icon::ReplaceNext)
+                                    .tooltip(move |cx| {
+                                        Tooltip::for_action("Replace next", &ReplaceNext, cx)
+                                    })
+                                    .on_click(cx.listener(|this, _, cx| {
+                                        this.replace_next(&ReplaceNext, cx)
+                                    })),
+                            )
+                            .child(
+                                IconButton::new("search-replace-all", ui::Icon::ReplaceAll)
+                                    .tooltip(move |cx| {
+                                        Tooltip::for_action("Replace all", &ReplaceAll, cx)
+                                    })
+                                    .on_click(
+                                        cx.listener(|this, _, cx| {
+                                            this.replace_all(&ReplaceAll, cx)
+                                        }),
+                                    ),
+                            )
+                        })
                     }),
             )
             .child(
                 h_stack()
                     .gap_0p5()
                     .flex_none()
-                    .child(self.render_action_button())
+                    .child(
+                        IconButton::new("select-all", ui::Icon::SelectAll)
+                            .on_click(|_, cx| cx.dispatch_action(SelectAllMatches.boxed_clone()))
+                            .tooltip(|cx| {
+                                Tooltip::for_action("Select all matches", &SelectAllMatches, cx)
+                            }),
+                    )
                     .children(match_count)
                     .child(render_nav_button(
                         ui::Icon::ChevronLeft,
@@ -625,12 +660,6 @@ impl BufferSearchBar {
         self.update_matches(cx)
     }
 
-    fn render_action_button(&self) -> impl IntoElement {
-        IconButton::new("select-all", ui::Icon::SelectAll)
-            .on_click(|_, cx| cx.dispatch_action(SelectAllMatches.boxed_clone()))
-            .tooltip(|cx| Tooltip::for_action("Select all matches", &SelectAllMatches, cx))
-    }
-
     fn render_search_option_button(
         &self,
         option: SearchOptions,

crates/search2/src/search.rs 🔗

@@ -4,11 +4,7 @@ use gpui::{actions, Action, AppContext, IntoElement};
 pub use mode::SearchMode;
 use project::search::SearchQuery;
 use ui::{prelude::*, Tooltip};
-use ui::{ButtonStyle, Icon, IconButton};
-//pub use project_search::{ProjectSearchBar, ProjectSearchView};
-// use theme::components::{
-//     action_button::Button, svg::Svg, ComponentExt, IconButtonStyle, ToggleIconButtonStyle,
-// };
+use ui::{ButtonStyle, IconButton};
 
 pub mod buffer_search;
 mod history;
@@ -109,30 +105,3 @@ impl SearchOptions {
             })
     }
 }
-
-fn toggle_replace_button(
-    active: bool,
-    action: impl Fn(&gpui::ClickEvent, &mut WindowContext) + 'static,
-) -> impl IntoElement {
-    // todo: add toggle_replace button
-    IconButton::new("buffer-search-bar-toggle-replace-button", Icon::Replace)
-        .on_click(action)
-        .style(ButtonStyle::Subtle)
-        .when(active, |button| button.style(ButtonStyle::Filled))
-        .tooltip(|cx| Tooltip::for_action("Toggle replace", &ToggleReplace, cx))
-}
-
-fn render_replace_button(
-    action: impl Action + 'static + Send + Sync,
-    icon: Icon,
-    tooltip: &'static str,
-    on_click: impl Fn(&gpui::ClickEvent, &mut WindowContext) + 'static,
-) -> impl IntoElement {
-    let id: SharedString = format!("search-replace-{}", action.name()).into();
-    IconButton::new(id, icon)
-        .tooltip({
-            let action = action.boxed_clone();
-            move |cx| Tooltip::for_action(tooltip, &*action, cx)
-        })
-        .on_click(on_click)
-}

crates/workspace2/src/toolbar.rs 🔗

@@ -102,16 +102,19 @@ impl Render for Toolbar {
 
         let secondary_item = self.secondary_items().next().map(|item| item.to_any());
 
+        let has_left_items = self.left_items().count() > 0;
+        let has_right_items = self.right_items().count() > 0;
+
         v_stack()
-            .p_1()
-            .gap_2()
+            .p_2()
+            .when(has_left_items || has_right_items, |this| this.gap_2())
             .border_b()
             .border_color(cx.theme().colors().border_variant)
             .bg(cx.theme().colors().toolbar_background)
             .child(
                 h_stack()
                     .justify_between()
-                    .when(self.left_items().count() > 0, |this| {
+                    .when(has_left_items, |this| {
                         this.child(
                             h_stack()
                                 .flex_1()
@@ -119,7 +122,7 @@ impl Render for Toolbar {
                                 .children(self.left_items().map(|item| item.to_any())),
                         )
                     })
-                    .when(self.right_items().count() > 0, |this| {
+                    .when(has_right_items, |this| {
                         this.child(
                             h_stack()
                                 .flex_1()