Iterate on design of channel management modal (#3923)

Marshall Bowers created

This PR refines the design of the channel management modal:

#### Before

<img width="585" alt="Screenshot 2024-01-05 at 4 17 32 PM"
src="https://github.com/zed-industries/zed/assets/1486634/a6d96674-b688-4549-8fb8-0a7b2c7c88be">

#### After

<img width="589" alt="Screenshot 2024-01-05 at 4 15 20 PM"
src="https://github.com/zed-industries/zed/assets/1486634/31ae8dc1-a129-4a15-963e-9779b9b79bd9">

Release Notes:

- Improved the look of the channel management modal.

Change summary

crates/collab_ui/src/collab_panel/channel_modal.rs |  67 +++--
crates/ui/src/components/checkbox.rs               | 179 ++-------------
2 files changed, 65 insertions(+), 181 deletions(-)

Detailed changes

crates/collab_ui/src/collab_panel/channel_modal.rs 🔗

@@ -162,8 +162,7 @@ impl Render for ChannelModal {
                 v_stack()
                     .px_2()
                     .py_1()
-                    .rounded_t(px(8.))
-                    .bg(cx.theme().colors().element_background)
+                    .gap_2()
                     .child(
                         h_stack()
                             .w_px()
@@ -175,7 +174,9 @@ impl Render for ChannelModal {
                     .child(
                         h_stack()
                             .w_full()
+                            .h(rems(22. / 16.))
                             .justify_between()
+                            .line_height(rems(1.25))
                             .child(
                                 h_stack()
                                     .gap_2()
@@ -190,38 +191,54 @@ impl Render for ChannelModal {
                                         )
                                         .on_click(cx.listener(Self::set_channel_visiblity)),
                                     )
-                                    .child(Label::new("Public")),
+                                    .child(Label::new("Public").size(LabelSize::Small)),
                             )
-                            .children(if visibility == ChannelVisibility::Public {
-                                Some(Button::new("copy-link", "Copy Link").on_click(cx.listener(
-                                    move |this, _, cx| {
-                                        if let Some(channel) =
-                                            this.channel_store.read(cx).channel_for_id(channel_id)
-                                        {
-                                            let item = ClipboardItem::new(channel.link());
-                                            cx.write_to_clipboard(item);
-                                        }
-                                    },
-                                )))
-                            } else {
-                                None
-                            }),
+                            .children(
+                                Some(
+                                    Button::new("copy-link", "Copy Link")
+                                        .label_size(LabelSize::Small)
+                                        .on_click(cx.listener(move |this, _, cx| {
+                                            if let Some(channel) = this
+                                                .channel_store
+                                                .read(cx)
+                                                .channel_for_id(channel_id)
+                                            {
+                                                let item = ClipboardItem::new(channel.link());
+                                                cx.write_to_clipboard(item);
+                                            }
+                                        })),
+                                )
+                                .filter(|_| visibility == ChannelVisibility::Public),
+                            ),
                     )
                     .child(
-                        div()
-                            .w_full()
-                            .flex()
-                            .flex_row()
+                        h_stack()
                             .child(
-                                Button::new("manage-members", "Manage Members")
-                                    .selected(mode == Mode::ManageMembers)
+                                div()
+                                    .id("manage-members")
+                                    .px_2()
+                                    .py_1()
+                                    .cursor_pointer()
+                                    .border_b_2()
+                                    .when(mode == Mode::ManageMembers, |this| {
+                                        this.border_color(cx.theme().colors().border)
+                                    })
+                                    .child(Label::new("Manage Members"))
                                     .on_click(cx.listener(|this, _, cx| {
                                         this.set_mode(Mode::ManageMembers, cx);
                                     })),
                             )
                             .child(
-                                Button::new("invite-members", "Invite Members")
-                                    .selected(mode == Mode::InviteMembers)
+                                div()
+                                    .id("invite-members")
+                                    .px_2()
+                                    .py_1()
+                                    .cursor_pointer()
+                                    .border_b_2()
+                                    .when(mode == Mode::InviteMembers, |this| {
+                                        this.border_color(cx.theme().colors().border)
+                                    })
+                                    .child(Label::new("Invite Members"))
                                     .on_click(cx.listener(|this, _, cx| {
                                         this.set_mode(Mode::InviteMembers, cx);
                                     })),

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

@@ -1,4 +1,4 @@
-use gpui::{div, prelude::*, Element, ElementId, IntoElement, Styled, WindowContext};
+use gpui::{div, prelude::*, ElementId, IntoElement, Styled, WindowContext};
 
 use crate::prelude::*;
 use crate::{Color, Icon, IconElement, Selection};
@@ -18,126 +18,6 @@ pub struct Checkbox {
     on_click: Option<CheckHandler>,
 }
 
-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 {
-            // When selected, we show a checkmark.
-            Selection::Selected => {
-                Some(
-                    IconElement::new(Icon::Check)
-                        .size(crate::IconSize::Small)
-                        .color(
-                            // If the checkbox is disabled we change the color of the icon.
-                            if self.disabled {
-                                Color::Disabled
-                            } else {
-                                Color::Selected
-                            },
-                        ),
-                )
-            }
-            // In an indeterminate state, we show a dash.
-            Selection::Indeterminate => {
-                Some(
-                    IconElement::new(Icon::Dash)
-                        .size(crate::IconSize::Small)
-                        .color(
-                            // If the checkbox is disabled we change the color of the icon.
-                            if self.disabled {
-                                Color::Disabled
-                            } else {
-                                Color::Selected
-                            },
-                        ),
-                )
-            }
-            // When unselected, we show nothing.
-            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 it's 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:
-        //
-        // ~~~rust
-        // ...
-        // .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,
-            ),
-        };
-
-        div()
-            .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)),
-            )
-    }
-}
 impl Checkbox {
     pub fn new(id: impl Into<ElementId>, checked: Selection) -> Self {
         Self {
@@ -160,42 +40,29 @@ impl Checkbox {
         self.on_click = Some(Box::new(handler));
         self
     }
+}
 
-    pub fn render(self, cx: &mut WindowContext) -> impl Element {
+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 {
-            // When selected, we show a checkmark.
-            Selection::Selected => {
-                Some(
-                    IconElement::new(Icon::Check)
-                        .size(crate::IconSize::Small)
-                        .color(
-                            // If the checkbox is disabled we change the color of the icon.
-                            if self.disabled {
-                                Color::Disabled
-                            } else {
-                                Color::Selected
-                            },
-                        ),
-                )
-            }
-            // In an indeterminate state, we show a dash.
-            Selection::Indeterminate => {
-                Some(
-                    IconElement::new(Icon::Dash)
-                        .size(crate::IconSize::Small)
-                        .color(
-                            // If the checkbox is disabled we change the color of the icon.
-                            if self.disabled {
-                                Color::Disabled
-                            } else {
-                                Color::Selected
-                            },
-                        ),
-                )
-            }
-            // When unselected, we show nothing.
+            Selection::Selected => Some(IconElement::new(Icon::Check).size(IconSize::Small).color(
+                if self.disabled {
+                    Color::Disabled
+                } else {
+                    Color::Selected
+                },
+            )),
+            Selection::Indeterminate => Some(
+                IconElement::new(Icon::Dash)
+                    .size(IconSize::Small)
+                    .color(if self.disabled {
+                        Color::Disabled
+                    } else {
+                        Color::Selected
+                    }),
+            ),
             Selection::Unselected => None,
         };
 
@@ -212,12 +79,12 @@ impl Checkbox {
 
         // We could use something like this to make the checkbox background when selected:
         //
-        // ~~~rust
+        // ```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.
@@ -236,7 +103,7 @@ impl Checkbox {
             ),
         };
 
-        div()
+        h_stack()
             .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