From 2e3baef2998a5bcb412853be010a1d10be679a3b Mon Sep 17 00:00:00 2001
From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
Date: Sun, 4 May 2025 07:53:21 -0300
Subject: [PATCH] agent: Polish single-file review toolbar controls (#29866)
---
assets/icons/list_collapse.svg | 1 +
crates/agent/src/agent_diff.rs | 93 ++++++++++++--------------
crates/git_ui/src/project_diff.rs | 3 +-
crates/icons/src/icons.rs | 1 +
crates/workspace/src/toolbar.rs | 8 ++-
crates/zed/src/zed/quick_action_bar.rs | 10 ++-
6 files changed, 56 insertions(+), 60 deletions(-)
create mode 100644 assets/icons/list_collapse.svg
diff --git a/assets/icons/list_collapse.svg b/assets/icons/list_collapse.svg
new file mode 100644
index 0000000000..a0e0ed604d
--- /dev/null
+++ b/assets/icons/list_collapse.svg
@@ -0,0 +1 @@
+
diff --git a/crates/agent/src/agent_diff.rs b/crates/agent/src/agent_diff.rs
index 79dc31232b..18d9635a41 100644
--- a/crates/agent/src/agent_diff.rs
+++ b/crates/agent/src/agent_diff.rs
@@ -26,7 +26,7 @@ use std::{
ops::Range,
sync::Arc,
};
-use ui::{IconButtonShape, KeyBinding, Tooltip, prelude::*, vertical_divider};
+use ui::{Divider, IconButtonShape, KeyBinding, Tooltip, prelude::*, vertical_divider};
use util::ResultExt;
use workspace::{
Item, ItemHandle, ItemNavHistory, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView,
@@ -937,7 +937,7 @@ impl AgentDiffToolbar {
Some(AgentDiffToolbarItem::Pane(_)) => ToolbarItemLocation::PrimaryRight,
Some(AgentDiffToolbarItem::Editor { state, .. }) => match state {
EditorState::Generating | EditorState::Reviewing => {
- ToolbarItemLocation::PrimaryLeft
+ ToolbarItemLocation::PrimaryRight
}
EditorState::Idle => ToolbarItemLocation::Hidden,
},
@@ -990,6 +990,11 @@ impl ToolbarItemView for AgentDiffToolbar {
impl Render for AgentDiffToolbar {
fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement {
+ let generating_label = div()
+ .w(rems_from_px(110.)) // Arbitrary size so the label doesn't dance around
+ .child(AnimatedLabel::new("Generating"))
+ .into_any();
+
let Some(active_item) = self.active_item.as_ref() else {
return Empty.into_any();
};
@@ -1004,24 +1009,13 @@ impl Render for AgentDiffToolbar {
let content = match state {
EditorState::Idle => return Empty.into_any(),
- EditorState::Generating => vec![
- h_flex()
- .ml_1()
- .gap_1p5()
- .w_32()
- .child(Icon::new(IconName::ZedAssistant))
- .child(
- div()
- .w(rems(6.5625))
- .child(AnimatedLabel::new("Generating")),
- )
- .into_any(),
- ],
+ EditorState::Generating => vec![generating_label],
EditorState::Reviewing => vec![
h_flex()
.child(
IconButton::new("hunk-up", IconName::ArrowUp)
- .tooltip(ui::Tooltip::for_action_title_in(
+ .icon_size(IconSize::Small)
+ .tooltip(Tooltip::for_action_title_in(
"Previous Hunk",
&GoToPreviousHunk,
&editor_focus_handle,
@@ -1039,7 +1033,8 @@ impl Render for AgentDiffToolbar {
)
.child(
IconButton::new("hunk-down", IconName::ArrowDown)
- .tooltip(ui::Tooltip::for_action_title_in(
+ .icon_size(IconSize::Small)
+ .tooltip(Tooltip::for_action_title_in(
"Next Hunk",
&GoToHunk,
&editor_focus_handle,
@@ -1053,8 +1048,9 @@ impl Render for AgentDiffToolbar {
}),
)
.into_any(),
- vertical_divider().into_any_element(),
+ Divider::vertical().into_any_element(),
h_flex()
+ .gap_0p5()
.child(
Button::new("reject-all", "Reject All")
.key_binding({
@@ -1086,18 +1082,37 @@ impl Render for AgentDiffToolbar {
})),
)
.into_any(),
+ Divider::vertical().into_any_element(),
],
};
h_flex()
- .bg(cx.theme().colors().surface_background)
- .rounded_md()
- .p_1()
- .mx_2()
- .gap_1()
- .children(content)
- .child(vertical_divider())
.track_focus(&editor_focus_handle)
+ .size_full()
+ .child(
+ h_flex()
+ .py(DynamicSpacing::Base08.rems(cx))
+ .px_2()
+ .gap_1()
+ .children(content)
+ .when_some(editor.read(cx).workspace(), |this, _workspace| {
+ this.child(
+ IconButton::new("review", IconName::ListCollapse)
+ .icon_size(IconSize::Small)
+ .tooltip(Tooltip::for_action_title_in(
+ "Review All Files",
+ &OpenAgentDiff,
+ &editor_focus_handle,
+ ))
+ .on_click({
+ cx.listener(move |this, _, window, cx| {
+ this.dispatch_action(&OpenAgentDiff, window, cx);
+ })
+ }),
+ )
+ }),
+ )
+ .child(vertical_divider())
.on_action({
let editor = editor.clone();
move |_action: &OpenAgentDiff, window, cx| {
@@ -1106,21 +1121,6 @@ impl Render for AgentDiffToolbar {
});
}
})
- .when_some(editor.read(cx).workspace(), |this, _workspace| {
- this.child(
- IconButton::new("review", IconName::ListTree)
- .tooltip(ui::Tooltip::for_action_title_in(
- "Review All Files",
- &OpenAgentDiff,
- &editor_focus_handle,
- ))
- .on_click({
- cx.listener(move |this, _, window, cx| {
- this.dispatch_action(&OpenAgentDiff, window, cx);
- })
- }),
- )
- })
.into_any()
}
AgentDiffToolbarItem::Pane(agent_diff) => {
@@ -1130,10 +1130,7 @@ impl Render for AgentDiffToolbar {
let is_generating = agent_diff.read(cx).thread.read(cx).is_generating();
if is_generating {
- return div()
- .w(rems(6.5625)) // Arbitrary 105px size—so the label doesn't dance around
- .child(AnimatedLabel::new("Generating"))
- .into_any();
+ return div().px_2().child(generating_label).into_any();
}
let is_empty = agent_diff.read(cx).multibuffer.read(cx).is_empty();
@@ -1144,11 +1141,9 @@ impl Render for AgentDiffToolbar {
let focus_handle = agent_diff.focus_handle(cx);
h_group_xl()
- .my_neg_1()
+ .px_2()
.items_center()
- .p_1()
.flex_wrap()
- .justify_between()
.child(
h_group_sm()
.child(
@@ -2086,7 +2081,7 @@ mod tests {
// The toolbar is displayed in the right state
assert_eq!(
diff_toolbar.read_with(cx, |toolbar, cx| toolbar.location(cx)),
- ToolbarItemLocation::PrimaryLeft
+ ToolbarItemLocation::PrimaryRight
);
assert!(diff_toolbar.read_with(cx, |toolbar, _cx| matches!(
toolbar.active_item,
@@ -2105,7 +2100,7 @@ mod tests {
override_toolbar_agent_review_setting(true, cx);
assert_eq!(
diff_toolbar.read_with(cx, |toolbar, cx| toolbar.location(cx)),
- ToolbarItemLocation::PrimaryLeft
+ ToolbarItemLocation::PrimaryRight
);
// After keeping a hunk, the cursor should be positioned on the second hunk.
diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs
index 61f7097409..6e7fa3c816 100644
--- a/crates/git_ui/src/project_diff.rs
+++ b/crates/git_ui/src/project_diff.rs
@@ -895,8 +895,7 @@ impl Render for ProjectDiffToolbar {
.my_neg_1()
.items_center()
.py_1()
- .pl_2()
- .pr_1()
+ .px_2()
.flex_wrap()
.justify_between()
.child(
diff --git a/crates/icons/src/icons.rs b/crates/icons/src/icons.rs
index 8d9d1ea8f0..79d5637ea2 100644
--- a/crates/icons/src/icons.rs
+++ b/crates/icons/src/icons.rs
@@ -151,6 +151,7 @@ pub enum IconName {
LightBulb,
LineHeight,
Link,
+ ListCollapse,
ListTree,
ListX,
LockOutlined,
diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs
index 6eccf68693..a8f831f3f0 100644
--- a/crates/workspace/src/toolbar.rs
+++ b/crates/workspace/src/toolbar.rs
@@ -105,23 +105,23 @@ impl Render for Toolbar {
v_flex()
.group("toolbar")
- .p(DynamicSpacing::Base08.rems(cx))
+ .relative()
.when(has_left_items || has_right_items, |this| {
this.gap(DynamicSpacing::Base08.rems(cx))
})
.border_b_1()
.border_color(cx.theme().colors().border_variant)
- .relative()
.bg(cx.theme().colors().toolbar_background)
.when(has_left_items || has_right_items, |this| {
this.child(
h_flex()
- .min_h(rems_from_px(24.))
+ .min_h_6()
.justify_between()
.gap(DynamicSpacing::Base08.rems(cx))
.when(has_left_items, |this| {
this.child(
h_flex()
+ .p(DynamicSpacing::Base08.rems(cx))
.flex_auto()
.justify_start()
.overflow_x_hidden()
@@ -131,6 +131,8 @@ impl Render for Toolbar {
.when(has_right_items, |this| {
this.child(
h_flex()
+ .h_full()
+ .flex_row_reverse()
.map(|el| {
if has_left_items {
// We're using `flex_none` here to prevent some flickering that can occur when the
diff --git a/crates/zed/src/zed/quick_action_bar.rs b/crates/zed/src/zed/quick_action_bar.rs
index 8a89b1ecc7..9ceceabce9 100644
--- a/crates/zed/src/zed/quick_action_bar.rs
+++ b/crates/zed/src/zed/quick_action_bar.rs
@@ -15,8 +15,8 @@ use gpui::{
use search::{BufferSearchBar, buffer_search};
use settings::{Settings, SettingsStore};
use ui::{
- ButtonStyle, ContextMenu, ContextMenuEntry, IconButton, IconButtonShape, IconName, IconSize,
- PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*,
+ ButtonStyle, ContextMenu, ContextMenuEntry, IconButton, IconName, IconSize, PopoverMenu,
+ PopoverMenuHandle, Tooltip, prelude::*,
};
use vim_mode_setting::VimModeSetting;
use workspace::{
@@ -141,7 +141,6 @@ impl Render for QuickActionBar {
PopoverMenu::new("editor-selections-dropdown")
.trigger_with_tooltip(
IconButton::new("toggle_editor_selections_icon", IconName::CursorIBeam)
- .shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.style(ButtonStyle::Subtle)
.toggle_state(self.toggle_selections_handle.is_deployed()),
@@ -190,7 +189,6 @@ impl Render for QuickActionBar {
PopoverMenu::new("editor-settings")
.trigger_with_tooltip(
IconButton::new("toggle_editor_settings_icon", IconName::Sliders)
- .shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.style(ButtonStyle::Subtle)
.toggle_state(self.toggle_settings_handle.is_deployed()),
@@ -435,7 +433,8 @@ impl Render for QuickActionBar {
h_flex()
.id("quick action bar")
- .gap(DynamicSpacing::Base04.rems(cx))
+ .p(DynamicSpacing::Base08.rems(cx))
+ .gap(DynamicSpacing::Base01.rems(cx))
.children(self.render_repl_menu(cx))
.children(self.render_toggle_markdown_preview(self.workspace.clone(), cx))
.children(search_button)
@@ -490,7 +489,6 @@ impl RenderOnce for QuickActionBarButton {
let action = self.action.boxed_clone();
IconButton::new(self.id.clone(), self.icon)
- .shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.style(ButtonStyle::Subtle)
.toggle_state(self.toggled)