Rework `Disclosure` component (#3442)

Marshall Bowers created

This PR reworks the `Disclosure` component.

The primary goal here was to remove the illegal state where a
`Disclosure` is rendered with a `Toggle::NotToggleable` state, as the
`Disclosure` shouldn't exist at all in this case.

Release Notes:

- N/A

Change summary

crates/collab_ui2/src/collab_panel.rs           | 13 +-
crates/storybook2/src/story_selector.rs         |  2 
crates/ui2/src/components.rs                    |  2 
crates/ui2/src/components/disclosure.rs         | 71 ++++++++++++------
crates/ui2/src/components/list.rs               | 10 +-
crates/ui2/src/components/list/list_header.rs   | 15 ++-
crates/ui2/src/components/list/list_item.rs     | 13 ++-
crates/ui2/src/components/stories.rs            |  2 
crates/ui2/src/components/stories/disclosure.rs | 20 +++++
crates/ui2/src/components/toggle.rs             | 41 ----------
crates/ui2/src/toggleable.rs                    | 38 ++++++++++
crates/ui2/src/ui2.rs                           |  2 
12 files changed, 140 insertions(+), 89 deletions(-)

Detailed changes

crates/collab_ui2/src/collab_panel.rs 🔗

@@ -178,7 +178,7 @@ use serde_derive::{Deserialize, Serialize};
 use settings::{Settings, SettingsStore};
 use ui::{
     h_stack, v_stack, Avatar, Button, Color, Icon, IconButton, IconElement, Label, List,
-    ListHeader, ListItem, Toggle, Tooltip,
+    ListHeader, ListItem, Toggleable, Tooltip,
 };
 use util::{maybe, ResultExt, TryFutureExt};
 use workspace::{
@@ -2534,9 +2534,10 @@ impl CollabPanel {
             .when_some(button, |el, button| el.right_button(button))
             .selected(is_selected)
             .when(can_collapse, |el| {
-                el.toggle(ui::Toggle::Toggled(is_collapsed)).on_toggle(
-                    cx.listener(move |this, _, cx| this.toggle_section_expanded(section, cx)),
-                )
+                el.toggle(Toggleable::Toggleable(is_collapsed.into()))
+                    .on_toggle(
+                        cx.listener(move |this, _, cx| this.toggle_section_expanded(section, cx)),
+                    )
             })
     }
 
@@ -2853,9 +2854,9 @@ impl CollabPanel {
                         ),
                 )
                 .toggle(if has_children {
-                    Toggle::Toggled(disclosed)
+                    Toggleable::Toggleable(disclosed.into())
                 } else {
-                    Toggle::NotToggleable
+                    Toggleable::NotToggleable
                 })
                 .on_toggle(
                     cx.listener(move |this, _, cx| this.toggle_channel_collapsed(channel_id, cx)),

crates/storybook2/src/story_selector.rs 🔗

@@ -16,6 +16,7 @@ pub enum ComponentStory {
     Button,
     Checkbox,
     ContextMenu,
+    Disclosure,
     Focus,
     Icon,
     IconButton,
@@ -36,6 +37,7 @@ impl ComponentStory {
             Self::Button => cx.build_view(|_| ui::ButtonStory).into(),
             Self::Checkbox => cx.build_view(|_| ui::CheckboxStory).into(),
             Self::ContextMenu => cx.build_view(|_| ui::ContextMenuStory).into(),
+            Self::Disclosure => cx.build_view(|_| ui::DisclosureStory).into(),
             Self::Focus => FocusStory::view(cx).into(),
             Self::Icon => cx.build_view(|_| ui::IconStory).into(),
             Self::IconButton => cx.build_view(|_| ui::IconButtonStory).into(),

crates/ui2/src/components.rs 🔗

@@ -13,7 +13,6 @@ mod list;
 mod popover;
 mod slot;
 mod stack;
-mod toggle;
 mod tooltip;
 
 #[cfg(feature = "stories")]
@@ -34,7 +33,6 @@ pub use list::*;
 pub use popover::*;
 pub use slot::*;
 pub use stack::*;
-pub use toggle::*;
 pub use tooltip::*;
 
 #[cfg(feature = "stories")]

crates/ui2/src/components/disclosure.rs 🔗

@@ -1,30 +1,55 @@
 use std::rc::Rc;
 
-use gpui::{div, ClickEvent, Element, IntoElement, ParentElement, WindowContext};
+use gpui::ClickEvent;
 
-use crate::{Color, Icon, IconButton, IconSize, Toggle};
+use crate::prelude::*;
+use crate::{Color, Icon, IconButton, IconSize, ToggleState, Toggleable};
 
-pub fn disclosure_control(
-    toggle: Toggle,
+#[derive(IntoElement)]
+pub struct Disclosure {
+    state: ToggleState,
     on_toggle: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
-) -> impl Element {
-    match (toggle.is_toggleable(), toggle.is_toggled()) {
-        (false, _) => div(),
-        (_, true) => div().child(
-            IconButton::new("toggle", Icon::ChevronDown)
-                .color(Color::Muted)
-                .size(IconSize::Small)
-                .when_some(on_toggle, move |el, on_toggle| {
-                    el.on_click(move |e, cx| on_toggle(e, cx))
-                }),
-        ),
-        (_, false) => div().child(
-            IconButton::new("toggle", Icon::ChevronRight)
-                .color(Color::Muted)
-                .size(IconSize::Small)
-                .when_some(on_toggle, move |el, on_toggle| {
-                    el.on_click(move |e, cx| on_toggle(e, cx))
-                }),
-        ),
+}
+
+impl Disclosure {
+    pub fn new(state: ToggleState) -> Self {
+        Self {
+            state,
+            on_toggle: None,
+        }
+    }
+
+    pub fn from_toggleable(toggleable: Toggleable) -> Option<Self> {
+        match toggleable {
+            Toggleable::Toggleable(state) => Some(Self::new(state)),
+            Toggleable::NotToggleable => None,
+        }
+    }
+
+    pub fn on_toggle(
+        mut self,
+        handler: impl Into<Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>>,
+    ) -> Self {
+        self.on_toggle = handler.into();
+        self
+    }
+}
+
+impl RenderOnce for Disclosure {
+    type Rendered = IconButton;
+
+    fn render(self, _cx: &mut WindowContext) -> Self::Rendered {
+        IconButton::new(
+            "toggle",
+            match self.state {
+                ToggleState::Toggled => Icon::ChevronDown,
+                ToggleState::NotToggled => Icon::ChevronRight,
+            },
+        )
+        .color(Color::Muted)
+        .size(IconSize::Small)
+        .when_some(self.on_toggle, move |this, on_toggle| {
+            this.on_click(move |event, cx| on_toggle(event, cx))
+        })
     }
 }

crates/ui2/src/components/list.rs 🔗

@@ -7,7 +7,7 @@ use gpui::{AnyElement, Div};
 use smallvec::SmallVec;
 
 use crate::prelude::*;
-use crate::{v_stack, Label, Toggle};
+use crate::{v_stack, Label, ToggleState, Toggleable};
 
 pub use list_header::*;
 pub use list_item::*;
@@ -20,7 +20,7 @@ pub struct List {
     /// Defaults to "No items"
     empty_message: SharedString,
     header: Option<ListHeader>,
-    toggle: Toggle,
+    toggle: Toggleable,
     children: SmallVec<[AnyElement; 2]>,
 }
 
@@ -29,7 +29,7 @@ impl List {
         Self {
             empty_message: "No items".into(),
             header: None,
-            toggle: Toggle::NotToggleable,
+            toggle: Toggleable::NotToggleable,
             children: SmallVec::new(),
         }
     }
@@ -44,7 +44,7 @@ impl List {
         self
     }
 
-    pub fn toggle(mut self, toggle: Toggle) -> Self {
+    pub fn toggle(mut self, toggle: Toggleable) -> Self {
         self.toggle = toggle;
         self
     }
@@ -66,7 +66,7 @@ impl RenderOnce for List {
             .children(self.header.map(|header| header))
             .map(|this| match (self.children.is_empty(), self.toggle) {
                 (false, _) => this.children(self.children),
-                (true, Toggle::Toggled(false)) => this,
+                (true, Toggleable::Toggleable(ToggleState::NotToggled)) => this,
                 (true, _) => this.child(Label::new(self.empty_message.clone()).color(Color::Muted)),
             })
     }

crates/ui2/src/components/list/list_header.rs 🔗

@@ -3,7 +3,7 @@ use std::rc::Rc;
 use gpui::{ClickEvent, Div};
 
 use crate::prelude::*;
-use crate::{disclosure_control, h_stack, Icon, IconButton, IconElement, IconSize, Label, Toggle};
+use crate::{h_stack, Disclosure, Icon, IconButton, IconElement, IconSize, Label, Toggleable};
 
 pub enum ListHeaderMeta {
     Tools(Vec<IconButton>),
@@ -17,7 +17,7 @@ pub struct ListHeader {
     label: SharedString,
     left_icon: Option<Icon>,
     meta: Option<ListHeaderMeta>,
-    toggle: Toggle,
+    toggle: Toggleable,
     on_toggle: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
     inset: bool,
     selected: bool,
@@ -30,13 +30,13 @@ impl ListHeader {
             left_icon: None,
             meta: None,
             inset: false,
-            toggle: Toggle::NotToggleable,
+            toggle: Toggleable::NotToggleable,
             on_toggle: None,
             selected: false,
         }
     }
 
-    pub fn toggle(mut self, toggle: Toggle) -> Self {
+    pub fn toggle(mut self, toggle: Toggleable) -> Self {
         self.toggle = toggle;
         self
     }
@@ -73,8 +73,6 @@ impl RenderOnce for ListHeader {
     type Rendered = Div;
 
     fn render(self, cx: &mut WindowContext) -> Self::Rendered {
-        let disclosure_control = disclosure_control(self.toggle, self.on_toggle);
-
         let meta = match self.meta {
             Some(ListHeaderMeta::Tools(icons)) => div().child(
                 h_stack()
@@ -115,7 +113,10 @@ impl RenderOnce for ListHeader {
                                 }))
                                 .child(Label::new(self.label.clone()).color(Color::Muted)),
                         )
-                        .child(disclosure_control),
+                        .children(
+                            Disclosure::from_toggleable(self.toggle)
+                                .map(|disclosure| disclosure.on_toggle(self.on_toggle)),
+                        ),
                 )
                 .child(meta),
         )

crates/ui2/src/components/list/list_item.rs 🔗

@@ -6,7 +6,7 @@ use gpui::{
 use smallvec::SmallVec;
 
 use crate::prelude::*;
-use crate::{disclosure_control, Avatar, GraphicSlot, Icon, IconElement, IconSize, Toggle};
+use crate::{Avatar, Disclosure, GraphicSlot, Icon, IconElement, IconSize, Toggleable};
 
 #[derive(IntoElement)]
 pub struct ListItem {
@@ -17,7 +17,7 @@ pub struct ListItem {
     indent_level: usize,
     indent_step_size: Pixels,
     left_slot: Option<GraphicSlot>,
-    toggle: Toggle,
+    toggle: Toggleable,
     inset: bool,
     on_click: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
     on_toggle: Option<Rc<dyn Fn(&ClickEvent, &mut WindowContext) + 'static>>,
@@ -33,7 +33,7 @@ impl ListItem {
             indent_level: 0,
             indent_step_size: px(12.),
             left_slot: None,
-            toggle: Toggle::NotToggleable,
+            toggle: Toggleable::NotToggleable,
             inset: false,
             on_click: None,
             on_secondary_mouse_down: None,
@@ -70,7 +70,7 @@ impl ListItem {
         self
     }
 
-    pub fn toggle(mut self, toggle: Toggle) -> Self {
+    pub fn toggle(mut self, toggle: Toggleable) -> Self {
         self.toggle = toggle;
         self
     }
@@ -150,7 +150,10 @@ impl RenderOnce for ListItem {
                     .gap_1()
                     .items_center()
                     .relative()
-                    .child(disclosure_control(self.toggle, self.on_toggle))
+                    .children(
+                        Disclosure::from_toggleable(self.toggle)
+                            .map(|disclosure| disclosure.on_toggle(self.on_toggle)),
+                    )
                     .map(|this| match self.left_slot {
                         Some(GraphicSlot::Icon(i)) => this.child(
                             IconElement::new(i)

crates/ui2/src/components/stories.rs 🔗

@@ -2,6 +2,7 @@ mod avatar;
 mod button;
 mod checkbox;
 mod context_menu;
+mod disclosure;
 mod icon;
 mod icon_button;
 mod keybinding;
@@ -13,6 +14,7 @@ pub use avatar::*;
 pub use button::*;
 pub use checkbox::*;
 pub use context_menu::*;
+pub use disclosure::*;
 pub use icon::*;
 pub use icon_button::*;
 pub use keybinding::*;

crates/ui2/src/components/stories/disclosure.rs 🔗

@@ -0,0 +1,20 @@
+use gpui::{Div, Render};
+use story::Story;
+
+use crate::prelude::*;
+use crate::{Disclosure, ToggleState};
+
+pub struct DisclosureStory;
+
+impl Render for DisclosureStory {
+    type Element = Div;
+
+    fn render(&mut self, _cx: &mut ViewContext<Self>) -> Self::Element {
+        Story::container()
+            .child(Story::title_for::<Disclosure>())
+            .child(Story::label("Toggled"))
+            .child(Disclosure::new(ToggleState::Toggled))
+            .child(Story::label("Not Toggled"))
+            .child(Disclosure::new(ToggleState::NotToggled))
+    }
+}

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

@@ -1,41 +0,0 @@
-/// Whether the entry is toggleable, and if so, whether it is currently toggled.
-///
-/// To make an element toggleable, simply add a `Toggle::Toggled(_)` and handle it's cases.
-///
-/// You can check if an element is toggleable with `.is_toggleable()`
-///
-/// Possible values:
-/// - `Toggle::NotToggleable` - The entry is not toggleable
-/// - `Toggle::Toggled(true)` - The entry is toggleable and toggled
-/// - `Toggle::Toggled(false)` - The entry is toggleable and not toggled
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-pub enum Toggle {
-    NotToggleable,
-    Toggled(bool),
-}
-
-impl Toggle {
-    /// Returns true if the entry is toggled (or is not toggleable.)
-    ///
-    /// As element that isn't toggleable is always "expanded" or "enabled"
-    /// returning true in that case makes sense.
-    pub fn is_toggled(&self) -> bool {
-        match self {
-            Self::Toggled(false) => false,
-            _ => true,
-        }
-    }
-
-    pub fn is_toggleable(&self) -> bool {
-        match self {
-            Self::Toggled(_) => true,
-            _ => false,
-        }
-    }
-}
-
-impl From<bool> for Toggle {
-    fn from(toggled: bool) -> Self {
-        Toggle::Toggled(toggled)
-    }
-}

crates/ui2/src/toggleable.rs 🔗

@@ -0,0 +1,38 @@
+/// Whether an element is able to be toggled.
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
+pub enum Toggleable {
+    Toggleable(ToggleState),
+    NotToggleable,
+}
+
+/// The current state of a [`Toggleable`] element.
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
+pub enum ToggleState {
+    Toggled,
+    NotToggled,
+}
+
+impl ToggleState {
+    /// Returns whether an entry is toggled.
+    pub fn is_toggled(&self) -> bool {
+        match self {
+            ToggleState::Toggled => true,
+            ToggleState::NotToggled => false,
+        }
+    }
+}
+
+impl From<bool> for ToggleState {
+    fn from(toggled: bool) -> Self {
+        match toggled {
+            true => Self::Toggled,
+            false => Self::NotToggled,
+        }
+    }
+}
+
+impl From<ToggleState> for bool {
+    fn from(value: ToggleState) -> Self {
+        value.is_toggled()
+    }
+}

crates/ui2/src/ui2.rs 🔗

@@ -20,6 +20,7 @@ pub mod prelude;
 mod selectable;
 mod styled_ext;
 mod styles;
+mod toggleable;
 pub mod utils;
 
 pub use clickable::*;
@@ -30,3 +31,4 @@ pub use prelude::*;
 pub use selectable::*;
 pub use styled_ext::*;
 pub use styles::*;
+pub use toggleable::*;