search: Fix recently introduced issues with the search bars (#36271)

Finn Evers created

Follow-up to https://github.com/zed-industries/zed/pull/36233

The above PR simplified the handling but introduced some bugs: The
replace buttons were no longer clickable, some buttons also lost their
toggle states, some buttons shared their element id and, lastly, some
buttons were clickable but would not trigger the right action. This PR
fixes all that.

Release Notes:

- N/A

Change summary

crates/search/src/buffer_search.rs  | 53 +++++++++++++++------------
crates/search/src/project_search.rs | 59 ++++++++++++++++++------------
crates/search/src/search.rs         | 55 ++++++++++++++++++++--------
crates/search/src/search_bar.rs     | 12 +++++-
4 files changed, 114 insertions(+), 65 deletions(-)

Detailed changes

crates/search/src/buffer_search.rs 🔗

@@ -2,9 +2,9 @@ mod registrar;
 
 use crate::{
     FocusSearch, NextHistoryQuery, PreviousHistoryQuery, ReplaceAll, ReplaceNext, SearchOption,
-    SearchOptions, SelectAllMatches, SelectNextMatch, SelectPreviousMatch, ToggleCaseSensitive,
-    ToggleRegex, ToggleReplace, ToggleSelection, ToggleWholeWord,
-    search_bar::{input_base_styles, render_action_button, render_text_input},
+    SearchOptions, SearchSource, SelectAllMatches, SelectNextMatch, SelectPreviousMatch,
+    ToggleCaseSensitive, ToggleRegex, ToggleReplace, ToggleSelection, ToggleWholeWord,
+    search_bar::{ActionButtonState, input_base_styles, render_action_button, render_text_input},
 };
 use any_vec::AnyVec;
 use anyhow::Context as _;
@@ -213,22 +213,25 @@ impl Render for BufferSearchBar {
                     h_flex()
                         .gap_1()
                         .when(case, |div| {
-                            div.child(
-                                SearchOption::CaseSensitive
-                                    .as_button(self.search_options, focus_handle.clone()),
-                            )
+                            div.child(SearchOption::CaseSensitive.as_button(
+                                self.search_options,
+                                SearchSource::Buffer,
+                                focus_handle.clone(),
+                            ))
                         })
                         .when(word, |div| {
-                            div.child(
-                                SearchOption::WholeWord
-                                    .as_button(self.search_options, focus_handle.clone()),
-                            )
+                            div.child(SearchOption::WholeWord.as_button(
+                                self.search_options,
+                                SearchSource::Buffer,
+                                focus_handle.clone(),
+                            ))
                         })
                         .when(regex, |div| {
-                            div.child(
-                                SearchOption::Regex
-                                    .as_button(self.search_options, focus_handle.clone()),
-                            )
+                            div.child(SearchOption::Regex.as_button(
+                                self.search_options,
+                                SearchSource::Buffer,
+                                focus_handle.clone(),
+                            ))
                         }),
                 )
             });
@@ -240,7 +243,7 @@ impl Render for BufferSearchBar {
                 this.child(render_action_button(
                     "buffer-search-bar-toggle",
                     IconName::Replace,
-                    self.replace_enabled,
+                    self.replace_enabled.then_some(ActionButtonState::Toggled),
                     "Toggle Replace",
                     &ToggleReplace,
                     focus_handle.clone(),
@@ -285,7 +288,9 @@ impl Render for BufferSearchBar {
                     .child(render_action_button(
                         "buffer-search-nav-button",
                         ui::IconName::ChevronLeft,
-                        self.active_match_index.is_some(),
+                        self.active_match_index
+                            .is_none()
+                            .then_some(ActionButtonState::Disabled),
                         "Select Previous Match",
                         &SelectPreviousMatch,
                         query_focus.clone(),
@@ -293,7 +298,9 @@ impl Render for BufferSearchBar {
                     .child(render_action_button(
                         "buffer-search-nav-button",
                         ui::IconName::ChevronRight,
-                        self.active_match_index.is_some(),
+                        self.active_match_index
+                            .is_none()
+                            .then_some(ActionButtonState::Disabled),
                         "Select Next Match",
                         &SelectNextMatch,
                         query_focus.clone(),
@@ -313,7 +320,7 @@ impl Render for BufferSearchBar {
                 el.child(render_action_button(
                     "buffer-search-nav-button",
                     IconName::SelectAll,
-                    true,
+                    Default::default(),
                     "Select All Matches",
                     &SelectAllMatches,
                     query_focus,
@@ -324,7 +331,7 @@ impl Render for BufferSearchBar {
                 el.child(render_action_button(
                     "buffer-search",
                     IconName::Close,
-                    true,
+                    Default::default(),
                     "Close Search Bar",
                     &Dismiss,
                     focus_handle.clone(),
@@ -352,7 +359,7 @@ impl Render for BufferSearchBar {
                     .child(render_action_button(
                         "buffer-search-replace-button",
                         IconName::ReplaceNext,
-                        true,
+                        Default::default(),
                         "Replace Next Match",
                         &ReplaceNext,
                         focus_handle.clone(),
@@ -360,7 +367,7 @@ impl Render for BufferSearchBar {
                     .child(render_action_button(
                         "buffer-search-replace-button",
                         IconName::ReplaceAll,
-                        true,
+                        Default::default(),
                         "Replace All Matches",
                         &ReplaceAll,
                         focus_handle,
@@ -394,7 +401,7 @@ impl Render for BufferSearchBar {
                     div.child(h_flex().absolute().right_0().child(render_action_button(
                         "buffer-search",
                         IconName::Close,
-                        true,
+                        Default::default(),
                         "Close Search Bar",
                         &Dismiss,
                         focus_handle.clone(),

crates/search/src/project_search.rs 🔗

@@ -1,9 +1,9 @@
 use crate::{
     BufferSearchBar, FocusSearch, NextHistoryQuery, PreviousHistoryQuery, ReplaceAll, ReplaceNext,
-    SearchOption, SearchOptions, SelectNextMatch, SelectPreviousMatch, ToggleCaseSensitive,
-    ToggleIncludeIgnored, ToggleRegex, ToggleReplace, ToggleWholeWord,
+    SearchOption, SearchOptions, SearchSource, SelectNextMatch, SelectPreviousMatch,
+    ToggleCaseSensitive, ToggleIncludeIgnored, ToggleRegex, ToggleReplace, ToggleWholeWord,
     buffer_search::Deploy,
-    search_bar::{input_base_styles, render_action_button, render_text_input},
+    search_bar::{ActionButtonState, input_base_styles, render_action_button, render_text_input},
 };
 use anyhow::Context as _;
 use collections::HashMap;
@@ -1665,7 +1665,7 @@ impl ProjectSearchBar {
         });
     }
 
-    fn toggle_search_option(
+    pub(crate) fn toggle_search_option(
         &mut self,
         option: SearchOptions,
         window: &mut Window,
@@ -1962,17 +1962,21 @@ impl Render for ProjectSearchBar {
             .child(
                 h_flex()
                     .gap_1()
-                    .child(
-                        SearchOption::CaseSensitive
-                            .as_button(search.search_options, focus_handle.clone()),
-                    )
-                    .child(
-                        SearchOption::WholeWord
-                            .as_button(search.search_options, focus_handle.clone()),
-                    )
-                    .child(
-                        SearchOption::Regex.as_button(search.search_options, focus_handle.clone()),
-                    ),
+                    .child(SearchOption::CaseSensitive.as_button(
+                        search.search_options,
+                        SearchSource::Project(cx),
+                        focus_handle.clone(),
+                    ))
+                    .child(SearchOption::WholeWord.as_button(
+                        search.search_options,
+                        SearchSource::Project(cx),
+                        focus_handle.clone(),
+                    ))
+                    .child(SearchOption::Regex.as_button(
+                        search.search_options,
+                        SearchSource::Project(cx),
+                        focus_handle.clone(),
+                    )),
             );
 
         let query_focus = search.query_editor.focus_handle(cx);
@@ -1985,7 +1989,10 @@ impl Render for ProjectSearchBar {
             .child(render_action_button(
                 "project-search-nav-button",
                 IconName::ChevronLeft,
-                search.active_match_index.is_some(),
+                search
+                    .active_match_index
+                    .is_none()
+                    .then_some(ActionButtonState::Disabled),
                 "Select Previous Match",
                 &SelectPreviousMatch,
                 query_focus.clone(),
@@ -1993,7 +2000,10 @@ impl Render for ProjectSearchBar {
             .child(render_action_button(
                 "project-search-nav-button",
                 IconName::ChevronRight,
-                search.active_match_index.is_some(),
+                search
+                    .active_match_index
+                    .is_none()
+                    .then_some(ActionButtonState::Disabled),
                 "Select Next Match",
                 &SelectNextMatch,
                 query_focus,
@@ -2054,7 +2064,7 @@ impl Render for ProjectSearchBar {
                 self.active_project_search
                     .as_ref()
                     .map(|search| search.read(cx).replace_enabled)
-                    .unwrap_or_default(),
+                    .and_then(|enabled| enabled.then_some(ActionButtonState::Toggled)),
                 "Toggle Replace",
                 &ToggleReplace,
                 focus_handle.clone(),
@@ -2079,7 +2089,7 @@ impl Render for ProjectSearchBar {
                 .child(render_action_button(
                     "project-search-replace-button",
                     IconName::ReplaceNext,
-                    true,
+                    Default::default(),
                     "Replace Next Match",
                     &ReplaceNext,
                     focus_handle.clone(),
@@ -2087,7 +2097,7 @@ impl Render for ProjectSearchBar {
                 .child(render_action_button(
                     "project-search-replace-button",
                     IconName::ReplaceAll,
-                    true,
+                    Default::default(),
                     "Replace All Matches",
                     &ReplaceAll,
                     focus_handle,
@@ -2129,10 +2139,11 @@ impl Render for ProjectSearchBar {
                             this.toggle_opened_only(window, cx);
                         })),
                 )
-                .child(
-                    SearchOption::IncludeIgnored
-                        .as_button(search.search_options, focus_handle.clone()),
-                );
+                .child(SearchOption::IncludeIgnored.as_button(
+                    search.search_options,
+                    SearchSource::Project(cx),
+                    focus_handle.clone(),
+                ));
             h_flex()
                 .w_full()
                 .gap_2()

crates/search/src/search.rs 🔗

@@ -1,7 +1,7 @@
 use bitflags::bitflags;
 pub use buffer_search::BufferSearchBar;
 use editor::SearchSettings;
-use gpui::{Action, App, FocusHandle, IntoElement, actions};
+use gpui::{Action, App, ClickEvent, FocusHandle, IntoElement, actions};
 use project::search::SearchQuery;
 pub use project_search::ProjectSearchView;
 use ui::{ButtonStyle, IconButton, IconButtonShape};
@@ -11,6 +11,8 @@ use workspace::{Toast, Workspace};
 
 pub use search_status_button::SEARCH_ICON;
 
+use crate::project_search::ProjectSearchBar;
+
 pub mod buffer_search;
 pub mod project_search;
 pub(crate) mod search_bar;
@@ -83,9 +85,14 @@ pub enum SearchOption {
     Backwards,
 }
 
+pub(crate) enum SearchSource<'a, 'b> {
+    Buffer,
+    Project(&'a Context<'b, ProjectSearchBar>),
+}
+
 impl SearchOption {
-    pub fn as_options(self) -> SearchOptions {
-        SearchOptions::from_bits(1 << self as u8).unwrap()
+    pub fn as_options(&self) -> SearchOptions {
+        SearchOptions::from_bits(1 << *self as u8).unwrap()
     }
 
     pub fn label(&self) -> &'static str {
@@ -119,25 +126,41 @@ impl SearchOption {
         }
     }
 
-    pub fn as_button(&self, active: SearchOptions, focus_handle: FocusHandle) -> impl IntoElement {
+    pub(crate) fn as_button(
+        &self,
+        active: SearchOptions,
+        search_source: SearchSource,
+        focus_handle: FocusHandle,
+    ) -> impl IntoElement {
         let action = self.to_toggle_action();
         let label = self.label();
-        IconButton::new(label, self.icon())
-            .on_click({
+        IconButton::new(
+            (label, matches!(search_source, SearchSource::Buffer) as u32),
+            self.icon(),
+        )
+        .map(|button| match search_source {
+            SearchSource::Buffer => {
                 let focus_handle = focus_handle.clone();
-                move |_, window, cx| {
+                button.on_click(move |_: &ClickEvent, window, cx| {
                     if !focus_handle.is_focused(&window) {
                         window.focus(&focus_handle);
                     }
-                    window.dispatch_action(action.boxed_clone(), cx)
-                }
-            })
-            .style(ButtonStyle::Subtle)
-            .shape(IconButtonShape::Square)
-            .toggle_state(active.contains(self.as_options()))
-            .tooltip({
-                move |window, cx| Tooltip::for_action_in(label, action, &focus_handle, window, cx)
-            })
+                    window.dispatch_action(action.boxed_clone(), cx);
+                })
+            }
+            SearchSource::Project(cx) => {
+                let options = self.as_options();
+                button.on_click(cx.listener(move |this, _: &ClickEvent, window, cx| {
+                    this.toggle_search_option(options, window, cx);
+                }))
+            }
+        })
+        .style(ButtonStyle::Subtle)
+        .shape(IconButtonShape::Square)
+        .toggle_state(active.contains(self.as_options()))
+        .tooltip({
+            move |window, cx| Tooltip::for_action_in(label, action, &focus_handle, window, cx)
+        })
     }
 }
 

crates/search/src/search_bar.rs 🔗

@@ -5,10 +5,15 @@ use theme::ThemeSettings;
 use ui::{IconButton, IconButtonShape};
 use ui::{Tooltip, prelude::*};
 
+pub(super) enum ActionButtonState {
+    Disabled,
+    Toggled,
+}
+
 pub(super) fn render_action_button(
     id_prefix: &'static str,
     icon: ui::IconName,
-    active: bool,
+    button_state: Option<ActionButtonState>,
     tooltip: &'static str,
     action: &'static dyn Action,
     focus_handle: FocusHandle,
@@ -28,7 +33,10 @@ pub(super) fn render_action_button(
         }
     })
     .tooltip(move |window, cx| Tooltip::for_action_in(tooltip, action, &focus_handle, window, cx))
-    .disabled(!active)
+    .when_some(button_state, |this, state| match state {
+        ActionButtonState::Toggled => this.toggle_state(true),
+        ActionButtonState::Disabled => this.disabled(true),
+    })
 }
 
 pub(crate) fn input_base_styles(border_color: Hsla, map: impl FnOnce(Div) -> Div) -> Div {