From cc9eff89f5226c419a9e697660208b964c1cf544 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Dec 2023 19:12:01 +0100 Subject: [PATCH] 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 --- crates/assistant2/src/assistant_panel.rs | 8 ++- .../quick_action_bar2/src/quick_action_bar.rs | 19 ++++--- crates/search2/src/buffer_search.rs | 50 +++++++++---------- .../ui2/src/components/button/icon_button.rs | 6 +-- crates/workspace2/src/dock.rs | 5 +- 5 files changed, 48 insertions(+), 40 deletions(-) diff --git a/crates/assistant2/src/assistant_panel.rs b/crates/assistant2/src/assistant_panel.rs index cea7199759477ee441a7f4d4427255199268fc44..202f8a2092152158341cc5aaf0752fa74ea23b2c 100644 --- a/crates/assistant2/src/assistant_panel.rs +++ b/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( diff --git a/crates/quick_action_bar2/src/quick_action_bar.rs b/crates/quick_action_bar2/src/quick_action_bar.rs index 3232de08adea814fcf33e7fc54b598a93bf18c25..3686ace2fb0b73a530a98681a5639c3a1bfc71e4 100644 --- a/crates/quick_action_bar2/src/quick_action_bar.rs +++ b/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, - Styled, Subscription, View, ViewContext, WeakView, + Action, ClickEvent, Div, ElementId, EventEmitter, InteractiveElement, ParentElement, Render, + 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
; fn render(&mut self, cx: &mut ViewContext) -> 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, tooltip: SharedString, tooltip_meta: Option, + on_click: Box, } impl QuickActionBarButton { @@ -163,6 +169,7 @@ impl QuickActionBarButton { toggled: bool, action: Box, tooltip: impl Into, + 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({ - let action = self.action.boxed_clone(); - move |_, cx| cx.dispatch_action(action.boxed_clone()) - }) + .on_click(move |event, cx| (self.on_click)(event, cx)) } } diff --git a/crates/search2/src/buffer_search.rs b/crates/search2/src/buffer_search.rs index b9fa36ef34e4b7413043a8d1c76357c55953270a..da32f51194cc68d869a76ba8b88023f5450b2844 100644 --- a/crates/search2/src/buffer_search.rs +++ b/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::() { - search_bar.update(cx, |this, cx| { - if this.is_dismissed() { - this.show(cx); - } else { - this.dismiss(&Dismiss, cx); - } - }); + search_bar.update(cx, |this, cx| this.toggle(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) { + if self.is_dismissed() { + self.show(cx); + } else { + self.dismiss(&Dismiss, cx); + } + } + pub fn show(&mut self, cx: &mut ViewContext) -> 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) -> 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) { diff --git a/crates/ui2/src/components/button/icon_button.rs b/crates/ui2/src/components/button/icon_button.rs index 94431ef642e08840034ff7f2e4025be5f220daac..f49120e90c1c1afea3857152ee6d286cb7051596 100644 --- a/crates/ui2/src/components/button/icon_button.rs +++ b/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) -> Self { - self.on_click(move |_event, cx| cx.dispatch_action(action.boxed_clone())) - } } impl Disableable for IconButton { diff --git a/crates/workspace2/src/dock.rs b/crates/workspace2/src/dock.rs index abcf5c49bc929c5954c3e40557e419e4ecaca702..a0a90293d603a3afde61dda2c4ca59ec5fa273a1 100644 --- a/crates/workspace2/src/dock.rs +++ b/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) }),