mirrors/zed
1
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2025-01-27 04:44:30 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Take a different approach to rendering channel buttons (#3991)

Browse source 
This PR changes the approach we're using to render the channel buttons
to use a more straightforward (and less hacky) approach.

### Motivation

Even with the variety of hacks that were employed to make the current
approach work, there are still a number of issues with the current
solution:

- Hovering in the empty space to the left of a channel doesn't correctly
apply the hover background to the container for the channel buttons
- Hovering to the very right of the collab panel (right on top of the
drag handle) causes the channel button container to apply its hover
background without applying it to the rest of the row
- The buttons would still get pushed off to the right by the indent
space in the channel tree, resulting in jagged indicators at small sizes

Additionally, the rectangular background placed behind the channel
buttons still didn't look great when it overlapped with the channel
names.

### Explanation

For these reasons, I decided to explore a simpler approach that
addresses these issues, albeit with some tradeoffs that I think are
acceptable.

We now render the absolutely-positioned button container as a sibling
element of the `ListItem`. This is to avoid issues with the container
getting pushed around based on the contents of the `ListItem` rather
than staying absolutely positioned at the end of the row.

We also have gotten rid of the background for the button container, and
now rely on the background of the individual `IconButton`s to occlude
the channel name behind them when the two are overlapping.

Here are some examples of the new UI in various configurations:

#### When the channel entry is hovered

<img width="270" alt="Screenshot 2024-01-09 at 6 15 24 PM"
src="https://github.com/zed-industries/zed/assets/1486634/0207a129-30eb-4067-8490-3b6c9cbf75ea">

#### Overlapping with the channel name

<img width="229" alt="Screenshot 2024-01-09 at 6 15 43 PM"
src="https://github.com/zed-industries/zed/assets/1486634/0a67507e-45c8-4325-a71e-d416edc3a115">

#### Narrow collab panel

<img width="116" alt="Screenshot 2024-01-09 at 6 16 07 PM"
src="https://github.com/zed-industries/zed/assets/1486634/887ebaa3-e4d6-4497-9141-afcc8b7cd356">

### Tradeoffs

The new approach comes with the following tradeoffs that I am currently
aware of:

The occlusion can look a little weird when the icons are in the middle
of a channel name (as opposed to fully occluding the end of the channel
name):
<img width="190" alt="Screenshot 2024-01-09 at 6 24 32 PM"
src="https://github.com/zed-industries/zed/assets/1486634/0062b806-1b8f-47eb-af8d-f061d9829366">

Hovering one of the icons causes the icon to be hovered instead of the
row:

<img width="232" alt="Screenshot 2024-01-09 at 6 31 38 PM"
src="https://github.com/zed-industries/zed/assets/1486634/cbcc1a45-64d2-4890-8ad6-c5a5ee961b86">

Release Notes:

- Improved the way channel buttons are displayed.
This commit is contained in:
Marshall Bowers 2024-01-09 18:39:29 -05:00 committed by GitHub
parent 684bd530f0
commit 8cb291baa4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 49 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

139
crates/collab_ui/src/collab_panel.rs
View file

@ -2228,47 +2228,6 @@ impl CollabPanel {
None
};
let button_container = |cx: &mut ViewContext<Self>| {
h_stack()
.absolute()
// We're using a negative coordinate for the right anchor to
// counteract the padding of the `ListItem`.
//
// This prevents a gap from showing up between the background
// of this element and the edge of the collab panel.
.right(rems(-0.5))
// HACK: Without this the channel name clips on top of the icons, but I'm not sure why.
.z_index(10)
.bg(cx.theme().colors().panel_background)
.when(is_selected || is_active, |this| {
this.bg(cx.theme().colors().ghost_element_selected)
})
};
let messages_button = |cx: &mut ViewContext<Self>| {
IconButton::new("channel_chat", IconName::MessageBubbles)
.icon_size(IconSize::Small)
.icon_color(if has_messages_notification {
Color::Default
} else {
Color::Muted
})
.on_click(cx.listener(move |this, _, cx| this.join_channel_chat(channel_id, cx)))
.tooltip(|cx| Tooltip::text("Open channel chat", cx))
};
let notes_button = |cx: &mut ViewContext<Self>| {
IconButton::new("channel_notes", IconName::File)
.icon_size(IconSize::Small)
.icon_color(if has_notes_notification {
Color::Default
} else {
Color::Muted
})
.on_click(cx.listener(move |this, _, cx| this.open_channel_notes(channel_id, cx)))
.tooltip(|cx| Tooltip::text("Open channel notes", cx))
};
let width = self.width.unwrap_or(px(240.));
div()
@ -2326,58 +2285,58 @@ impl CollabPanel {
.child(
h_stack()
.id(channel_id as usize)
// HACK: This is a dirty hack to help with the positioning of the button container.
//
// We're using a pixel width for the elements but then allowing the contents to
// overflow. This means that the label and facepile will be shown, but will not
// push the button container off the edge of the panel.
.w_px()
.child(Label::new(channel.name.clone()))
.children(face_pile.map(|face_pile| face_pile.render(cx))),
)
.end_slot::<Div>(
// If we have a notification for either button, we want to show the corresponding
// button(s) as indicators.
if has_messages_notification || has_notes_notification {
Some(
button_container(cx).child(
h_stack()
.px_1()
.children(
// We only want to render the messages button if there are unseen messages.
// This way we don't take up any space that might overlap the channel name
// when there are no notifications.
has_messages_notification.then(|| messages_button(cx)),
)
.child(
// We always want the notes button to take up space to prevent layout
// shift when hovering over the channel.
// However, if there are is no notes notification we just show an empty slot.
notes_button(cx)
.when(!has_notes_notification, |this| {
this.visible_on_hover("")
}),
),
),
),
)
.child(
h_stack()
.absolute()
.right(rems(0.))
.h_full()
// HACK: Without this the channel name clips on top of the icons, but I'm not sure why.
.z_index(10)
.child(
h_stack()
.h_full()
.gap_1()
.px_1()
.child(
IconButton::new("channel_chat", IconName::MessageBubbles)
.style(ButtonStyle::Filled)
.size(ButtonSize::Compact)
.icon_size(IconSize::Small)
.icon_color(if has_messages_notification {
Color::Default
} else {
Color::Muted
})
.on_click(cx.listener(move |this, _, cx| {
this.join_channel_chat(channel_id, cx)
}))
.tooltip(|cx| Tooltip::text("Open channel chat", cx))
.when(!has_messages_notification, |this| {
this.visible_on_hover("")
}),
)
} else {
None
},
)
.end_hover_slot(
// When we hover the channel entry we want to always show both buttons.
button_container(cx).child(
h_stack()
.px_1()
// The element hover background has a slight transparency to it, so we
// need to apply it to the inner element so that it blends with the solid
// background color of the absolutely-positioned element.
.group_hover("", |style| {
style.bg(cx.theme().colors().ghost_element_hover)
})
.child(messages_button(cx))
.child(notes_button(cx)),
),
.child(
IconButton::new("channel_notes", IconName::File)
.style(ButtonStyle::Filled)
.size(ButtonSize::Compact)
.icon_size(IconSize::Small)
.icon_color(if has_notes_notification {
Color::Default
} else {
Color::Muted
})
.on_click(cx.listener(move |this, _, cx| {
this.open_channel_notes(channel_id, cx)
}))
.tooltip(|cx| Tooltip::text("Open channel notes", cx))
.when(!has_notes_notification, |this| {
this.visible_on_hover("")
}),
),
),
)
.tooltip(|cx| Tooltip::text("Join channel", cx))