From a8d4abea69f9e13a02ad2eee6f1478859404642c Mon Sep 17 00:00:00 2001
From: Jason Lee <huacnlee@gmail.com>
Date: Fri, 13 Sep 2024 12:00:47 +0800
Subject: [PATCH] button: Improve ButtonGroup to add style, size methods.
 (#241)

- Add `ButtonStyled` trait to apply the style for Button and
ButtonGroup.
---
 crates/app/src/story_workspace.rs   |   2 +-
 crates/story/src/button_story.rs    | 144 ++++++++++++++--------------
 crates/story/src/modal_story.rs     |   2 +-
 crates/story/src/popup_story.rs     |   2 +-
 crates/ui/src/button.rs             |  88 +++++++++--------
 crates/ui/src/button_group.rs       |  71 ++++++++++----
 crates/ui/src/clipboard.rs          |   5 +-
 crates/ui/src/dock/tab_panel.rs     |   2 +-
 crates/ui/src/drawer.rs             |   9 +-
 crates/ui/src/input/clear_button.rs |   5 +-
 crates/ui/src/modal.rs              |   6 +-
 crates/ui/src/notification.rs       |   7 +-
 crates/ui/src/theme.rs              |   8 +-
 crates/ui/src/time/calendar.rs      |   8 +-
 14 files changed, 211 insertions(+), 148 deletions(-)

diff --git a/crates/app/src/story_workspace.rs b/crates/app/src/story_workspace.rs
index 150e4b78..4be99858 100644
--- a/crates/app/src/story_workspace.rs
+++ b/crates/app/src/story_workspace.rs
@@ -9,7 +9,7 @@ use story::{
     SwitchStory, TableStory, TextStory, TooltipStory,
 };
 use ui::{
-    button::Button,
+    button::{Button, ButtonStyled as _},
     color_picker::{ColorPicker, ColorPickerEvent},
     dock::{DockArea, DockEvent, DockItem, DockItemState},
     h_flex,
diff --git a/crates/story/src/button_story.rs b/crates/story/src/button_story.rs
index c2e8b013..018e9ec6 100644
--- a/crates/story/src/button_story.rs
+++ b/crates/story/src/button_story.rs
@@ -4,7 +4,7 @@ use gpui::{
 };
 
 use ui::{
-    button::{Button, ButtonCustomStyle},
+    button::{Button, ButtonCustomStyle, ButtonStyled as _},
     button_group::ButtonGroup,
     checkbox::Checkbox,
     h_flex,
@@ -21,7 +21,7 @@ pub struct ButtonStory {
     loading: bool,
     selected: bool,
     compact: bool,
-    multiple: bool,
+    toggle_multiple: bool,
 }
 
 impl ButtonStory {
@@ -32,7 +32,7 @@ impl ButtonStory {
             loading: false,
             selected: false,
             compact: false,
-            multiple: false,
+            toggle_multiple: false,
         })
     }
 
@@ -71,7 +71,7 @@ impl Render for ButtonStory {
         let loading = self.loading;
         let selected = self.selected;
         let compact = self.compact;
-        let multiple = self.multiple;
+        let toggle_multiple = self.toggle_multiple;
 
         v_flex()
             .gap_6()
@@ -436,79 +436,81 @@ impl Render for ButtonStory {
                     ),
             )
             .child(
-                section("Button Group", cx).child(
-                    ButtonGroup::new("button-group")
-                        .disabled(disabled)
-                        .child(
-                            Button::new("button-one", cx)
-                                .label("One")
-                                .disabled(disabled)
-                                .selected(selected)
-                                .loading(loading)
-                                .when(compact, |this| this.compact())
-                                .on_click(Self::on_click),
-                        )
-                        .child(
-                            Button::new("button-two", cx)
-                                .label("Two")
-                                .disabled(disabled)
-                                .selected(selected)
-                                .loading(loading)
-                                .when(compact, |this| this.compact())
-                                .on_click(Self::on_click),
-                        )
-                        .child(
-                            Button::new("button-three", cx)
-                                .label("Three")
-                                .disabled(disabled)
-                                .selected(selected)
-                                .loading(loading)
-                                .when(compact, |this| this.compact())
-                                .on_click(Self::on_click),
-                        ),
-                ),
-            )
-            .child(
-                section("Toggle Button Group", cx)
+                section("Button Group", cx)
                     .child(
-                        Checkbox::new("multiple-button")
-                            .label("Multiple")
-                            .checked(self.multiple)
-                            .on_click(cx.listener(|view, _, cx| {
-                                view.multiple = !view.multiple;
-                                cx.notify();
-                            })),
+                        ButtonGroup::new("button-group")
+                            .small()
+                            .disabled(disabled)
+                            .child(
+                                Button::new("button-one", cx)
+                                    .label("One")
+                                    .disabled(disabled)
+                                    .selected(selected)
+                                    .when(compact, |this| this.compact())
+                                    .on_click(Self::on_click),
+                            )
+                            .child(
+                                Button::new("button-two", cx)
+                                    .label("Two")
+                                    .disabled(disabled)
+                                    .selected(selected)
+                                    .when(compact, |this| this.compact())
+                                    .on_click(Self::on_click),
+                            )
+                            .child(
+                                Button::new("button-three", cx)
+                                    .label("Three")
+                                    .disabled(disabled)
+                                    .selected(selected)
+                                    .when(compact, |this| this.compact())
+                                    .on_click(Self::on_click),
+                            ),
                     )
                     .child(
-                        ButtonGroup::new("toggle-button-group")
-                            .multiple(multiple)
+                        h_flex()
+                            .gap_2()
                             .child(
-                                Button::new("disabled-toggle-button", cx)
-                                    .label("Disabled")
-                                    .selected(disabled),
+                                Checkbox::new("multiple-button")
+                                    .label("Multiple")
+                                    .checked(toggle_multiple)
+                                    .on_click(cx.listener(|view, _, cx| {
+                                        view.toggle_multiple = !view.toggle_multiple;
+                                        cx.notify();
+                                    })),
                             )
                             .child(
-                                Button::new("loading-toggle-button", cx)
-                                    .label("Loading")
-                                    .selected(loading),
-                            )
-                            .child(
-                                Button::new("selected-toggle-button", cx)
-                                    .label("Selected")
-                                    .selected(selected),
-                            )
-                            .child(
-                                Button::new("compact-toggle-button", cx)
-                                    .label("Compact")
-                                    .selected(compact),
-                            )
-                            .on_click(cx.listener(|view, selected: &Vec<usize>, cx| {
-                                view.disabled = selected.contains(&0);
-                                view.loading = selected.contains(&1);
-                                view.selected = selected.contains(&2);
-                                view.compact = selected.contains(&3);
-                                cx.notify();
-                            })),
+                                ButtonGroup::new("toggle-button-group")
+                                    .primary()
+                                    .compact()
+                                    .multiple(toggle_multiple)
+                                    .child(
+                                        Button::new("disabled-toggle-button", cx)
+                                            .label("Disabled")
+                                            .selected(disabled),
+                                    )
+                                    .child(
+                                        Button::new("loading-toggle-button", cx)
+                                            .label("Loading")
+                                            .selected(loading),
+                                    )
+                                    .child(
+                                        Button::new("selected-toggle-button", cx)
+                                            .label("Selected")
+                                            .selected(selected),
+                                    )
+                                    .child(
+                                        Button::new("compact-toggle-button", cx)
+                                            .label("Compact")
+                                            .selected(compact),
+                                    )
+                                    .on_click(cx.listener(|view, selected: &Vec<usize>, cx| {
+                                        view.disabled = selected.contains(&0);
+                                        view.loading = selected.contains(&1);
+                                        view.selected = selected.contains(&2);
+                                        view.compact = selected.contains(&3);
+                                        cx.notify();
+                                    })),
+                            ),
                     ),
             )
             .child(
diff --git a/crates/story/src/modal_story.rs b/crates/story/src/modal_story.rs
index 67ef8867..70222112 100644
--- a/crates/story/src/modal_story.rs
+++ b/crates/story/src/modal_story.rs
@@ -8,7 +8,7 @@ use gpui::{
 };
 
 use ui::{
-    button::{Button, ButtonStyle},
+    button::{Button, ButtonStyle, ButtonStyled as _},
     checkbox::Checkbox,
     date_picker::DatePicker,
     h_flex,
diff --git a/crates/story/src/popup_story.rs b/crates/story/src/popup_story.rs
index 189aca34..9643a06c 100644
--- a/crates/story/src/popup_story.rs
+++ b/crates/story/src/popup_story.rs
@@ -6,7 +6,7 @@ use gpui::{
 };
 use serde::Deserialize;
 use ui::{
-    button::Button,
+    button::{Button, ButtonStyled as _},
     context_menu::ContextMenuExt,
     divider::Divider,
     h_flex,
diff --git a/crates/ui/src/button.rs b/crates/ui/src/button.rs
index f3d80346..201359a4 100644
--- a/crates/ui/src/button.rs
+++ b/crates/ui/src/button.rs
@@ -34,6 +34,45 @@ pub struct ButtonCustomStyle {
     active: Hsla,
 }
 
+pub trait ButtonStyled: Sized {
+    fn with_style(self, style: ButtonStyle) -> Self;
+
+    /// With the primary style for the Button.
+    fn primary(self) -> Self {
+        self.with_style(ButtonStyle::Primary)
+    }
+
+    /// With the danger style for the Button.
+    fn danger(self) -> Self {
+        self.with_style(ButtonStyle::Danger)
+    }
+
+    /// With the outline style for the Button.
+    fn outline(self) -> Self {
+        self.with_style(ButtonStyle::Outline)
+    }
+
+    /// With the ghost style for the Button.
+    fn ghost(self) -> Self {
+        self.with_style(ButtonStyle::Ghost)
+    }
+
+    /// With the link style for the Button.
+    fn link(self) -> Self {
+        self.with_style(ButtonStyle::Link)
+    }
+
+    /// With the text style for the Button, it will no padding look like a normal text.
+    fn text(self) -> Self {
+        self.with_style(ButtonStyle::Text)
+    }
+
+    /// With the custom style for the Button.
+    fn custom(self, style: ButtonCustomStyle) -> Self {
+        self.with_style(ButtonStyle::Custom(style))
+    }
+}
+
 impl ButtonCustomStyle {
     pub fn new(cx: &WindowContext) -> Self {
         Self {
@@ -149,48 +188,6 @@ impl Button {
         }
     }
 
-    /// With the primary style for the Button.
-    pub fn primary(mut self) -> Self {
-        self.style = ButtonStyle::Primary;
-        self
-    }
-
-    /// With the secondary style for the Button.
-    pub fn danger(mut self) -> Self {
-        self.style = ButtonStyle::Danger;
-        self
-    }
-
-    /// With the ghost style for the Button.
-    pub fn ghost(mut self) -> Self {
-        self.style = ButtonStyle::Ghost;
-        self
-    }
-
-    /// With the outline style for the Button.
-    pub fn outline(mut self) -> Self {
-        self.style = ButtonStyle::Outline;
-        self
-    }
-
-    /// With the link style for the Button.
-    pub fn link(mut self) -> Self {
-        self.style = ButtonStyle::Link;
-        self
-    }
-
-    /// With the text style for the Button, it will no padding look like a normal text.
-    pub fn text(mut self) -> Self {
-        self.style = ButtonStyle::Text;
-        self
-    }
-
-    /// With the custom style for the Button.
-    pub fn custom(mut self, custom: ButtonCustomStyle) -> Self {
-        self.style = ButtonStyle::Custom(custom);
-        self
-    }
-
     /// Set the border radius of the Button.
     pub fn rounded(mut self, rounded: impl Into<ButtonRounded>) -> Self {
         self.rounded = rounded.into();
@@ -277,6 +274,13 @@ impl Sizable for Button {
     }
 }
 
+impl ButtonStyled for Button {
+    fn with_style(mut self, style: ButtonStyle) -> Self {
+        self.style = style;
+        self
+    }
+}
+
 impl Styled for Button {
     fn style(&mut self) -> &mut gpui::StyleRefinement {
         self.base.style()
diff --git a/crates/ui/src/button_group.rs b/crates/ui/src/button_group.rs
index 13e23d65..3db8a1f2 100644
--- a/crates/ui/src/button_group.rs
+++ b/crates/ui/src/button_group.rs
@@ -4,7 +4,10 @@ use gpui::{
 };
 use std::{cell::Cell, rc::Rc};
 
-use crate::{button::Button, Disableable};
+use crate::{
+    button::{Button, ButtonStyle, ButtonStyled},
+    Disableable, Sizable, Size,
+};
 
 #[derive(IntoElement)]
 pub struct ButtonGroup {
@@ -13,6 +16,12 @@ pub struct ButtonGroup {
     children: Vec<Button>,
     multiple: bool,
     disabled: bool,
+
+    // The button props
+    compact: Option<bool>,
+    style: Option<ButtonStyle>,
+    size: Option<Size>,
+
     on_click: Option<Box<dyn Fn(&Vec<usize>, &mut WindowContext) + 'static>>,
 }
 
@@ -30,6 +39,9 @@ impl ButtonGroup {
             base: div(),
             children: Vec::new(),
             id: id.into(),
+            style: None,
+            size: None,
+            compact: None,
             multiple: false,
             disabled: false,
             on_click: None,
@@ -42,34 +54,56 @@ impl ButtonGroup {
         self
     }
 
-    /// Sets the multiple selection mode.
+    /// With the multiple selection mode.
     pub fn multiple(mut self, multiple: bool) -> Self {
         self.multiple = multiple;
         self
     }
 
+    /// With the compact mode for the ButtonGroup.
+    pub fn compact(mut self) -> Self {
+        self.compact = Some(true);
+        self
+    }
+
     /// Sets the on_click handler for the ButtonGroup.
+    ///
+    /// The handler first argument is a vector of the selected button indices.
     pub fn on_click(mut self, handler: impl Fn(&Vec<usize>, &mut WindowContext) + 'static) -> Self {
         self.on_click = Some(Box::new(handler));
         self
     }
 }
 
+impl Sizable for ButtonGroup {
+    fn with_size(mut self, size: impl Into<Size>) -> Self {
+        self.size = Some(size.into());
+        self
+    }
+}
+
 impl Styled for ButtonGroup {
     fn style(&mut self) -> &mut gpui::StyleRefinement {
         self.base.style()
     }
 }
 
+impl ButtonStyled for ButtonGroup {
+    fn with_style(mut self, style: ButtonStyle) -> Self {
+        self.style = Some(style);
+        self
+    }
+}
+
 impl RenderOnce for ButtonGroup {
     fn render(self, _cx: &mut WindowContext) -> impl IntoElement {
         let children_len = self.children.len();
-        let mut selected: Vec<usize> = Vec::new();
-        let shared_state = Rc::new(Cell::new(None)); // Shared state to store the child index
+        let mut selected_ixs: Vec<usize> = Vec::new();
+        let state = Rc::new(Cell::new(None));
 
-        for (child_index, child) in self.children.iter().enumerate() {
+        for (ix, child) in self.children.iter().enumerate() {
             if child.selected {
-                selected.push(child_index);
+                selected_ixs.push(ix);
             }
         }
 
@@ -82,7 +116,7 @@ impl RenderOnce for ButtonGroup {
                     .into_iter()
                     .enumerate()
                     .map(|(child_index, child)| {
-                        let shared_state_clone = Rc::clone(&shared_state);
+                        let state = Rc::clone(&state);
                         let child = if children_len == 1 {
                             child
                         } else if child_index == 0 {
@@ -127,8 +161,11 @@ impl RenderOnce for ButtonGroup {
                                 })
                         }
                         .stop_propagation(false)
-                        .on_click(move |_, cx| {
-                            shared_state_clone.set(Some(child_index));
+                        .when_some(self.size, |this, size| this.with_size(size))
+                        .when_some(self.style, |this, style| this.style(style))
+                        .when_some(self.compact, |this, _| this.compact())
+                        .on_click(move |_, _| {
+                            state.set(Some(child_index));
                         });
 
                         child
@@ -138,21 +175,21 @@ impl RenderOnce for ButtonGroup {
                 self.on_click.filter(|_| !self.disabled),
                 move |this, on_click| {
                     this.on_click(move |_, cx| {
-                        let mut selected = selected.clone();
-                        if let Some(index) = shared_state.get() {
+                        let mut selected_ixs = selected_ixs.clone();
+                        if let Some(ix) = state.get() {
                             if self.multiple {
-                                if let Some(pos) = selected.iter().position(|&i| i == index) {
-                                    selected.remove(pos); // Toggle off if already selected
+                                if let Some(pos) = selected_ixs.iter().position(|&i| i == ix) {
+                                    selected_ixs.remove(pos);
                                 } else {
-                                    selected.push(index); // Toggle on if not selected
+                                    selected_ixs.push(ix);
                                 }
                             } else {
-                                selected.clear(); // Clear the existing selection
-                                selected.push(index); // Replace with the new selection
+                                selected_ixs.clear();
+                                selected_ixs.push(ix);
                             }
                         }
 
-                        on_click(&selected, cx);
+                        on_click(&selected_ixs, cx);
                     })
                 },
             )
diff --git a/crates/ui/src/clipboard.rs b/crates/ui/src/clipboard.rs
index 87875557..0ce9716d 100644
--- a/crates/ui/src/clipboard.rs
+++ b/crates/ui/src/clipboard.rs
@@ -3,7 +3,10 @@ use gpui::{
     RenderOnce, SharedString, Styled, WindowContext,
 };
 
-use crate::{button::Button, h_flex, IconName, Sizable};
+use crate::{
+    button::{Button, ButtonStyled as _},
+    h_flex, IconName, Sizable as _,
+};
 
 #[derive(IntoElement)]
 pub struct Clipboard {
diff --git a/crates/ui/src/dock/tab_panel.rs b/crates/ui/src/dock/tab_panel.rs
index c8dcd618..9eee596d 100644
--- a/crates/ui/src/dock/tab_panel.rs
+++ b/crates/ui/src/dock/tab_panel.rs
@@ -9,7 +9,7 @@ use gpui::{
 use rust_i18n::t;
 
 use crate::{
-    button::Button,
+    button::{Button, ButtonStyled as _},
     dock::DockItemInfo,
     h_flex,
     popup_menu::{PopupMenu, PopupMenuExt},
diff --git a/crates/ui/src/drawer.rs b/crates/ui/src/drawer.rs
index 47d1e58f..24c53d6f 100644
--- a/crates/ui/src/drawer.rs
+++ b/crates/ui/src/drawer.rs
@@ -8,8 +8,13 @@ use gpui::{
 };
 
 use crate::{
-    button::Button, h_flex, modal::overlay_color, root::ContextModal as _, scroll::ScrollbarAxis,
-    theme::ActiveTheme, v_flex, IconName, Placement, Sizable, StyledExt as _,
+    button::{Button, ButtonStyled as _},
+    h_flex,
+    modal::overlay_color,
+    root::ContextModal as _,
+    scroll::ScrollbarAxis,
+    theme::ActiveTheme,
+    v_flex, IconName, Placement, Sizable, StyledExt as _,
 };
 
 #[derive(IntoElement)]
diff --git a/crates/ui/src/input/clear_button.rs b/crates/ui/src/input/clear_button.rs
index 64204fd6..dc15d2b2 100644
--- a/crates/ui/src/input/clear_button.rs
+++ b/crates/ui/src/input/clear_button.rs
@@ -1,6 +1,9 @@
 use gpui::{px, WindowContext};
 
-use crate::{button::Button, IconName, Sizable as _};
+use crate::{
+    button::{Button, ButtonStyled as _},
+    IconName, Sizable as _,
+};
 
 pub(crate) struct ClearButton {}
 
diff --git a/crates/ui/src/modal.rs b/crates/ui/src/modal.rs
index 7deaf7ef..9033a899 100644
--- a/crates/ui/src/modal.rs
+++ b/crates/ui/src/modal.rs
@@ -8,8 +8,10 @@ use gpui::{
 };
 
 use crate::{
-    animation::cubic_bezier, button::Button, theme::ActiveTheme as _, v_flex, ContextModal,
-    IconName, Sizable as _,
+    animation::cubic_bezier,
+    button::{Button, ButtonStyled as _},
+    theme::ActiveTheme as _,
+    v_flex, ContextModal, IconName, Sizable as _,
 };
 
 actions!(modal, [Escape]);
diff --git a/crates/ui/src/notification.rs b/crates/ui/src/notification.rs
index c983b85d..31fe9ecc 100644
--- a/crates/ui/src/notification.rs
+++ b/crates/ui/src/notification.rs
@@ -8,8 +8,11 @@ use gpui::{
 use smol::Timer;
 
 use crate::{
-    animation::cubic_bezier, button::Button, h_flex, theme::ActiveTheme as _, v_flex, Icon,
-    IconName, Sizable as _, StyledExt,
+    animation::cubic_bezier,
+    button::{Button, ButtonStyled as _},
+    h_flex,
+    theme::ActiveTheme as _,
+    v_flex, Icon, IconName, Sizable as _, StyledExt,
 };
 
 pub enum NotificationType {
diff --git a/crates/ui/src/theme.rs b/crates/ui/src/theme.rs
index f9be1479..08fbf1fd 100644
--- a/crates/ui/src/theme.rs
+++ b/crates/ui/src/theme.rs
@@ -182,8 +182,8 @@ impl Colors {
             popover: hsl(0.0, 0.0, 100.0),
             popover_foreground: hsl(240.0, 10.0, 3.9),
             primary: hsl(223.0, 5.9, 10.0),
-            primary_hover: hsl(223.0, 5.9, 30.0),
-            primary_active: hsl(223.0, 5.9, 45.0),
+            primary_hover: hsl(223.0, 5.9, 15.0),
+            primary_active: hsl(223.0, 1.9, 25.0),
             primary_foreground: hsl(223.0, 0.0, 98.0),
             secondary: hsl(240.0, 4.8, 95.9),
             secondary_hover: hsl(240.0, 5.8, 10.).opacity(0.05),
@@ -224,8 +224,8 @@ impl Colors {
             popover: hsl(240.0, 10.0, 3.9),
             popover_foreground: hsl(0.0, 0.0, 98.0),
             primary: hsl(223.0, 0.0, 98.0),
-            primary_hover: hsl(223.0, 0.0, 85.0),
-            primary_active: hsl(223.0, 0.0, 60.0),
+            primary_hover: hsl(223.0, 0.0, 90.0),
+            primary_active: hsl(223.0, 0.0, 80.0),
             primary_foreground: hsl(223.0, 5.9, 10.0),
             secondary: hsl(240.0, 3.7, 15.9),
             secondary_hover: hsl(240.0, 3.7, 20.9).opacity(0.5),
diff --git a/crates/ui/src/time/calendar.rs b/crates/ui/src/time/calendar.rs
index 285bbb0a..9a0f8f42 100644
--- a/crates/ui/src/time/calendar.rs
+++ b/crates/ui/src/time/calendar.rs
@@ -9,7 +9,10 @@ use gpui::{
 use rust_i18n::t;
 
 use crate::{
-    button::Button, h_flex, theme::ActiveTheme, v_flex, Disableable as _, IconName, Selectable,
+    button::{Button, ButtonStyled as _},
+    h_flex,
+    theme::ActiveTheme,
+    v_flex, Disableable as _, IconName, Selectable,
 };
 
 use super::utils::days_in_month;
@@ -593,7 +596,8 @@ impl Calendar {
                         let active = (ix + 1) as u8 == self.current_month;
 
                         self.item_button(ix, month.to_string(), active, false, false, cx)
-                            .w(relative(0.3)).text_sm()
+                            .w(relative(0.3))
+                            .text_sm()
                             .on_click(cx.listener(move |view, _, cx| {
                                 view.current_month = (ix + 1) as u8;
                                 view.set_view_mode(ViewMode::Day, cx);