Implement `VisibleOnHover` for `IconButton` (#3642)

Marshall Bowers created

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

Change summary

crates/collab_ui2/src/chat_panel.rs             | 20 +--
crates/collab_ui2/src/collab_panel.rs           | 94 ++++++++----------
crates/editor2/src/editor.rs                    | 21 +--
crates/ui2/src/components/button/button_like.rs | 13 ++
crates/ui2/src/components/button/icon_button.rs |  7 +
crates/ui2/src/visible_on_hover.rs              |  8 +
6 files changed, 81 insertions(+), 82 deletions(-)

Detailed changes

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<u64>, cx: &mut ViewContext<ChatPanel>) -> 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<Event> for ChatPanel {}
 
 impl Render for ChatPanel {

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)),
                             ),
                     ),
             )

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()

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<ElementId>) -> 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<SharedString>) -> 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<Div>;
 
     fn render(self, cx: &mut WindowContext) -> Self::Rendered {
-        h_stack()
+        self.base
+            .h_flex()
             .id(self.id.clone())
             .group("")
             .flex_none()

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<SharedString>) -> Self {
+        self.base = self.base.visible_on_hover(group_name);
+        self
+    }
+}
+
 impl RenderOnce for IconButton {
     type Rendered = ButtonLike;
 

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<SharedString>) -> Self;
+}
+
+impl<E: InteractiveElement + Styled> VisibleOnHover for E {
     fn visible_on_hover(self, group_name: impl Into<SharedString>) -> Self {
         self.invisible()
             .group_hover(group_name, |style| style.visible())
     }
 }
-
-impl<E: InteractiveElement + Styled> VisibleOnHover for E {}