Remove the `SwitchWithLabel` component (#23240)

Danilo Leal and Agus Zubiaga created

This PR removes the `SwitchWithLabel` component because we're adding
`label` as a method to `Switch`. Thus, we no longer need an extra
component just to append a label. Additionally, we're also adding
`keybinding` as a method.

Release Notes:

- N/A

---------

Co-authored-by: Agus Zubiaga <hi@aguz.me>

Change summary

crates/assistant2/src/message_editor.rs   |  27 ++--
crates/ui/src/components/button/button.rs |   2 
crates/ui/src/components/toggle.rs        | 126 ++++++++----------------
crates/workspace/src/theme_preview.rs     |   3 
4 files changed, 58 insertions(+), 100 deletions(-)

Detailed changes

crates/assistant2/src/message_editor.rs 🔗

@@ -13,8 +13,7 @@ use rope::Point;
 use settings::Settings;
 use theme::{get_ui_font_size, ThemeSettings};
 use ui::{
-    prelude::*, ButtonLike, ElevationIndex, KeyBinding, PopoverMenu, PopoverMenuHandle,
-    SwitchWithLabel,
+    prelude::*, ButtonLike, ElevationIndex, KeyBinding, PopoverMenu, PopoverMenuHandle, Switch,
 };
 use workspace::Workspace;
 
@@ -315,19 +314,17 @@ impl Render for MessageEditor {
                     .child(
                         h_flex()
                             .justify_between()
-                            .child(SwitchWithLabel::new(
-                                "use-tools",
-                                Label::new("Tools").size(LabelSize::Small),
-                                self.use_tools.into(),
-                                cx.listener(|this, selection, _cx| {
-                                    this.use_tools = match selection {
-                                        ToggleState::Selected => true,
-                                        ToggleState::Unselected | ToggleState::Indeterminate => {
-                                            false
-                                        }
-                                    };
-                                }),
-                            ))
+                            .child(
+                                Switch::new("use-tools", self.use_tools.into())
+                                    .label("Tools")
+                                    .on_click(cx.listener(|this, selection, _cx| {
+                                        this.use_tools = match selection {
+                                            ToggleState::Selected => true,
+                                            ToggleState::Unselected
+                                            | ToggleState::Indeterminate => false,
+                                        };
+                                    })),
+                            )
                             .child(
                                 h_flex().gap_1().child(self.model_selector.clone()).child(
                                     ButtonLike::new("chat")

crates/ui/src/components/button/button.rs 🔗

@@ -181,7 +181,7 @@ impl Button {
         self
     }
 
-    /// Binds a key combination to the button for keyboard shortcuts.
+    /// Display the keybinding that triggers the button action.
     pub fn key_binding(mut self, key_binding: impl Into<Option<KeyBinding>>) -> Self {
         self.key_binding = key_binding.into();
         self

crates/ui/src/components/toggle.rs 🔗

@@ -2,10 +2,10 @@ use gpui::{div, hsla, prelude::*, AnyView, ElementId, Hsla, IntoElement, Styled,
 use std::sync::Arc;
 
 use crate::utils::is_light;
-use crate::{prelude::*, ElevationIndex};
+use crate::{prelude::*, ElevationIndex, KeyBinding};
 use crate::{Color, Icon, IconName, ToggleState};
 
-// TODO: Checkbox, CheckboxWithLabel, Switch, SwitchWithLabel all could be
+// TODO: Checkbox, CheckboxWithLabel, and Switch could all be
 // restructured to use a ToggleLike, similar to Button/Buttonlike, Label/Labellike
 
 /// Creates a new checkbox.
@@ -273,6 +273,8 @@ pub struct Switch {
     toggle_state: ToggleState,
     disabled: bool,
     on_click: Option<Box<dyn Fn(&ToggleState, &mut WindowContext) + 'static>>,
+    label: Option<SharedString>,
+    key_binding: Option<KeyBinding>,
 }
 
 impl Switch {
@@ -283,6 +285,8 @@ impl Switch {
             toggle_state: state,
             disabled: false,
             on_click: None,
+            label: None,
+            key_binding: None,
         }
     }
 
@@ -300,6 +304,18 @@ impl Switch {
         self.on_click = Some(Box::new(handler));
         self
     }
+
+    /// Sets the label of the [`Switch`].
+    pub fn label(mut self, label: impl Into<SharedString>) -> Self {
+        self.label = Some(label.into());
+        self
+    }
+
+    /// Display the keybinding that triggers the switch action.
+    pub fn key_binding(mut self, key_binding: impl Into<Option<KeyBinding>>) -> Self {
+        self.key_binding = key_binding.into();
+        self
+    }
 }
 
 impl RenderOnce for Switch {
@@ -333,9 +349,7 @@ impl RenderOnce for Switch {
 
         let group_id = format!("switch_group_{:?}", self.id);
 
-        h_flex()
-            .id(self.id)
-            .items_center()
+        let switch = h_flex()
             .w(DynamicSpacing::Base32.rems(cx))
             .h(DynamicSpacing::Base20.rems(cx))
             .group(group_id.clone())
@@ -363,60 +377,22 @@ impl RenderOnce for Switch {
                             })
                             .opacity(thumb_opacity),
                     ),
-            )
+            );
+
+        h_flex()
+            .id(self.id)
+            .gap(DynamicSpacing::Base06.rems(cx))
+            .child(switch)
             .when_some(
                 self.on_click.filter(|_| !self.disabled),
                 |this, on_click| {
                     this.on_click(move |_, cx| on_click(&self.toggle_state.inverse(), cx))
                 },
             )
-    }
-}
-
-/// A [`Switch`] that has a [`Label`].
-#[derive(IntoElement)]
-pub struct SwitchWithLabel {
-    id: ElementId,
-    label: Label,
-    checked: ToggleState,
-    on_click: Arc<dyn Fn(&ToggleState, &mut WindowContext) + 'static>,
-}
-
-impl SwitchWithLabel {
-    /// Creates a switch with an attached label.
-    pub fn new(
-        id: impl Into<ElementId>,
-        label: Label,
-        checked: ToggleState,
-        on_click: impl Fn(&ToggleState, &mut WindowContext) + 'static,
-    ) -> Self {
-        Self {
-            id: id.into(),
-            label,
-            checked,
-            on_click: Arc::new(on_click),
-        }
-    }
-}
-
-impl RenderOnce for SwitchWithLabel {
-    fn render(self, cx: &mut WindowContext) -> impl IntoElement {
-        h_flex()
-            .gap(DynamicSpacing::Base08.rems(cx))
-            .child(Switch::new(self.id.clone(), self.checked).on_click({
-                let on_click = self.on_click.clone();
-                move |checked, cx| {
-                    (on_click)(checked, cx);
-                }
-            }))
-            .child(
-                div()
-                    .id(SharedString::from(format!("{}-label", self.id)))
-                    .on_click(move |_event, cx| {
-                        (self.on_click)(&self.checked.inverse(), cx);
-                    })
-                    .child(self.label),
-            )
+            .when_some(self.label, |this, label| {
+                this.child(Label::new(label).size(LabelSize::Small))
+            })
+            .children(self.key_binding)
     }
 }
 
@@ -647,6 +623,21 @@ impl ComponentPreview for Switch {
                     ),
                 ],
             ),
+            example_group_with_title(
+                "Label Permutations",
+                vec![
+                    single_example(
+                        "Label",
+                        Switch::new("switch_with_label", ToggleState::Selected)
+                            .label("Always save on quit"),
+                    ),
+                    single_example(
+                        "Keybinding",
+                        Switch::new("switch_with_label", ToggleState::Selected)
+                            .key_binding(theme_preview_keybinding("cmd-shift-e")),
+                    ),
+                ],
+            ),
         ]
     }
 }
@@ -688,32 +679,3 @@ impl ComponentPreview for CheckboxWithLabel {
         ])]
     }
 }
-
-impl ComponentPreview for SwitchWithLabel {
-    fn description() -> impl Into<Option<&'static str>> {
-        "A switch with an associated label, allowing users to select an option while providing a descriptive text."
-    }
-
-    fn examples(_: &mut WindowContext) -> Vec<ComponentExampleGroup<Self>> {
-        vec![example_group(vec![
-            single_example(
-                "Off",
-                SwitchWithLabel::new(
-                    "switch_with_label_unselected",
-                    Label::new("Always save on quit"),
-                    ToggleState::Unselected,
-                    |_, _| {},
-                ),
-            ),
-            single_example(
-                "On",
-                SwitchWithLabel::new(
-                    "switch_with_label_selected",
-                    Label::new("Always save on quit"),
-                    ToggleState::Selected,
-                    |_, _| {},
-                ),
-            ),
-        ])]
-    }
-}

crates/workspace/src/theme_preview.rs 🔗

@@ -6,7 +6,7 @@ use ui::{
     element_cell, prelude::*, string_cell, utils::calculate_contrast_ratio, AudioStatus,
     Availability, Avatar, AvatarAudioStatusIndicator, AvatarAvailabilityIndicator, ButtonLike,
     Checkbox, CheckboxWithLabel, ContentGroup, DecoratedIcon, ElevationIndex, Facepile,
-    IconDecoration, Indicator, Switch, SwitchWithLabel, Table, TintColor, Tooltip,
+    IconDecoration, Indicator, Switch, Table, TintColor, Tooltip,
 };
 
 use crate::{Item, Workspace};
@@ -379,7 +379,6 @@ impl ThemePreview {
             .child(IconDecoration::render_component_previews(cx))
             .child(Indicator::render_component_previews(cx))
             .child(Switch::render_component_previews(cx))
-            .child(SwitchWithLabel::render_component_previews(cx))
             .child(Table::render_component_previews(cx))
     }