agent: Polish single-file review toolbar controls (#29866)

assets/icons/list_collapse.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-list-collapse-icon lucide-list-collapse"><path d="m3 10 2.5-2.5L3 5"/><path d="m3 19 2.5-2.5L3 14"/><path d="M10 6h11"/><path d="M10 12h11"/><path d="M10 18h11"/></svg>

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<Self>) -> 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![
+                    EditorState::Generating => vec![generating_label],
-                        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::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()
+                    return div().px_2().child(generating_label).into_any();
-                        .w(rems(6.5625)) // Arbitrary 105px size—so the label doesn't dance around
-                        .child(AnimatedLabel::new("Generating"))
-                        .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.

crates/git_ui/src/project_diff.rs

@@ -895,8 +895,7 @@ impl Render for ProjectDiffToolbar {
             .my_neg_1()
             .items_center()
             .py_1()
-            .pl_2()
+            .px_2()
-            .pr_1()
             .flex_wrap()
             .justify_between()
             .child(

crates/icons/src/icons.rs

@@ -151,6 +151,7 @@ pub enum IconName {
     LightBulb,
     LineHeight,
     Link,
+    ListCollapse,
     ListTree,
     ListX,
     LockOutlined,

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

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,
+    ButtonStyle, ContextMenu, ContextMenuEntry, IconButton, IconName, IconSize, PopoverMenu,
-    PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*,
+    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)