From 7a5851e1558d7e0f4a064a51438db966b297a27f Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Sat, 22 Nov 2025 22:26:26 -0300 Subject: [PATCH] ui: Remove `CheckboxWithLabel` and improve `Switch` and `Checkbox` (#43343) This PR finally removes the `CheckboxWithLabel` component, which is not fully needed given the `Checkbox` can take a `label` method. Then, took advantage of the opportunity to add more methods with regards to label customization (position, size, and color) in both the `Checkbox` and `Switch` components. Release Notes: - N/A --- .../src/collab_panel/channel_modal.rs | 24 +- crates/debugger_ui/src/new_process_modal.rs | 23 +- crates/ui/src/components/toggle.rs | 288 +++++++----------- crates/workspace/src/theme_preview.rs | 6 +- 4 files changed, 144 insertions(+), 197 deletions(-) diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index e558835dbaf0e34e2efa1b4f64fd8f6cb96016c5..9d882562cab710f562145087e5c38474fda4808b 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -10,7 +10,7 @@ use gpui::{ }; use picker::{Picker, PickerDelegate}; use std::sync::Arc; -use ui::{Avatar, CheckboxWithLabel, ContextMenu, ListItem, ListItemSpacing, prelude::*}; +use ui::{Avatar, Checkbox, ContextMenu, ListItem, ListItemSpacing, prelude::*}; use util::TryFutureExt; use workspace::{ModalView, notifications::DetachAndPromptErr}; @@ -165,16 +165,18 @@ impl Render for ChannelModal { .h(rems_from_px(22.)) .justify_between() .line_height(rems(1.25)) - .child(CheckboxWithLabel::new( - "is-public", - Label::new("Public").size(LabelSize::Small), - if visibility == ChannelVisibility::Public { - ui::ToggleState::Selected - } else { - ui::ToggleState::Unselected - }, - cx.listener(Self::set_channel_visibility), - )) + .child( + Checkbox::new( + "is-public", + if visibility == ChannelVisibility::Public { + ui::ToggleState::Selected + } else { + ui::ToggleState::Unselected + }, + ) + .label("Public") + .on_click(cx.listener(Self::set_channel_visibility)), + ) .children( Some( Button::new("copy-link", "Copy Link") diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index c343110b47527adc0f8d4e3e097a5f769b80682c..4460284fadc8f43c86400d1f47d3e9d68c42b39b 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -25,9 +25,9 @@ use settings::Settings; use task::{DebugScenario, RevealTarget, VariableName, ZedDebugConfig}; use theme::ThemeSettings; use ui::{ - CheckboxWithLabel, ContextMenu, DropdownMenu, FluentBuilder, IconWithIndicator, Indicator, - KeyBinding, ListItem, ListItemSpacing, ToggleButtonGroup, ToggleButtonSimple, ToggleState, - Tooltip, prelude::*, + ContextMenu, DropdownMenu, FluentBuilder, IconWithIndicator, Indicator, KeyBinding, ListItem, + ListItemSpacing, Switch, SwitchLabelPosition, ToggleButtonGroup, ToggleButtonSimple, + ToggleState, Tooltip, prelude::*, }; use util::{ResultExt, debug_panic, rel_path::RelPath, shell::ShellKind}; use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane}; @@ -910,13 +910,12 @@ impl ConfigureMode { .child(render_editor(&self.cwd, window, cx)), ) .child( - CheckboxWithLabel::new( - "debugger-stop-on-entry", - Label::new("Stop on Entry") - .size(LabelSize::Small) - .color(Color::Muted), - self.stop_on_entry, - { + Switch::new("debugger-stop-on-entry", self.stop_on_entry) + .label("Stop on Entry") + .label_position(SwitchLabelPosition::Start) + .label_size(LabelSize::Default) + .color(ui::SwitchColor::Accent) + .on_click({ let this = cx.weak_entity(); move |state, _, cx| { this.update(cx, |this, _| { @@ -924,9 +923,7 @@ impl ConfigureMode { }) .ok(); } - }, - ) - .checkbox_position(ui::IconPosition::End), + }), ) } } diff --git a/crates/ui/src/components/toggle.rs b/crates/ui/src/components/toggle.rs index ab66b71996d6c7b64d0d3867ab73bd9727816316..a41dce6c61de1cabdbccee1478afe143feee4987 100644 --- a/crates/ui/src/components/toggle.rs +++ b/crates/ui/src/components/toggle.rs @@ -1,7 +1,8 @@ use gpui::{ - AnyElement, AnyView, ClickEvent, ElementId, Hsla, IntoElement, Styled, Window, div, hsla, - prelude::*, + AnyElement, AnyView, ClickEvent, ElementId, Hsla, IntoElement, KeybindingKeystroke, Keystroke, + Styled, Window, div, hsla, prelude::*, }; +use settings::KeybindSource; use std::{rc::Rc, sync::Arc}; use crate::utils::is_light; @@ -50,6 +51,8 @@ pub struct Checkbox { style: ToggleStyle, tooltip: Option AnyView>>, label: Option, + label_size: LabelSize, + label_color: Color, } impl Checkbox { @@ -64,6 +67,8 @@ impl Checkbox { style: ToggleStyle::default(), tooltip: None, label: None, + label_size: LabelSize::Default, + label_color: Color::Muted, placeholder: false, } } @@ -128,6 +133,16 @@ impl Checkbox { self.label = Some(label.into()); self } + + pub fn label_size(mut self, size: LabelSize) -> Self { + self.label_size = size; + self + } + + pub fn label_color(mut self, color: Color) -> Self { + self.label_color = color; + self + } } impl Checkbox { @@ -155,7 +170,6 @@ impl Checkbox { } } - /// container size pub fn container_size() -> Pixels { px(20.0) } @@ -169,6 +183,7 @@ impl RenderOnce for Checkbox { } else { Color::Selected }; + let icon = match self.toggle_state { ToggleState::Selected => { if self.placeholder { @@ -232,6 +247,7 @@ impl RenderOnce for Checkbox { h_flex() .id(self.id) + .cursor_pointer() .gap(DynamicSpacing::Base06.rems(cx)) .child(checkbox) .when_some( @@ -242,110 +258,15 @@ impl RenderOnce for Checkbox { }) }, ) - // TODO: Allow label size to be different from default. - // TODO: Allow label color to be different from muted. .when_some(self.label, |this, label| { - this.child(Label::new(label).color(Color::Muted)) - }) - .when_some(self.tooltip, |this, tooltip| { - this.tooltip(move |window, cx| tooltip(window, cx)) - }) - } -} - -/// A [`Checkbox`] that has a [`Label`]. -#[derive(IntoElement, RegisterComponent)] -pub struct CheckboxWithLabel { - id: ElementId, - label: Label, - checked: ToggleState, - on_click: Arc, - filled: bool, - style: ToggleStyle, - checkbox_position: IconPosition, -} - -// TODO: Remove `CheckboxWithLabel` now that `label` is a method of `Checkbox`. -impl CheckboxWithLabel { - /// Creates a checkbox with an attached label. - pub fn new( - id: impl Into, - label: Label, - checked: ToggleState, - on_click: impl Fn(&ToggleState, &mut Window, &mut App) + 'static, - ) -> Self { - Self { - id: id.into(), - label, - checked, - on_click: Arc::new(on_click), - filled: false, - style: ToggleStyle::default(), - checkbox_position: IconPosition::Start, - } - } - - /// Sets the style of the checkbox using the specified [`ToggleStyle`]. - pub fn style(mut self, style: ToggleStyle) -> Self { - self.style = style; - self - } - - /// Match the style of the checkbox to the current elevation using [`ToggleStyle::ElevationBased`]. - pub fn elevation(mut self, elevation: ElevationIndex) -> Self { - self.style = ToggleStyle::ElevationBased(elevation); - self - } - - /// Sets the `fill` setting of the checkbox, indicating whether it should be filled. - pub fn fill(mut self) -> Self { - self.filled = true; - self - } - - pub fn checkbox_position(mut self, position: IconPosition) -> Self { - self.checkbox_position = position; - self - } -} - -impl RenderOnce for CheckboxWithLabel { - fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { - h_flex() - .gap(DynamicSpacing::Base08.rems(cx)) - .when(self.checkbox_position == IconPosition::Start, |this| { this.child( - Checkbox::new(self.id.clone(), self.checked) - .style(self.style.clone()) - .when(self.filled, Checkbox::fill) - .on_click({ - let on_click = self.on_click.clone(); - move |checked, window, cx| { - (on_click)(checked, window, cx); - } - }), + Label::new(label) + .color(self.label_color) + .size(self.label_size), ) }) - .child( - div() - .id(SharedString::from(format!("{}-label", self.id))) - .on_click({ - let on_click = self.on_click.clone(); - move |_event, window, cx| { - (on_click)(&self.checked.inverse(), window, cx); - } - }) - .child(self.label), - ) - .when(self.checkbox_position == IconPosition::End, |this| { - this.child( - Checkbox::new(self.id.clone(), self.checked) - .style(self.style) - .when(self.filled, Checkbox::fill) - .on_click(move |checked, window, cx| { - (self.on_click)(checked, window, cx); - }), - ) + .when_some(self.tooltip, |this, tooltip| { + this.tooltip(move |window, cx| tooltip(window, cx)) }) } } @@ -412,6 +333,14 @@ impl From for Color { } } +/// Defines the color for a switch component. +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Default)] +pub enum SwitchLabelPosition { + Start, + #[default] + End, +} + /// # Switch /// /// Switches are used to represent opposite states, such as enabled or disabled. @@ -422,6 +351,9 @@ pub struct Switch { disabled: bool, on_click: Option>, label: Option, + label_position: Option, + label_size: LabelSize, + full_width: bool, key_binding: Option, color: SwitchColor, tab_index: Option, @@ -436,6 +368,9 @@ impl Switch { disabled: false, on_click: None, label: None, + label_position: None, + label_size: LabelSize::Small, + full_width: false, key_binding: None, color: SwitchColor::default(), tab_index: None, @@ -469,6 +404,24 @@ impl Switch { self } + pub fn label_position( + mut self, + label_position: impl Into>, + ) -> Self { + self.label_position = label_position.into(); + self + } + + pub fn label_size(mut self, size: LabelSize) -> Self { + self.label_size = size; + self + } + + pub fn full_width(mut self, full_width: bool) -> Self { + self.full_width = full_width; + self + } + /// Display the keybinding that triggers the switch action. pub fn key_binding(mut self, key_binding: impl Into>) -> Self { self.key_binding = key_binding.into(); @@ -503,6 +456,7 @@ impl RenderOnce for Switch { }; let group_id = format!("switch_group_{:?}", self.id); + let label = self.label; let switch = div() .id((self.id.clone(), "switch")) @@ -555,9 +509,27 @@ impl RenderOnce for Switch { h_flex() .id(self.id) - .gap(DynamicSpacing::Base06.rems(cx)) .cursor_pointer() + .gap(DynamicSpacing::Base06.rems(cx)) + .when(self.full_width, |this| this.w_full().justify_between()) + .when( + self.label_position == Some(SwitchLabelPosition::Start), + |this| { + this.when_some(label.clone(), |this, label| { + this.child(Label::new(label).size(self.label_size)) + }) + }, + ) .child(switch) + .when( + self.label_position == Some(SwitchLabelPosition::End), + |this| { + this.when_some(label, |this, label| { + this.child(Label::new(label).size(self.label_size)) + }) + }, + ) + .children(self.key_binding) .when_some( self.on_click.filter(|_| !self.disabled), |this, on_click| { @@ -566,10 +538,6 @@ impl RenderOnce for Switch { }) }, ) - .when_some(self.label, |this, label| { - this.child(Label::new(label).size(LabelSize::Small)) - }) - .children(self.key_binding) } } @@ -1070,75 +1038,55 @@ impl Component for Switch { "With Label", vec![ single_example( - "Label", - Switch::new("switch_with_label", ToggleState::Selected) + "Start Label", + Switch::new("switch_with_label_start", ToggleState::Selected) + .label("Always save on quit") + .label_position(SwitchLabelPosition::Start) + .into_any_element(), + ), + single_example( + "End Label", + Switch::new("switch_with_label_end", ToggleState::Selected) .label("Always save on quit") + .label_position(SwitchLabelPosition::End) + .into_any_element(), + ), + single_example( + "Default Size Label", + Switch::new( + "switch_with_label_default_size", + ToggleState::Selected, + ) + .label("Always save on quit") + .label_size(LabelSize::Default) + .into_any_element(), + ), + single_example( + "Small Size Label", + Switch::new("switch_with_label_small_size", ToggleState::Selected) + .label("Always save on quit") + .label_size(LabelSize::Small) .into_any_element(), ), - // TODO: Where did theme_preview_keybinding go? - // single_example( - // "Keybinding", - // Switch::new("switch_with_keybinding", ToggleState::Selected) - // .key_binding(theme_preview_keybinding("cmd-shift-e")) - // .into_any_element(), - // ), ], ), + example_group_with_title( + "With Keybinding", + vec![single_example( + "Keybinding", + Switch::new("switch_with_keybinding", ToggleState::Selected) + .key_binding(Some(KeyBinding::from_keystrokes( + vec![KeybindingKeystroke::from_keystroke( + Keystroke::parse("cmd-s").unwrap(), + )] + .into(), + KeybindSource::Base, + ))) + .into_any_element(), + )], + ), ]) .into_any_element(), ) } } - -impl Component for CheckboxWithLabel { - fn scope() -> ComponentScope { - ComponentScope::Input - } - - fn description() -> Option<&'static str> { - Some("A checkbox component with an attached label") - } - - fn preview(_window: &mut Window, _cx: &mut App) -> Option { - Some( - v_flex() - .gap_6() - .children(vec![example_group_with_title( - "States", - vec![ - single_example( - "Unselected", - CheckboxWithLabel::new( - "checkbox_with_label_unselected", - Label::new("Always save on quit"), - ToggleState::Unselected, - |_, _, _| {}, - ) - .into_any_element(), - ), - single_example( - "Indeterminate", - CheckboxWithLabel::new( - "checkbox_with_label_indeterminate", - Label::new("Always save on quit"), - ToggleState::Indeterminate, - |_, _, _| {}, - ) - .into_any_element(), - ), - single_example( - "Selected", - CheckboxWithLabel::new( - "checkbox_with_label_selected", - Label::new("Always save on quit"), - ToggleState::Selected, - |_, _, _| {}, - ) - .into_any_element(), - ), - ], - )]) - .into_any_element(), - ) - } -} diff --git a/crates/workspace/src/theme_preview.rs b/crates/workspace/src/theme_preview.rs index 94a280b4da1283178201898bd3e8c2c71e5f0b1f..f978da706b7476d04bf656ed63faf5bd38b83d20 100644 --- a/crates/workspace/src/theme_preview.rs +++ b/crates/workspace/src/theme_preview.rs @@ -6,9 +6,9 @@ use strum::IntoEnumIterator; use theme::all_theme_colors; use ui::{ AudioStatus, Avatar, AvatarAudioStatusIndicator, AvatarAvailabilityIndicator, ButtonLike, - Checkbox, CheckboxWithLabel, CollaboratorAvailability, ContentGroup, DecoratedIcon, - ElevationIndex, Facepile, IconDecoration, Indicator, KeybindingHint, Switch, TintColor, - Tooltip, prelude::*, utils::calculate_contrast_ratio, + Checkbox, CollaboratorAvailability, ContentGroup, DecoratedIcon, ElevationIndex, Facepile, + IconDecoration, Indicator, KeybindingHint, Switch, TintColor, Tooltip, prelude::*, + utils::calculate_contrast_ratio, }; use crate::{Item, Workspace};