From ef551cedef35fc8847c6ea612a2d85a65ea8030c Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 15 Feb 2024 21:00:30 -0500 Subject: [PATCH] Add `CheckboxWithLabel` component (#7881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR builds on top of #7878 by adding a general-purpose `CheckboxWithLabel` component to use for checkboxes that have attached labels. This component encompasses the functionality of allowing to click on the label to toggle the value of the checkbox. There was only one other occurrence of a checkbox with a label—the "Public" checkbox in the channel management modal—and this has been updated to use `CheckboxWithLabel`. Resolves #7794. Release Notes: - Added support for clicking the label of the "Public" checkbox in the channel management modal to toggle the value ([#7794](https://github.com/zed-industries/zed/issues/7794)). --- .../src/collab_panel/channel_modal.rs | 28 ++-- crates/ui/src/components/checkbox.rs | 147 +----------------- crates/ui/src/components/checkbox/checkbox.rs | 144 +++++++++++++++++ .../checkbox/checkbox_with_label.rs | 49 ++++++ crates/welcome/src/welcome.rs | 53 +------ 5 files changed, 212 insertions(+), 209 deletions(-) create mode 100644 crates/ui/src/components/checkbox/checkbox.rs create mode 100644 crates/ui/src/components/checkbox/checkbox_with_label.rs diff --git a/crates/collab_ui/src/collab_panel/channel_modal.rs b/crates/collab_ui/src/collab_panel/channel_modal.rs index 826a61eb48..f3f77a28f6 100644 --- a/crates/collab_ui/src/collab_panel/channel_modal.rs +++ b/crates/collab_ui/src/collab_panel/channel_modal.rs @@ -11,7 +11,7 @@ use gpui::{ }; use picker::{Picker, PickerDelegate}; use std::sync::Arc; -use ui::{prelude::*, Avatar, Checkbox, ContextMenu, ListItem, ListItemSpacing}; +use ui::{prelude::*, Avatar, CheckboxWithLabel, ContextMenu, ListItem, ListItemSpacing}; use util::TryFutureExt; use workspace::{notifications::DetachAndPromptErr, ModalView}; @@ -177,22 +177,16 @@ impl Render for ChannelModal { .h(rems(22. / 16.)) .justify_between() .line_height(rems(1.25)) - .child( - h_flex() - .gap_2() - .child( - Checkbox::new( - "is-public", - if visibility == ChannelVisibility::Public { - ui::Selection::Selected - } else { - ui::Selection::Unselected - }, - ) - .on_click(cx.listener(Self::set_channel_visibility)), - ) - .child(Label::new("Public").size(LabelSize::Small)), - ) + .child(CheckboxWithLabel::new( + "is-public", + Label::new("Public").size(LabelSize::Small), + if visibility == ChannelVisibility::Public { + ui::Selection::Selected + } else { + ui::Selection::Unselected + }, + cx.listener(Self::set_channel_visibility), + )) .children( Some( Button::new("copy-link", "Copy Link") diff --git a/crates/ui/src/components/checkbox.rs b/crates/ui/src/components/checkbox.rs index d0750f7389..732730f216 100644 --- a/crates/ui/src/components/checkbox.rs +++ b/crates/ui/src/components/checkbox.rs @@ -1,144 +1,5 @@ -use gpui::{div, prelude::*, ElementId, IntoElement, Styled, WindowContext}; +mod checkbox; +mod checkbox_with_label; -use crate::prelude::*; -use crate::{Color, Icon, IconName, Selection}; - -/// # Checkbox -/// -/// Checkboxes are used for multiple choices, not for mutually exclusive choices. -/// Each checkbox works independently from other checkboxes in the list, -/// therefore checking an additional box does not affect any other selections. -#[derive(IntoElement)] -pub struct Checkbox { - id: ElementId, - checked: Selection, - disabled: bool, - on_click: Option>, -} - -impl Checkbox { - pub fn new(id: impl Into, checked: Selection) -> Self { - Self { - id: id.into(), - checked, - disabled: false, - on_click: None, - } - } - - pub fn disabled(mut self, disabled: bool) -> Self { - self.disabled = disabled; - self - } - - pub fn on_click(mut self, handler: impl Fn(&Selection, &mut WindowContext) + 'static) -> Self { - self.on_click = Some(Box::new(handler)); - self - } -} - -impl RenderOnce for Checkbox { - fn render(self, cx: &mut WindowContext) -> impl IntoElement { - let group_id = format!("checkbox_group_{:?}", self.id); - - let icon = match self.checked { - Selection::Selected => Some(Icon::new(IconName::Check).size(IconSize::Small).color( - if self.disabled { - Color::Disabled - } else { - Color::Selected - }, - )), - Selection::Indeterminate => Some( - Icon::new(IconName::Dash) - .size(IconSize::Small) - .color(if self.disabled { - Color::Disabled - } else { - Color::Selected - }), - ), - Selection::Unselected => None, - }; - - // A checkbox could be in an indeterminate state, - // for example the indeterminate state could represent: - // - a group of options of which only some are selected - // - an enabled option that is no longer available - // - a previously agreed to license that has been updated - // - // For the sake of styles we treat the indeterminate state as selected, - // but its icon will be different. - let selected = - self.checked == Selection::Selected || self.checked == Selection::Indeterminate; - - // We could use something like this to make the checkbox background when selected: - // - // ```rs - // ... - // .when(selected, |this| { - // this.bg(cx.theme().colors().element_selected) - // }) - // ``` - // - // But we use a match instead here because the checkbox might be disabled, - // and it could be disabled _while_ it is selected, as well as while it is not selected. - let (bg_color, border_color) = match (self.disabled, selected) { - (true, _) => ( - cx.theme().colors().ghost_element_disabled, - cx.theme().colors().border_disabled, - ), - (false, true) => ( - cx.theme().colors().element_selected, - cx.theme().colors().border, - ), - (false, false) => ( - cx.theme().colors().element_background, - cx.theme().colors().border, - ), - }; - - h_flex() - .id(self.id) - // Rather than adding `px_1()` to add some space around the checkbox, - // we use a larger parent element to create a slightly larger - // click area for the checkbox. - .size_5() - // Because we've enlarged the click area, we need to create a - // `group` to pass down interactivity events to the checkbox. - .group(group_id.clone()) - .child( - div() - .flex() - // This prevent the flex element from growing - // or shrinking in response to any size changes - .flex_none() - // The combo of `justify_center()` and `items_center()` - // is used frequently to center elements in a flex container. - // - // We use this to center the icon in the checkbox. - .justify_center() - .items_center() - .m_1() - .size_4() - .rounded_sm() - .bg(bg_color) - .border() - .border_color(border_color) - // We only want the interactivity states to fire when we - // are in a checkbox that isn't disabled. - .when(!self.disabled, |this| { - // Here instead of `hover()` we use `group_hover()` - // to pass it the group id. - this.group_hover(group_id.clone(), |el| { - el.bg(cx.theme().colors().element_hover) - }) - }) - .children(icon), - ) - .when_some( - self.on_click.filter(|_| !self.disabled), - |this, on_click| this.on_click(move |_, cx| on_click(&self.checked.inverse(), cx)), - ) - } -} +pub use checkbox::*; +pub use checkbox_with_label::*; diff --git a/crates/ui/src/components/checkbox/checkbox.rs b/crates/ui/src/components/checkbox/checkbox.rs new file mode 100644 index 0000000000..d0750f7389 --- /dev/null +++ b/crates/ui/src/components/checkbox/checkbox.rs @@ -0,0 +1,144 @@ +use gpui::{div, prelude::*, ElementId, IntoElement, Styled, WindowContext}; + +use crate::prelude::*; +use crate::{Color, Icon, IconName, Selection}; + +/// # Checkbox +/// +/// Checkboxes are used for multiple choices, not for mutually exclusive choices. +/// Each checkbox works independently from other checkboxes in the list, +/// therefore checking an additional box does not affect any other selections. +#[derive(IntoElement)] +pub struct Checkbox { + id: ElementId, + checked: Selection, + disabled: bool, + on_click: Option>, +} + +impl Checkbox { + pub fn new(id: impl Into, checked: Selection) -> Self { + Self { + id: id.into(), + checked, + disabled: false, + on_click: None, + } + } + + pub fn disabled(mut self, disabled: bool) -> Self { + self.disabled = disabled; + self + } + + pub fn on_click(mut self, handler: impl Fn(&Selection, &mut WindowContext) + 'static) -> Self { + self.on_click = Some(Box::new(handler)); + self + } +} + +impl RenderOnce for Checkbox { + fn render(self, cx: &mut WindowContext) -> impl IntoElement { + let group_id = format!("checkbox_group_{:?}", self.id); + + let icon = match self.checked { + Selection::Selected => Some(Icon::new(IconName::Check).size(IconSize::Small).color( + if self.disabled { + Color::Disabled + } else { + Color::Selected + }, + )), + Selection::Indeterminate => Some( + Icon::new(IconName::Dash) + .size(IconSize::Small) + .color(if self.disabled { + Color::Disabled + } else { + Color::Selected + }), + ), + Selection::Unselected => None, + }; + + // A checkbox could be in an indeterminate state, + // for example the indeterminate state could represent: + // - a group of options of which only some are selected + // - an enabled option that is no longer available + // - a previously agreed to license that has been updated + // + // For the sake of styles we treat the indeterminate state as selected, + // but its icon will be different. + let selected = + self.checked == Selection::Selected || self.checked == Selection::Indeterminate; + + // We could use something like this to make the checkbox background when selected: + // + // ```rs + // ... + // .when(selected, |this| { + // this.bg(cx.theme().colors().element_selected) + // }) + // ``` + // + // But we use a match instead here because the checkbox might be disabled, + // and it could be disabled _while_ it is selected, as well as while it is not selected. + let (bg_color, border_color) = match (self.disabled, selected) { + (true, _) => ( + cx.theme().colors().ghost_element_disabled, + cx.theme().colors().border_disabled, + ), + (false, true) => ( + cx.theme().colors().element_selected, + cx.theme().colors().border, + ), + (false, false) => ( + cx.theme().colors().element_background, + cx.theme().colors().border, + ), + }; + + h_flex() + .id(self.id) + // Rather than adding `px_1()` to add some space around the checkbox, + // we use a larger parent element to create a slightly larger + // click area for the checkbox. + .size_5() + // Because we've enlarged the click area, we need to create a + // `group` to pass down interactivity events to the checkbox. + .group(group_id.clone()) + .child( + div() + .flex() + // This prevent the flex element from growing + // or shrinking in response to any size changes + .flex_none() + // The combo of `justify_center()` and `items_center()` + // is used frequently to center elements in a flex container. + // + // We use this to center the icon in the checkbox. + .justify_center() + .items_center() + .m_1() + .size_4() + .rounded_sm() + .bg(bg_color) + .border() + .border_color(border_color) + // We only want the interactivity states to fire when we + // are in a checkbox that isn't disabled. + .when(!self.disabled, |this| { + // Here instead of `hover()` we use `group_hover()` + // to pass it the group id. + this.group_hover(group_id.clone(), |el| { + el.bg(cx.theme().colors().element_hover) + }) + }) + .children(icon), + ) + .when_some( + self.on_click.filter(|_| !self.disabled), + |this, on_click| this.on_click(move |_, cx| on_click(&self.checked.inverse(), cx)), + ) + } +} diff --git a/crates/ui/src/components/checkbox/checkbox_with_label.rs b/crates/ui/src/components/checkbox/checkbox_with_label.rs new file mode 100644 index 0000000000..91cb80d068 --- /dev/null +++ b/crates/ui/src/components/checkbox/checkbox_with_label.rs @@ -0,0 +1,49 @@ +use std::sync::Arc; + +use crate::{prelude::*, Checkbox}; + +/// A [`Checkbox`] that has a [`Label`]. +#[derive(IntoElement)] +pub struct CheckboxWithLabel { + id: ElementId, + label: Label, + checked: Selection, + on_click: Arc, +} + +impl CheckboxWithLabel { + pub fn new( + id: impl Into, + label: Label, + checked: Selection, + on_click: impl Fn(&Selection, &mut WindowContext) + 'static, + ) -> Self { + Self { + id: id.into(), + label, + checked, + on_click: Arc::new(on_click), + } + } +} + +impl RenderOnce for CheckboxWithLabel { + fn render(self, _cx: &mut WindowContext) -> impl IntoElement { + h_flex() + .gap_2() + .child(Checkbox::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), + ) + } +} diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 1f7dfd29e5..61581d5b6f 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -10,7 +10,7 @@ use gpui::{ }; use settings::{Settings, SettingsStore}; use std::sync::Arc; -use ui::{prelude::*, Checkbox}; +use ui::{prelude::*, CheckboxWithLabel}; use vim::VimModeSetting; use workspace::{ dock::DockPosition, @@ -144,7 +144,7 @@ impl Render for WelcomePage { .border_1() .border_color(cx.theme().colors().border) .rounded_md() - .child(LabelledCheckbox::new( + .child(CheckboxWithLabel::new( "enable-vim", Label::new("Enable vim mode"), if VimModeSetting::get_global(cx).0 { @@ -162,7 +162,7 @@ impl Render for WelcomePage { ); }), )) - .child(LabelledCheckbox::new( + .child(CheckboxWithLabel::new( "enable-telemetry", Label::new("Send anonymous usage data"), if TelemetrySettings::get_global(cx).metrics { @@ -188,7 +188,7 @@ impl Render for WelcomePage { }); }), )) - .child(LabelledCheckbox::new( + .child(CheckboxWithLabel::new( "enable-crash", Label::new("Send crash reports"), if TelemetrySettings::get_global(cx).diagnostics { @@ -308,48 +308,3 @@ impl Item for WelcomePage { f(*event) } } - -#[derive(IntoElement)] -struct LabelledCheckbox { - id: ElementId, - label: Label, - checked: Selection, - on_click: Arc, -} - -impl LabelledCheckbox { - pub fn new( - id: impl Into, - label: Label, - checked: Selection, - on_click: impl Fn(&Selection, &mut WindowContext) + 'static, - ) -> Self { - Self { - id: id.into(), - label, - checked, - on_click: Arc::new(on_click), - } - } -} - -impl RenderOnce for LabelledCheckbox { - fn render(self, _cx: &mut WindowContext) -> impl IntoElement { - h_flex() - .gap_2() - .child(Checkbox::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), - ) - } -}