audit button

crates/core/src/button.rs

@@ -11,6 +11,7 @@ use gpui_native_utils::focus_ring;
 use std::rc::Rc;
 
 use crate::common::{Sizable, h_flex};
+use crate::spinner::Spinner;
 
 /// The single activation handler, shared by pointer, Enter, and Space.
 type ClickHandler = Box<dyn Fn(&ClickEvent, &mut Window, &mut App)>;
@@ -20,10 +21,10 @@ type TooltipBuilder = Rc<dyn Fn(&mut Window, &mut App) -> AnyView>;
 /// Which visual treatment a button uses.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
 pub enum ButtonVariant {
-    /// Accent-filled; the platform's default action.
+    /// The platform's default action.
     #[default]
     Primary,
-    /// Neutral and bordered; the everyday companion to `Primary`.
+    /// The everyday companion to `Primary`.
     Secondary,
     /// Unpainted until hovered, for toolbars and dense UI.
     Ghost,
@@ -40,6 +41,7 @@ pub struct Button {
     variant: ButtonVariant,
     size: Size,
     disabled: bool,
+    loading: bool,
     tab_stop: bool,
     debug_selector: Option<Rc<dyn Fn() -> String>>,
     a11y_label: Option<SharedString>,
@@ -60,6 +62,7 @@ impl Button {
             variant: ButtonVariant::default(),
             size: Size::default(),
             disabled: false,
+            loading: false,
             tab_stop: true,
             debug_selector: None,
             a11y_label: None,
@@ -119,6 +122,12 @@ impl Button {
         self
     }
 
+    /// Shows the spinner in place of the icon and suppresses activation.
+    pub fn loading(mut self, loading: bool) -> Self {
+        self.loading = loading;
+        self
+    }
+
     /// Overrides the accessibility label, which defaults to the visible label.
     pub fn accessibility_label<L>(mut self, label: L) -> Self
     where
@@ -149,11 +158,11 @@ impl RenderOnce for Button {
         let theme = cx.theme();
         let colors = theme.colors;
         let ramps = theme.ramps;
+        let borders = theme.borders;
         let metrics = theme.sizes.resolve(self.size);
 
         let on_click = self.on_click;
         let a11y_label = self.a11y_label.or_else(|| self.label.clone());
-        let icon_only = self.label.is_none() && self.icon.is_some();
 
         let (background, foreground, hover, pressed) = match self.variant {
             ButtonVariant::Primary => (
@@ -183,35 +192,47 @@ impl RenderOnce for Button {
         };
 
         h_flex()
-            .id(self.id)
+            .id(self.id.clone())
             .when_some(self.debug_selector, |button, selector| {
                 button.debug_selector(move || selector())
             })
             .role(Role::Button)
             .justify_center()
-            .when(icon_only, |button| button.size(metrics.control_height))
-            .when(!icon_only, |button| {
-                button.h(metrics.control_height).px(metrics.padding_x)
+            .h(metrics.control_height)
+            .when(self.icon.is_some(), |button| {
+                button.px(metrics.dense_padding_x)
             })
+            .when(self.icon.is_none(), |button| button.px(metrics.padding_x))
             .line_height(relative(1.0))
-            .gap_1p5()
+            .gap_2()
             .rounded(theme.radius.control)
             .text_size(metrics.text_size)
-            .when(self.variant == ButtonVariant::Secondary, |button| {
-                button.border_1().border_color(colors.border)
-            })
+            .when(
+                borders.buttons && self.variant != ButtonVariant::Ghost,
+                |button| button.border_1().border_color(colors.border),
+            )
             .when_some(background, |button, background| button.bg(background))
             .text_color(foreground)
             .when_some(a11y_label, |button, label| button.aria_label(label))
-            .when_some(self.icon, |button, role| {
-                button.child(icon(role).size(metrics.icon_size).color(foreground))
+            .when(self.loading, |button| {
+                button.child(
+                    Spinner::new((self.id.clone(), "spinner"))
+                        .with_size(self.size)
+                        .variant(crate::spinner::SpinnerVariant::Icon)
+                        .color(foreground),
+                )
+            })
+            .when(!self.loading, |button| {
+                button.when_some(self.icon, |button, role| {
+                    button.child(icon(role).size(metrics.icon_size).color(foreground))
+                })
             })
             .when_some(self.label, |button, label| button.child(label))
             .when(self.disabled, |button| button.opacity(0.5))
             .when_some(self.tooltip, |button, tooltip| {
                 button.tooltip(move |window, cx| tooltip(window, cx))
             })
-            .when(!self.disabled, |button| {
+            .when(!self.disabled && !self.loading, |button| {
                 button
                     .cursor_pointer()
                     .focusable()

crates/core/src/spinner.rs

@@ -41,6 +41,7 @@ pub struct Spinner {
     value: Option<f32>,
     variant: SpinnerVariant,
     size: Size,
+    color: Option<Hsla>,
     a11y_label: Option<SharedString>,
 }
 
@@ -55,10 +56,17 @@ impl Spinner {
             value: None,
             variant: SpinnerVariant::default(),
             size: Size::default(),
+            color: None,
             a11y_label: None,
         }
     }
 
+    /// Overrides the arc color, for accent-filled surfaces.
+    pub fn color(mut self, color: Hsla) -> Self {
+        self.color = Some(color);
+        self
+    }
+
     /// Sets the spinner's visual language.
     pub fn variant(mut self, variant: SpinnerVariant) -> Self {
         self.variant = variant;
@@ -93,7 +101,7 @@ impl RenderOnce for Spinner {
         let side = theme.sizes.resolve(self.size).icon_size;
         let stroke = side * RING_STROKE;
         let track = theme.colors.border;
-        let accent = theme.ramps.primary.base;
+        let accent = self.color.unwrap_or(theme.ramps.primary.base);
         let value = self.value.map(|value| value.clamp(0.0, 1.0));
 
         div()

crates/core/tests/button.rs

@@ -11,6 +11,7 @@ use gpui_native_theme::Theme;
 struct Harness {
     clicks: Rc<RefCell<usize>>,
     disabled: bool,
+    loading: bool,
 }
 
 impl Render for Harness {
@@ -20,14 +21,18 @@ impl Render for Harness {
             Button::new("test-button")
                 .label("Save")
                 .disabled(self.disabled)
+                .loading(self.loading)
                 .on_click(move |_, _, _| *clicks.borrow_mut() += 1),
         )
     }
 }
 
-/// A maximized window whose button stretches across the top 28px, so
+/// A maximized window whose button stretches across the top band, so
 /// `point(px(60.), px(14.))` always lands on it.
-fn button_window(disabled: bool) -> (VisualTestContext, AnyWindowHandle, Rc<RefCell<usize>>) {
+fn button_window(
+    disabled: bool,
+    loading: bool,
+) -> (VisualTestContext, AnyWindowHandle, Rc<RefCell<usize>>) {
     let mut cx = TestAppContext::single();
     cx.update(Theme::init);
     let clicks = Rc::new(RefCell::new(0));
@@ -35,6 +40,7 @@ fn button_window(disabled: bool) -> (VisualTestContext, AnyWindowHandle, Rc<RefC
     let window = cx.add_window(move |_, _| Harness {
         clicks: harness_clicks,
         disabled,
+        loading,
     });
     let window = window.into();
     cx.update_window(window, |_, window, cx| {
@@ -53,21 +59,28 @@ fn draw(cx: &mut VisualTestContext, window: AnyWindowHandle) {
 
 #[test]
 fn pointer_click_fires_on_click() {
-    let (mut cx, _window, clicks) = button_window(false);
+    let (mut cx, _window, clicks) = button_window(false, false);
     cx.simulate_click(point(px(60.), px(14.)), Modifiers::default());
     assert_eq!(*clicks.borrow(), 1);
 }
 
 #[test]
 fn disabled_button_ignores_clicks() {
-    let (mut cx, _window, clicks) = button_window(true);
+    let (mut cx, _window, clicks) = button_window(true, false);
+    cx.simulate_click(point(px(60.), px(14.)), Modifiers::default());
+    assert_eq!(*clicks.borrow(), 0);
+}
+
+#[test]
+fn loading_button_ignores_clicks() {
+    let (mut cx, _window, clicks) = button_window(false, true);
     cx.simulate_click(point(px(60.), px(14.)), Modifiers::default());
     assert_eq!(*clicks.borrow(), 0);
 }
 
 #[test]
 fn enter_and_space_fire_on_click_while_focused() {
-    let (mut cx, window, clicks) = button_window(false);
+    let (mut cx, window, clicks) = button_window(false, false);
     cx.simulate_click(point(px(60.), px(14.)), Modifiers::default());
     assert_eq!(*clicks.borrow(), 1, "the pointer click itself activates");