Use a handler instead of an action for clicks

This prevents dispatching actions on buttons that were not the target of the click.

Co-Authored-By: Marshall <marshall@zed.dev>

crates/assistant2/src/assistant_panel.rs

@@ -2574,7 +2574,9 @@ impl Render for InlineAssistant {
                     .w(measurements.gutter_width)
                     .child(
                         IconButton::new("include_conversation", Icon::Ai)
-                            .action(Box::new(ToggleIncludeConversation))
+                            .on_click(cx.listener(|this, _, cx| {
+                                this.toggle_include_conversation(&ToggleIncludeConversation, cx)
+                            }))
                             .selected(self.include_conversation)
                             .tooltip(|cx| {
                                 Tooltip::for_action(
@@ -2587,7 +2589,9 @@ impl Render for InlineAssistant {
                     .children(if SemanticIndex::enabled(cx) {
                         Some(
                             IconButton::new("retrieve_context", Icon::MagnifyingGlass)
-                                .action(Box::new(ToggleRetrieveContext))
+                                .on_click(cx.listener(|this, _, cx| {
+                                    this.toggle_retrieve_context(&ToggleRetrieveContext, cx)
+                                }))
                                 .selected(self.retrieve_context)
                                 .tooltip(|cx| {
                                     Tooltip::for_action(

crates/quick_action_bar2/src/quick_action_bar.rs

@@ -2,8 +2,8 @@
 use editor::Editor;
 
 use gpui::{
-    Action, Div, ElementId, EventEmitter, InteractiveElement, ParentElement, Render, Stateful,
+    Action, ClickEvent, Div, ElementId, EventEmitter, InteractiveElement, ParentElement, Render,
-    Styled, Subscription, View, ViewContext, WeakView,
+    Stateful, Styled, Subscription, View, ViewContext, WeakView,
 };
 use search::BufferSearchBar;
 use ui::{prelude::*, ButtonSize, ButtonStyle, Icon, IconButton, IconSize, Tooltip};
@@ -41,19 +41,24 @@ impl Render for QuickActionBar {
     type Element = Stateful<Div>;
 
     fn render(&mut self, cx: &mut ViewContext<Self>) -> Self::Element {
+        let buffer_search_bar = self.buffer_search_bar.clone();
         let search_button = QuickActionBarButton::new(
             "toggle buffer search",
             Icon::MagnifyingGlass,
             !self.buffer_search_bar.read(cx).is_dismissed(),
             Box::new(search::buffer_search::Deploy { focus: false }),
             "Buffer Search",
+            move |_, cx| {
+                buffer_search_bar.update(cx, |search_bar, cx| search_bar.toggle(cx));
+            },
         );
         let assistant_button = QuickActionBarButton::new(
-            "toggle inline assitant",
+            "toggle inline assistant",
             Icon::MagicWand,
             false,
             Box::new(gpui::NoAction),
             "Inline assistant",
+            |_, _cx| todo!(),
         );
         h_stack()
             .id("quick action bar")
@@ -154,6 +159,7 @@ struct QuickActionBarButton {
     action: Box<dyn Action>,
     tooltip: SharedString,
     tooltip_meta: Option<SharedString>,
+    on_click: Box<dyn Fn(&ClickEvent, &mut WindowContext)>,
 }
 
 impl QuickActionBarButton {
@@ -163,6 +169,7 @@ impl QuickActionBarButton {
         toggled: bool,
         action: Box<dyn Action>,
         tooltip: impl Into<SharedString>,
+        on_click: impl Fn(&ClickEvent, &mut WindowContext) + 'static,
     ) -> Self {
         Self {
             id: id.into(),
@@ -171,6 +178,7 @@ impl QuickActionBarButton {
             action,
             tooltip: tooltip.into(),
             tooltip_meta: None,
+            on_click: Box::new(on_click),
         }
     }
 
@@ -201,10 +209,7 @@ impl RenderOnce for QuickActionBarButton {
                     Tooltip::for_action(tooltip.clone(), &*action, cx)
                 }
             })
-            .on_click({
+            .on_click(move |event, cx| (self.on_click)(event, cx))
-                let action = self.action.boxed_clone();
-                move |_, cx| cx.dispatch_action(action.boxed_clone())
-            })
     }
 }
 

crates/search2/src/buffer_search.rs

@@ -18,7 +18,7 @@ use project::search::SearchQuery;
 use serde::Deserialize;
 use std::{any::Any, sync::Arc};
 
-use ui::{h_stack, Icon, IconButton, IconElement};
+use ui::{h_stack, Clickable, Icon, IconButton, IconElement};
 use util::ResultExt;
 use workspace::{
     item::ItemHandle,
@@ -161,16 +161,6 @@ impl Render for BufferSearchBar {
 
                 Some(ui::Label::new(message))
             });
-        let nav_button_for_direction = |icon, direction| {
-            render_nav_button(
-                icon,
-                self.active_match_index.is_some(),
-                cx.listener(move |this, _, cx| match direction {
-                    Direction::Prev => this.select_prev_match(&Default::default(), cx),
-                    Direction::Next => this.select_next_match(&Default::default(), cx),
-                }),
-            )
-        };
         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));
@@ -237,15 +227,21 @@ impl Render for BufferSearchBar {
                 h_stack()
                     .gap_0p5()
                     .flex_none()
-                    .child(self.render_action_button())
+                    .child(self.render_action_button(cx))
                     .children(match_count)
-                    .child(nav_button_for_direction(
+                    .child(render_nav_button(
                         ui::Icon::ChevronLeft,
-                        Direction::Prev,
+                        self.active_match_index.is_some(),
+                        cx.listener(move |this, _, cx| {
+                            this.select_prev_match(&Default::default(), cx);
+                        }),
                     ))
-                    .child(nav_button_for_direction(
+                    .child(render_nav_button(
                         ui::Icon::ChevronRight,
-                        Direction::Next,
+                        self.active_match_index.is_some(),
+                        cx.listener(move |this, _, cx| {
+                            this.select_next_match(&Default::default(), cx);
+                        }),
                     )),
             )
     }
@@ -317,13 +313,7 @@ impl BufferSearchBar {
             pane.update(cx, |this, cx| {
                 this.toolbar().update(cx, |this, cx| {
                     if let Some(search_bar) = this.item_of_type::<BufferSearchBar>() {
-                        search_bar.update(cx, |this, cx| {
+                        search_bar.update(cx, |this, cx| this.toggle(cx));
-                            if this.is_dismissed() {
-                                this.show(cx);
-                            } else {
-                                this.dismiss(&Dismiss, cx);
-                            }
-                        });
                         return;
                     }
                     let view = cx.build_view(|cx| BufferSearchBar::new(cx));
@@ -487,6 +477,14 @@ impl BufferSearchBar {
         false
     }
 
+    pub fn toggle(&mut self, cx: &mut ViewContext<Self>) {
+        if self.is_dismissed() {
+            self.show(cx);
+        } else {
+            self.dismiss(&Dismiss, cx);
+        }
+    }
+
     pub fn show(&mut self, cx: &mut ViewContext<Self>) -> bool {
         if self.active_searchable_item.is_none() {
             return false;
@@ -588,12 +586,14 @@ impl BufferSearchBar {
         self.update_matches(cx)
     }
 
-    fn render_action_button(&self) -> impl IntoElement {
+    fn render_action_button(&self, cx: &mut ViewContext<Self>) -> impl IntoElement {
         // let tooltip_style = theme.tooltip.clone();
 
         // let style = theme.search.action_button.clone();
 
-        IconButton::new(0, ui::Icon::SelectAll).action(Box::new(SelectAllMatches))
+        IconButton::new("select-all", ui::Icon::SelectAll).on_click(cx.listener(|this, _, cx| {
+            this.select_all_matches(&SelectAllMatches, cx);
+        }))
     }
 
     pub fn activate_search_mode(&mut self, mode: SearchMode, cx: &mut ViewContext<Self>) {

crates/ui2/src/components/button/icon_button.rs

@@ -1,4 +1,4 @@
-use gpui::{Action, AnyView, DefiniteLength};
+use gpui::{AnyView, DefiniteLength};
 
 use crate::prelude::*;
 use crate::{ButtonCommon, ButtonLike, ButtonSize, ButtonStyle, Icon, IconSize};
@@ -39,10 +39,6 @@ impl IconButton {
         self.selected_icon = icon.into();
         self
     }
-
-    pub fn action(self, action: Box<dyn Action>) -> Self {
-        self.on_click(move |_event, cx| cx.dispatch_action(action.boxed_clone()))
-    }
 }
 
 impl Disableable for IconButton {

crates/workspace2/src/dock.rs

@@ -724,7 +724,10 @@ impl Render for PanelButtons {
                         .trigger(
                             IconButton::new(name, icon)
                                 .selected(is_active_button)
-                                .action(action.boxed_clone())
+                                .on_click({
+                                    let action = action.boxed_clone();
+                                    move |_, cx| cx.dispatch_action(action.boxed_clone())
+                                })
                                 .tooltip(move |cx| {
                                     Tooltip::for_action(tooltip.clone(), &*action, cx)
                                 }),