agent_ui: Add some thread view design improvements (#52322)

Danilo Leal created 2 weeks ago

This PR adds some small design refinements to the thread view: making
keybinding size consistent throughout, improving the activity bar
shadow, fixing the keybinding to open a file from an edit tool diff, and
other smaller spacing/color tweaks.

Release Notes:

- N/A

Change summary

assets/keymaps/default-linux.json                    |  9 ++
assets/keymaps/default-macos.json                    |  9 ++
assets/keymaps/default-windows.json                  |  9 ++
crates/agent_ui/src/conversation_view/thread_view.rs | 48 ++++++-------
crates/sidebar/src/sidebar.rs                        | 12 ++
crates/ui/src/components/ai/thread_item.rs           |  2 
crates/ui/src/components/disclosure.rs               |  3 
7 files changed, 58 insertions(+), 34 deletions(-)

Detailed changes

assets/keymaps/default-linux.json

@@ -338,6 +338,13 @@
       "ctrl-alt-.": "agent::ToggleFastMode",
     },
   },
+  {
+    "context": "AcpThread > Editor && mode == full",
+    "use_key_equivalents": true,
+    "bindings": {
+      "alt-enter": "editor::OpenExcerpts",
+    },
+  },
   {
     "context": "AcpThread > Editor && !use_modifier_to_send",
     "use_key_equivalents": true,
@@ -1345,7 +1352,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "ctrl-,": "zed::OpenSettings",
-    }
+    },
   },
   {
     "context": "SettingsWindow",

assets/keymaps/default-macos.json

@@ -382,6 +382,13 @@
       "cmd-alt-.": "agent::ToggleFastMode",
     },
   },
+  {
+    "context": "AcpThread > Editor && mode == full",
+    "use_key_equivalents": true,
+    "bindings": {
+      "alt-enter": "editor::OpenExcerpts",
+    },
+  },
   {
     "context": "AcpThread > Editor && !use_modifier_to_send",
     "use_key_equivalents": true,
@@ -1447,7 +1454,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "cmd-,": "zed::OpenSettings",
-    }
+    },
   },
   {
     "context": "SettingsWindow",

assets/keymaps/default-windows.json

@@ -340,6 +340,13 @@
       "ctrl-alt-.": "agent::ToggleFastMode",
     },
   },
+  {
+    "context": "AcpThread > Editor && mode == full",
+    "use_key_equivalents": true,
+    "bindings": {
+      "alt-enter": "editor::OpenExcerpts",
+    },
+  },
   {
     "context": "AcpThread > Editor && !use_modifier_to_send",
     "use_key_equivalents": true,
@@ -1363,7 +1370,7 @@
     "use_key_equivalents": true,
     "bindings": {
       "ctrl-,": "zed::OpenSettings",
-    }
+    },
   },
   {
     "context": "SettingsWindow",

crates/agent_ui/src/conversation_view/thread_view.rs

@@ -2128,7 +2128,6 @@ impl ThreadView {
         let queue_expanded = self.queue_expanded;
 
         v_flex()
-            .mt_1()
             .mx_2()
             .bg(self.activity_bar_bg(cx))
             .border_1()
@@ -2136,9 +2135,9 @@ impl ThreadView {
             .border_color(cx.theme().colors().border)
             .rounded_t_md()
             .shadow(vec![gpui::BoxShadow {
-                color: gpui::black().opacity(0.15),
+                color: gpui::black().opacity(0.12),
                 offset: point(px(1.), px(-1.)),
-                blur_radius: px(3.),
+                blur_radius: px(2.),
                 spread_radius: px(0.),
             }])
             .when(!plan.is_empty(), |this| {
@@ -2434,7 +2433,10 @@ impl ThreadView {
             .child(
                 Button::new("clear_queue", "Clear All")
                     .label_size(LabelSize::Small)
-                    .key_binding(KeyBinding::for_action(&ClearMessageQueue, cx))
+                    .key_binding(
+                        KeyBinding::for_action(&ClearMessageQueue, cx)
+                            .map(|kb| kb.size(rems_from_px(12.))),
+                    )
                     .on_click(cx.listener(|this, _, _, cx| {
                         this.clear_queue(cx);
                         this.can_fast_track_queue = false;
@@ -2723,7 +2725,7 @@ impl ThreadView {
                             })
                             .key_binding(
                                 KeyBinding::for_action_in(&RejectAll, &focus_handle.clone(), cx)
-                                    .map(|kb| kb.size(rems_from_px(10.))),
+                                    .map(|kb| kb.size(rems_from_px(12.))),
                             )
                             .on_click(cx.listener(move |this, _, window, cx| {
                                 this.reject_all(&RejectAll, window, cx);
@@ -2738,7 +2740,7 @@ impl ThreadView {
                             })
                             .key_binding(
                                 KeyBinding::for_action_in(&KeepAll, &focus_handle, cx)
-                                    .map(|kb| kb.size(rems_from_px(10.))),
+                                    .map(|kb| kb.size(rems_from_px(12.))),
                             )
                             .on_click(cx.listener(move |this, _, window, cx| {
                                 this.keep_all(&KeepAll, window, cx);
@@ -5500,7 +5502,7 @@ impl ThreadView {
             matches!(tool_call.kind, acp::ToolKind::Edit) || tool_call.diffs().next().is_some();
 
         let is_cancelled_edit = is_edit && matches!(tool_call.status, ToolCallStatus::Canceled);
-        let (has_revealed_diff, tool_call_output_focus) = tool_call
+        let (has_revealed_diff, tool_call_output_focus, tool_call_output_focus_handle) = tool_call
             .diffs()
             .next()
             .and_then(|diff| {
@@ -5511,9 +5513,10 @@ impl ThreadView {
                     .and_then(|entry| entry.editor_for_diff(diff))?;
                 let has_revealed_diff = diff.read(cx).has_revealed_range(cx);
                 let has_focus = editor.read(cx).is_focused(window);
-                Some((has_revealed_diff, has_focus))
+                let focus_handle = editor.focus_handle(cx);
+                Some((has_revealed_diff, has_focus, focus_handle))
             })
-            .unwrap_or((false, false));
+            .unwrap_or_else(|| (false, false, focus_handle.clone()));
 
         let use_card_layout = needs_confirmation || is_edit || is_terminal_tool;
 
@@ -5738,7 +5741,6 @@ impl ThreadView {
                             .group(&card_header_id)
                             .relative()
                             .w_full()
-                            .gap_1()
                             .justify_between()
                             .when(use_card_layout, |this| {
                                 this.p_0p5()
@@ -5757,7 +5759,6 @@ impl ThreadView {
                             ))
                             .child(
                                 h_flex()
-                                    .gap_0p5()
                                     .when(is_collapsible || failed_or_canceled, |this| {
                                         let diff_for_discard = if has_revealed_diff
                                             && is_cancelled_edit
@@ -5770,10 +5771,7 @@ impl ThreadView {
 
                                         this.child(
                                             h_flex()
-                                                .px_1()
-                                                .when_some(diff_for_discard.clone(), |this, _| {
-                                                    this.pr_0p5()
-                                                })
+                                                .pr_0p5()
                                                 .gap_1()
                                                 .when(is_collapsible, |this| {
                                                     this.child(
@@ -5881,10 +5879,10 @@ impl ThreadView {
                                     .when(tool_call_output_focus, |this| {
                                         this.child(
                                             Button::new("open-file-button", "Open File")
+                                                .style(ButtonStyle::Outlined)
                                                 .label_size(LabelSize::Small)
-                                                .style(ButtonStyle::OutlinedGhost)
                                                 .key_binding(
-                                                    KeyBinding::for_action(&OpenExcerpts, cx)
+                                                    KeyBinding::for_action_in(&OpenExcerpts, &tool_call_output_focus_handle, cx)
                                                         .map(|s| s.size(rems_from_px(12.))),
                                                 )
                                                 .on_click(|_, window, cx| {
@@ -6023,7 +6021,7 @@ impl ThreadView {
                                         focus_handle,
                                         cx,
                                     )
-                                    .map(|kb| kb.size(rems_from_px(10.))),
+                                    .map(|kb| kb.size(rems_from_px(12.))),
                                 )
                             })
                             .on_click(cx.listener({
@@ -6047,7 +6045,7 @@ impl ThreadView {
                                         focus_handle,
                                         cx,
                                     )
-                                    .map(|kb| kb.size(rems_from_px(10.))),
+                                    .map(|kb| kb.size(rems_from_px(12.))),
                                 )
                             })
                             .on_click(cx.listener({
@@ -6095,7 +6093,7 @@ impl ThreadView {
                                 &self.focus_handle(cx),
                                 cx,
                             )
-                            .map(|kb| kb.size(rems_from_px(10.))),
+                            .map(|kb| kb.size(rems_from_px(12.))),
                         )
                     }),
             )
@@ -6185,7 +6183,7 @@ impl ThreadView {
                                 &self.focus_handle(cx),
                                 cx,
                             )
-                            .map(|kb| kb.size(rems_from_px(10.))),
+                            .map(|kb| kb.size(rems_from_px(12.))),
                         )
                     }),
             )
@@ -6384,7 +6382,7 @@ impl ThreadView {
 
                         this.key_binding(
                             KeyBinding::for_action_in(action, focus_handle, cx)
-                                .map(|kb| kb.size(rems_from_px(10.))),
+                                .map(|kb| kb.size(rems_from_px(12.))),
                         )
                     })
                     .label_size(LabelSize::Small)
@@ -6464,10 +6462,10 @@ impl ThreadView {
 
         let file_icon = if has_location {
             FileIcons::get_icon(&tool_call.locations[0].path, cx)
-                .map(Icon::from_path)
-                .unwrap_or(Icon::new(IconName::ToolPencil))
+                .map(|from_path| Icon::from_path(from_path).color(Color::Muted))
+                .unwrap_or(Icon::new(IconName::ToolPencil).color(Color::Muted))
         } else {
-            Icon::new(IconName::ToolPencil)
+            Icon::new(IconName::ToolPencil).color(Color::Muted)
         };
 
         let tool_icon = if is_file && has_failed && has_revealed_diff {

crates/sidebar/src/sidebar.rs

@@ -2892,15 +2892,21 @@ impl Sidebar {
             .h(header_height)
             .mt_px()
             .pb_px()
-            .when(traffic_lights, |this| {
-                this.pl(px(ui::utils::TRAFFIC_LIGHT_PADDING))
+            .map(|this| {
+                if traffic_lights {
+                    this.pl(px(ui::utils::TRAFFIC_LIGHT_PADDING))
+                } else {
+                    this.pl_1p5()
+                }
             })
             .pr_1p5()
             .gap_1()
             .when(!no_open_projects, |this| {
                 this.border_b_1()
                     .border_color(cx.theme().colors().border)
-                    .child(Divider::vertical().color(ui::DividerColor::Border))
+                    .when(traffic_lights, |this| {
+                        this.child(Divider::vertical().color(ui::DividerColor::Border))
+                    })
                     .child(
                         div().ml_1().child(
                             Icon::new(IconName::MagnifyingGlass)

crates/ui/src/components/ai/thread_item.rs

@@ -401,7 +401,7 @@ impl RenderOnce for ThreadItem {
                             this.child(
                                 h_flex()
                                     .id(format!("{}-worktree", self.id.clone()))
-                                    .gap_1()
+                                    .gap_0p5()
                                     .child(
                                         Icon::new(IconName::GitWorktree)
                                             .size(IconSize::XSmall)

crates/ui/src/components/disclosure.rs

@@ -2,7 +2,7 @@ use std::sync::Arc;
 
 use gpui::{ClickEvent, CursorStyle, SharedString};
 
-use crate::{Color, IconButton, IconButtonShape, IconName, IconSize, prelude::*};
+use crate::prelude::*;
 
 #[derive(IntoElement, RegisterComponent)]
 pub struct Disclosure {
@@ -91,7 +91,6 @@ impl RenderOnce for Disclosure {
                 false => self.closed_icon,
             },
         )
-        .shape(IconButtonShape::Square)
         .icon_color(Color::Muted)
         .icon_size(IconSize::Small)
         .disabled(self.disabled)