Search bar UI enhancements (#7675)

Andrew Lygin created

This PR introduces several enhancements (along with fixing a couple of
bugs) in the search bar UI (copied from [this
comment](https://github.com/zed-industries/zed/issues/7663#issuecomment-1937659091)):

- Moving the Replace field under the Search field makes it easier to
compare texts and avoid typos. Also, less eyes movements.
- Use red (error) color to indicate that nothing matched. VSCode, IDEA
do this. Again, it helps to get a quicker feedback on typos without
moving your eyes.
- Much less moving parts and no place for flickering.
- Better fits to narrow panes.
- The Close button that allows to close the search bar with the mouse.
- Better keyboard handling (tab, shift+tab in the replacement mode),
autofocus on the Replace field.

How it looks:


https://github.com/zed-industries/zed/assets/2101250/93b0edb4-4311-4a7f-9f43-b30c4d1aede5

Implementation details:

- Using `Self::on_query_editor_event` looked suspicious
[here](https://github.com/zed-industries/zed/blob/288013503755f0d44feff6a517169a2861b43f26/crates/search/src/buffer_search.rs#L491)
because it triggered searching on the replacement text changes. I've
created a separate method for the replacement editor.
- These changes don't affect the project search bar. If the PR is
accepted, the same changes may be implemented there.

Fixed issues:

- #7661
- #7663

Release Notes:

- Buffer search bar UI enhancements.

Change summary

crates/search/src/buffer_search.rs | 286 +++++++++++++++++++------------
1 file changed, 177 insertions(+), 109 deletions(-)

Detailed changes

crates/search/src/buffer_search.rs 🔗

@@ -9,13 +9,16 @@ use crate::{
     ToggleCaseSensitive, ToggleReplace, ToggleWholeWord,
 };
 use collections::HashMap;
-use editor::{actions::Tab, Editor, EditorElement, EditorStyle};
+use editor::{
+    actions::{Tab, TabPrev},
+    Editor, EditorElement, EditorStyle,
+};
 use futures::channel::oneshot;
 use gpui::{
     actions, div, impl_actions, Action, AppContext, ClickEvent, EventEmitter, FocusableView,
-    FontStyle, FontWeight, InteractiveElement as _, IntoElement, KeyContext, ParentElement as _,
-    Render, Styled, Subscription, Task, TextStyle, View, ViewContext, VisualContext as _,
-    WhiteSpace, WindowContext,
+    FontStyle, FontWeight, Hsla, InteractiveElement as _, IntoElement, KeyContext,
+    ParentElement as _, Render, Styled, Subscription, Task, TextStyle, View, ViewContext,
+    VisualContext as _, WhiteSpace, WindowContext,
 };
 use project::search::SearchQuery;
 use serde::Deserialize;
@@ -23,7 +26,7 @@ use settings::Settings;
 use std::{any::Any, sync::Arc};
 use theme::ThemeSettings;
 
-use ui::{h_flex, prelude::*, Icon, IconButton, IconName, ToggleButton, Tooltip};
+use ui::{h_flex, prelude::*, IconButton, IconName, ToggleButton, Tooltip};
 use util::ResultExt;
 use workspace::{
     item::ItemHandle,
@@ -34,6 +37,9 @@ use workspace::{
 pub use registrar::DivRegistrar;
 use registrar::{ForDeployed, ForDismissed, SearchActionsRegistrar, WithResults};
 
+const MIN_INPUT_WIDTH_REMS: f32 = 15.;
+const MAX_INPUT_WIDTH_REMS: f32 = 25.;
+
 #[derive(PartialEq, Clone, Deserialize)]
 pub struct Deploy {
     pub focus: bool,
@@ -54,7 +60,9 @@ pub fn init(cx: &mut AppContext) {
 
 pub struct BufferSearchBar {
     query_editor: View<Editor>,
+    query_editor_focused: bool,
     replacement_editor: View<Editor>,
+    replacement_editor_focused: bool,
     active_searchable_item: Option<Box<dyn SearchableItemHandle>>,
     active_match_index: Option<usize>,
     active_searchable_item_subscription: Option<Subscription>,
@@ -72,13 +80,18 @@ pub struct BufferSearchBar {
 }
 
 impl BufferSearchBar {
-    fn render_text_input(&self, editor: &View<Editor>, cx: &ViewContext<Self>) -> impl IntoElement {
+    fn render_text_input(
+        &self,
+        editor: &View<Editor>,
+        color: Hsla,
+        cx: &ViewContext<Self>,
+    ) -> impl IntoElement {
         let settings = ThemeSettings::get_global(cx);
         let text_style = TextStyle {
             color: if editor.read(cx).read_only(cx) {
                 cx.theme().colors().text_disabled
             } else {
-                cx.theme().colors().text
+                color
             },
             font_family: settings.ui_font.family.clone(),
             font_features: settings.ui_font.features,
@@ -161,7 +174,8 @@ impl Render for BufferSearchBar {
             editor.set_placeholder_text("Replace with...", cx);
         });
 
-        let match_count = self
+        let mut match_color = Color::Default;
+        let match_text = self
             .active_searchable_item
             .as_ref()
             .and_then(|searchable_item| {
@@ -171,14 +185,15 @@ impl Render for BufferSearchBar {
                 let matches = self
                     .searchable_items_with_matches
                     .get(&searchable_item.downgrade())?;
-                let message = if let Some(match_ix) = self.active_match_index {
-                    format!("{}/{}", match_ix + 1, matches.len())
+                if let Some(match_ix) = self.active_match_index {
+                    Some(format!("{}/{}", match_ix + 1, matches.len()))
                 } else {
-                    "No matches".to_string()
-                };
-
-                Some(ui::Label::new(message))
-            });
+                    match_color = Color::Error; // No matches found
+                    None
+                }
+            })
+            .unwrap_or_else(|| "No matches".to_string());
+        let match_count = Label::new(match_text).color(match_color);
         let should_show_replace_input = self.replace_enabled && supported_options.replacement;
         let in_replace = self.replacement_editor.focus_handle(cx).is_focused(cx);
 
@@ -192,35 +207,9 @@ impl Render for BufferSearchBar {
         } else {
             cx.theme().colors().border
         };
-        h_flex()
-            .w_full()
+
+        let search_line = h_flex()
             .gap_2()
-            .key_context(key_context)
-            .capture_action(cx.listener(Self::tab))
-            .on_action(cx.listener(Self::previous_history_query))
-            .on_action(cx.listener(Self::next_history_query))
-            .on_action(cx.listener(Self::dismiss))
-            .on_action(cx.listener(Self::select_next_match))
-            .on_action(cx.listener(Self::select_prev_match))
-            .on_action(cx.listener(|this, _: &ActivateRegexMode, cx| {
-                this.activate_search_mode(SearchMode::Regex, cx);
-            }))
-            .on_action(cx.listener(|this, _: &ActivateTextMode, cx| {
-                this.activate_search_mode(SearchMode::Text, cx);
-            }))
-            .when(self.supported_options().replacement, |this| {
-                this.on_action(cx.listener(Self::toggle_replace))
-                    .when(in_replace, |this| {
-                        this.on_action(cx.listener(Self::replace_next))
-                            .on_action(cx.listener(Self::replace_all))
-                    })
-            })
-            .when(self.supported_options().case, |this| {
-                this.on_action(cx.listener(Self::toggle_case_sensitive))
-            })
-            .when(self.supported_options().word, |this| {
-                this.on_action(cx.listener(Self::toggle_whole_word))
-            })
             .child(
                 h_flex()
                     .flex_1()
@@ -229,10 +218,10 @@ impl Render for BufferSearchBar {
                     .gap_2()
                     .border_1()
                     .border_color(editor_border)
-                    .min_w(rems(384. / 16.))
+                    .min_w(rems(MIN_INPUT_WIDTH_REMS))
+                    .max_w(rems(MAX_INPUT_WIDTH_REMS))
                     .rounded_lg()
-                    .child(Icon::new(IconName::MagnifyingGlass))
-                    .child(self.render_text_input(&self.query_editor, cx))
+                    .child(self.render_text_input(&self.query_editor, match_color.color(cx), cx))
                     .children(supported_options.case.then(|| {
                         self.render_search_option_button(
                             SearchOptions::CASE_SENSITIVE,
@@ -308,49 +297,6 @@ impl Render for BufferSearchBar {
                         )
                     }),
             )
-            .child(
-                h_flex()
-                    .gap_0p5()
-                    .flex_1()
-                    .when(self.replace_enabled, |this| {
-                        this.child(
-                            h_flex()
-                                .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::IconName::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::IconName::ReplaceAll)
-                                    .tooltip(move |cx| {
-                                        Tooltip::for_action("Replace all", &ReplaceAll, cx)
-                                    })
-                                    .on_click(
-                                        cx.listener(|this, _, cx| {
-                                            this.replace_all(&ReplaceAll, cx)
-                                        }),
-                                    ),
-                            )
-                        })
-                    }),
-            )
             .child(
                 h_flex()
                     .gap_0p5()
@@ -362,7 +308,7 @@ impl Render for BufferSearchBar {
                                 Tooltip::for_action("Select all matches", &SelectAllMatches, cx)
                             }),
                     )
-                    .children(match_count)
+                    .child(div().min_w(rems(6.)).child(match_count))
                     .child(render_nav_button(
                         ui::IconName::ChevronLeft,
                         self.active_match_index.is_some(),
@@ -375,7 +321,89 @@ impl Render for BufferSearchBar {
                         "Select next match",
                         &SelectNextMatch,
                     )),
+            );
+
+        let replace_line = self.replace_enabled.then(|| {
+            h_flex()
+                .gap_0p5()
+                .flex_1()
+                .child(
+                    h_flex()
+                        .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()
+                        .min_w(rems(MIN_INPUT_WIDTH_REMS))
+                        .max_w(rems(MAX_INPUT_WIDTH_REMS))
+                        .child(self.render_text_input(
+                            &self.replacement_editor,
+                            cx.theme().colors().text,
+                            cx,
+                        )),
+                )
+                .when(should_show_replace_input, |this| {
+                    this.child(
+                        IconButton::new("search-replace-next", ui::IconName::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::IconName::ReplaceAll)
+                            .tooltip(move |cx| Tooltip::for_action("Replace all", &ReplaceAll, cx))
+                            .on_click(cx.listener(|this, _, cx| this.replace_all(&ReplaceAll, cx))),
+                    )
+                })
+        });
+
+        v_flex()
+            .key_context(key_context)
+            .capture_action(cx.listener(Self::tab))
+            .capture_action(cx.listener(Self::tab_prev))
+            .on_action(cx.listener(Self::previous_history_query))
+            .on_action(cx.listener(Self::next_history_query))
+            .on_action(cx.listener(Self::dismiss))
+            .on_action(cx.listener(Self::select_next_match))
+            .on_action(cx.listener(Self::select_prev_match))
+            .on_action(cx.listener(|this, _: &ActivateRegexMode, cx| {
+                this.activate_search_mode(SearchMode::Regex, cx);
+            }))
+            .on_action(cx.listener(|this, _: &ActivateTextMode, cx| {
+                this.activate_search_mode(SearchMode::Text, cx);
+            }))
+            .when(self.supported_options().replacement, |this| {
+                this.on_action(cx.listener(Self::toggle_replace))
+                    .when(in_replace, |this| {
+                        this.on_action(cx.listener(Self::replace_next))
+                            .on_action(cx.listener(Self::replace_all))
+                    })
+            })
+            .when(self.supported_options().case, |this| {
+                this.on_action(cx.listener(Self::toggle_case_sensitive))
+            })
+            .when(self.supported_options().word, |this| {
+                this.on_action(cx.listener(Self::toggle_whole_word))
+            })
+            .gap_1()
+            .child(
+                h_flex().child(search_line.w_full()).child(
+                    IconButton::new(SharedString::from("Close"), IconName::Close)
+                        .tooltip(move |cx| Tooltip::for_action("Close search bar", &Dismiss, cx))
+                        .on_click(
+                            cx.listener(|this, _: &ClickEvent, cx| this.dismiss(&Dismiss, cx)),
+                        ),
+                ),
             )
+            .children(replace_line)
     }
 }
 
@@ -488,11 +516,13 @@ impl BufferSearchBar {
         cx.subscribe(&query_editor, Self::on_query_editor_event)
             .detach();
         let replacement_editor = cx.new_view(|cx| Editor::single_line(cx));
-        cx.subscribe(&replacement_editor, Self::on_query_editor_event)
+        cx.subscribe(&replacement_editor, Self::on_replacement_editor_event)
             .detach();
         Self {
             query_editor,
+            query_editor_focused: false,
             replacement_editor,
+            replacement_editor_focused: false,
             active_searchable_item: None,
             active_searchable_item_subscription: None,
             active_match_index: None,
@@ -765,15 +795,33 @@ impl BufferSearchBar {
         event: &editor::EditorEvent,
         cx: &mut ViewContext<Self>,
     ) {
-        if let editor::EditorEvent::Edited = event {
-            self.query_contains_error = false;
-            self.clear_matches(cx);
-            let search = self.update_matches(cx);
-            cx.spawn(|this, mut cx| async move {
-                search.await?;
-                this.update(&mut cx, |this, cx| this.activate_current_match(cx))
-            })
-            .detach_and_log_err(cx);
+        match event {
+            editor::EditorEvent::Focused => self.query_editor_focused = true,
+            editor::EditorEvent::Blurred => self.query_editor_focused = false,
+            editor::EditorEvent::Edited => {
+                self.query_contains_error = false;
+                self.clear_matches(cx);
+                let search = self.update_matches(cx);
+                cx.spawn(|this, mut cx| async move {
+                    search.await?;
+                    this.update(&mut cx, |this, cx| this.activate_current_match(cx))
+                })
+                .detach_and_log_err(cx);
+            }
+            _ => {}
+        }
+    }
+
+    fn on_replacement_editor_event(
+        &mut self,
+        _: View<Editor>,
+        event: &editor::EditorEvent,
+        _: &mut ViewContext<Self>,
+    ) {
+        match event {
+            editor::EditorEvent::Focused => self.replacement_editor_focused = true,
+            editor::EditorEvent::Blurred => self.replacement_editor_focused = false,
+            _ => {}
         }
     }
 
@@ -911,11 +959,29 @@ impl BufferSearchBar {
     }
 
     fn tab(&mut self, _: &Tab, cx: &mut ViewContext<Self>) {
-        if let Some(item) = self.active_searchable_item.as_ref() {
-            let focus_handle = item.focus_handle(cx);
-            cx.focus(&focus_handle);
-            cx.stop_propagation();
-        }
+        // Search -> Replace -> Editor
+        let focus_handle = if self.replace_enabled && self.query_editor_focused {
+            self.replacement_editor.focus_handle(cx)
+        } else if let Some(item) = self.active_searchable_item.as_ref() {
+            item.focus_handle(cx)
+        } else {
+            return;
+        };
+        cx.focus(&focus_handle);
+        cx.stop_propagation();
+    }
+
+    fn tab_prev(&mut self, _: &TabPrev, cx: &mut ViewContext<Self>) {
+        // Search -> Replace -> Search
+        let focus_handle = if self.replace_enabled && self.query_editor_focused {
+            self.replacement_editor.focus_handle(cx)
+        } else if self.replacement_editor_focused {
+            self.query_editor.focus_handle(cx)
+        } else {
+            return;
+        };
+        cx.focus(&focus_handle);
+        cx.stop_propagation();
     }
 
     fn next_history_query(&mut self, _: &NextHistoryQuery, cx: &mut ViewContext<Self>) {
@@ -945,10 +1011,12 @@ impl BufferSearchBar {
     fn toggle_replace(&mut self, _: &ToggleReplace, cx: &mut ViewContext<Self>) {
         if let Some(_) = &self.active_searchable_item {
             self.replace_enabled = !self.replace_enabled;
-            if !self.replace_enabled {
-                let handle = self.query_editor.focus_handle(cx);
-                cx.focus(&handle);
-            }
+            let handle = if self.replace_enabled {
+                self.replacement_editor.focus_handle(cx)
+            } else {
+                self.query_editor.focus_handle(cx)
+            };
+            cx.focus(&handle);
             cx.notify();
         }
     }