From 057b235c564cf8d5c7e87314f19247eccbfb241e Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Wed, 13 Dec 2023 20:42:27 -0500 Subject: [PATCH] Implement `VisibleOnHover` for `IconButton` (#3642) This PR implements the `VisibleOnHover` trait for `IconButton`s. I noticed that in a lot of places we were wrapping an `IconButton` in an extra `div` just so we could call `visible_on_hover` on it. By implementing the trait on `IconButton` directly it allows us to avoid the interstitial `div` entirely. Release Notes: - N/A --- crates/collab_ui2/src/chat_panel.rs | 20 ++-- crates/collab_ui2/src/collab_panel.rs | 94 +++++++++---------- crates/editor2/src/editor.rs | 21 ++--- .../ui2/src/components/button/button_like.rs | 13 ++- .../ui2/src/components/button/icon_button.rs | 7 ++ crates/ui2/src/visible_on_hover.rs | 8 +- 6 files changed, 81 insertions(+), 82 deletions(-) diff --git a/crates/collab_ui2/src/chat_panel.rs b/crates/collab_ui2/src/chat_panel.rs index 9096770166f40eb9da465612a6e408ac9b350c7f..f3f2a37171b32c5be5c610725951b199b1a28525 100644 --- a/crates/collab_ui2/src/chat_panel.rs +++ b/crates/collab_ui2/src/chat_panel.rs @@ -384,7 +384,13 @@ impl ChatPanel { .right_2() .w_8() .visible_on_hover("") - .child(render_remove(message_id_to_remove, cx)), + .children(message_id_to_remove.map(|message_id| { + IconButton::new(("remove", message_id), Icon::XCircle).on_click( + cx.listener(move |this, _, cx| { + this.remove_message(message_id, cx); + }), + ) + })), ) .into_any() } @@ -524,18 +530,6 @@ impl ChatPanel { } } -fn render_remove(message_id_to_remove: Option, cx: &mut ViewContext) -> AnyElement { - if let Some(message_id) = message_id_to_remove { - IconButton::new(("remove", message_id), Icon::XCircle) - .on_click(cx.listener(move |this, _, cx| { - this.remove_message(message_id, cx); - })) - .into_any_element() - } else { - div().into_any_element() - } -} - impl EventEmitter for ChatPanel {} impl Render for ChatPanel { diff --git a/crates/collab_ui2/src/collab_panel.rs b/crates/collab_ui2/src/collab_panel.rs index cf1ac5205a170c631289ab503fca8871bdbe2752..95ca7cfd2545180fad9135a37fb80526e53ec030 100644 --- a/crates/collab_ui2/src/collab_panel.rs +++ b/crates/collab_ui2/src/collab_panel.rs @@ -2289,18 +2289,15 @@ impl CollabPanel { let button = match section { Section::ActiveCall => channel_link.map(|channel_link| { let channel_link_copy = channel_link.clone(); - div() + IconButton::new("channel-link", Icon::Copy) + .icon_size(IconSize::Small) + .size(ButtonSize::None) .visible_on_hover("section-header") - .child( - IconButton::new("channel-link", Icon::Copy) - .icon_size(IconSize::Small) - .size(ButtonSize::None) - .on_click(move |_, cx| { - let item = ClipboardItem::new(channel_link_copy.clone()); - cx.write_to_clipboard(item) - }) - .tooltip(|cx| Tooltip::text("Copy channel link", cx)), - ) + .on_click(move |_, cx| { + let item = ClipboardItem::new(channel_link_copy.clone()); + cx.write_to_clipboard(item) + }) + .tooltip(|cx| Tooltip::text("Copy channel link", cx)) .into_any_element() }), Section::Contacts => Some( @@ -2380,17 +2377,16 @@ impl CollabPanel { }) .when(!calling, |el| { el.child( - div().visible_on_hover("").child( - IconButton::new("remove_contact", Icon::Close) - .icon_color(Color::Muted) - .tooltip(|cx| Tooltip::text("Remove Contact", cx)) - .on_click(cx.listener({ - let github_login = github_login.clone(); - move |this, _, cx| { - this.remove_contact(user_id, &github_login, cx); - } - })), - ), + IconButton::new("remove_contact", Icon::Close) + .icon_color(Color::Muted) + .visible_on_hover("") + .tooltip(|cx| Tooltip::text("Remove Contact", cx)) + .on_click(cx.listener({ + let github_login = github_login.clone(); + move |this, _, cx| { + this.remove_contact(user_id, &github_login, cx); + } + })), ) }), ) @@ -2619,38 +2615,32 @@ impl CollabPanel { .end_slot( h_stack() .child( - div() - .id("channel_chat") - .when(!has_messages_notification, |el| el.visible_on_hover("")) - .child( - IconButton::new("channel_chat", Icon::MessageBubbles) - .icon_color(if has_messages_notification { - Color::Default - } else { - Color::Muted - }) - .on_click(cx.listener(move |this, _, cx| { - this.join_channel_chat(channel_id, cx) - })) - .tooltip(|cx| Tooltip::text("Open channel chat", cx)), - ), + IconButton::new("channel_chat", Icon::MessageBubbles) + .icon_color(if has_messages_notification { + Color::Default + } else { + Color::Muted + }) + .when(!has_messages_notification, |this| { + this.visible_on_hover("") + }) + .on_click(cx.listener(move |this, _, cx| { + this.join_channel_chat(channel_id, cx) + })) + .tooltip(|cx| Tooltip::text("Open channel chat", cx)), ) .child( - div() - .id("channel_notes") - .when(!has_notes_notification, |el| el.visible_on_hover("")) - .child( - IconButton::new("channel_notes", Icon::File) - .icon_color(if has_notes_notification { - Color::Default - } else { - Color::Muted - }) - .on_click(cx.listener(move |this, _, cx| { - this.open_channel_notes(channel_id, cx) - })) - .tooltip(|cx| Tooltip::text("Open channel notes", cx)), - ), + IconButton::new("channel_notes", Icon::File) + .icon_color(if has_notes_notification { + Color::Default + } else { + Color::Muted + }) + .when(!has_notes_notification, |this| this.visible_on_hover("")) + .on_click(cx.listener(move |this, _, cx| { + this.open_channel_notes(channel_id, cx) + })) + .tooltip(|cx| Tooltip::text("Open channel notes", cx)), ), ), ) diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index aba8dbd4d4292173e0cdd3e372e10f63d3f0b629..89b5fd2efb91ff43f4683b0923f5658888ecc72a 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -9763,18 +9763,15 @@ pub fn diagnostic_block_renderer(diagnostic: Diagnostic, is_valid: bool) -> Rend .px_1p5() .child(HighlightedLabel::new(line.clone(), highlights.clone())) .child( - div() - .border() - .border_color(gpui::red()) - .visible_on_hover(group_id) - .child( - IconButton::new(copy_id.clone(), Icon::Copy) - .icon_color(Color::Muted) - .size(ButtonSize::Compact) - .style(ButtonStyle::Transparent) - .on_click(cx.listener(move |_, _, cx| write_to_clipboard)) - .tooltip(|cx| Tooltip::text("Copy diagnostic message", cx)), - ), + div().border().border_color(gpui::red()).child( + IconButton::new(copy_id.clone(), Icon::Copy) + .icon_color(Color::Muted) + .size(ButtonSize::Compact) + .style(ButtonStyle::Transparent) + .visible_on_hover(group_id) + .on_click(cx.listener(move |_, _, cx| write_to_clipboard)) + .tooltip(|cx| Tooltip::text("Copy diagnostic message", cx)), + ), ) })) .into_any_element() diff --git a/crates/ui2/src/components/button/button_like.rs b/crates/ui2/src/components/button/button_like.rs index 7203b3494f1ce0608b5e8528e5d136932d9b63f9..8255490476b8a58f4a39d03de10412151f320db6 100644 --- a/crates/ui2/src/components/button/button_like.rs +++ b/crates/ui2/src/components/button/button_like.rs @@ -2,7 +2,6 @@ use gpui::{relative, DefiniteLength}; use gpui::{rems, transparent_black, AnyElement, AnyView, ClickEvent, Div, Hsla, Rems, Stateful}; use smallvec::SmallVec; -use crate::h_stack; use crate::prelude::*; pub trait ButtonCommon: Clickable + Disableable { @@ -250,6 +249,7 @@ impl ButtonSize { /// This is also used to build the prebuilt buttons. #[derive(IntoElement)] pub struct ButtonLike { + base: Div, id: ElementId, pub(super) style: ButtonStyle, pub(super) disabled: bool, @@ -264,6 +264,7 @@ pub struct ButtonLike { impl ButtonLike { pub fn new(id: impl Into) -> Self { Self { + base: div(), id: id.into(), style: ButtonStyle::default(), disabled: false, @@ -331,6 +332,13 @@ impl ButtonCommon for ButtonLike { } } +impl VisibleOnHover for ButtonLike { + fn visible_on_hover(mut self, group_name: impl Into) -> Self { + self.base = self.base.visible_on_hover(group_name); + self + } +} + impl ParentElement for ButtonLike { fn children_mut(&mut self) -> &mut SmallVec<[AnyElement; 2]> { &mut self.children @@ -341,7 +349,8 @@ impl RenderOnce for ButtonLike { type Rendered = Stateful
; fn render(self, cx: &mut WindowContext) -> Self::Rendered { - h_stack() + self.base + .h_flex() .id(self.id.clone()) .group("") .flex_none() diff --git a/crates/ui2/src/components/button/icon_button.rs b/crates/ui2/src/components/button/icon_button.rs index f49120e90c1c1afea3857152ee6d286cb7051596..3a53bb6cb0178ffb3046d385debd3c0ecb35d23d 100644 --- a/crates/ui2/src/components/button/icon_button.rs +++ b/crates/ui2/src/components/button/icon_button.rs @@ -98,6 +98,13 @@ impl ButtonCommon for IconButton { } } +impl VisibleOnHover for IconButton { + fn visible_on_hover(mut self, group_name: impl Into) -> Self { + self.base = self.base.visible_on_hover(group_name); + self + } +} + impl RenderOnce for IconButton { type Rendered = ButtonLike; diff --git a/crates/ui2/src/visible_on_hover.rs b/crates/ui2/src/visible_on_hover.rs index dfab5ab3e6476fa39600ad4a9f405daeb93ea7bc..aefa7ac10c50646edca24f6ff2219418a44c8be9 100644 --- a/crates/ui2/src/visible_on_hover.rs +++ b/crates/ui2/src/visible_on_hover.rs @@ -1,13 +1,15 @@ use gpui::{InteractiveElement, SharedString, Styled}; -pub trait VisibleOnHover: InteractiveElement + Styled + Sized { +pub trait VisibleOnHover { /// Sets the element to only be visible when the specified group is hovered. /// /// Pass `""` as the `group_name` to use the global group. + fn visible_on_hover(self, group_name: impl Into) -> Self; +} + +impl VisibleOnHover for E { fn visible_on_hover(self, group_name: impl Into) -> Self { self.invisible() .group_hover(group_name, |style| style.visible()) } } - -impl VisibleOnHover for E {}