From 8e290b446e025cea2c4f9d4f3175dd97ee39e1bf Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Wed, 6 Aug 2025 20:31:11 -0300 Subject: [PATCH] thread view: Add UI refinements (#35754) More notably around how we render tool calls. Nothing too drastic, though. Release Notes: - N/A --- .../icons/{tool_bulb.svg => tool_think.svg} | 0 assets/keymaps/default-linux.json | 4 +- assets/keymaps/default-macos.json | 4 +- crates/agent_ui/src/acp/thread_view.rs | 275 ++++++++++++------ crates/agent_ui/src/active_thread.rs | 2 +- crates/assistant_tools/src/thinking_tool.rs | 2 +- crates/icons/src/icons.rs | 2 +- crates/ui/src/components/disclosure.rs | 2 +- 8 files changed, 188 insertions(+), 103 deletions(-) rename assets/icons/{tool_bulb.svg => tool_think.svg} (100%) diff --git a/assets/icons/tool_bulb.svg b/assets/icons/tool_think.svg similarity index 100% rename from assets/icons/tool_bulb.svg rename to assets/icons/tool_think.svg diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 81f5c695a2..2a4c095124 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -332,7 +332,9 @@ "enter": "agent::Chat", "up": "agent::PreviousHistoryMessage", "down": "agent::NextHistoryMessage", - "shift-ctrl-r": "agent::OpenAgentDiff" + "shift-ctrl-r": "agent::OpenAgentDiff", + "ctrl-shift-y": "agent::KeepAll", + "ctrl-shift-n": "agent::RejectAll" } }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 69958fd1f8..1a6cda4b64 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -384,7 +384,9 @@ "enter": "agent::Chat", "up": "agent::PreviousHistoryMessage", "down": "agent::NextHistoryMessage", - "shift-ctrl-r": "agent::OpenAgentDiff" + "shift-ctrl-r": "agent::OpenAgentDiff", + "cmd-shift-y": "agent::KeepAll", + "cmd-shift-n": "agent::RejectAll" } }, { diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 6475b7eeee..a9b39e6cea 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -858,6 +858,7 @@ impl AcpThreadView { .into_any() } AgentThreadEntry::ToolCall(tool_call) => div() + .w_full() .py_1p5() .px_5() .child(self.render_tool_call(index, tool_call, window, cx)) @@ -903,6 +904,7 @@ impl AcpThreadView { cx: &Context, ) -> AnyElement { let header_id = SharedString::from(format!("thinking-block-header-{}", entry_ix)); + let card_header_id = SharedString::from("inner-card-header"); let key = (entry_ix, chunk_ix); let is_open = self.expanded_thinking_blocks.contains(&key); @@ -910,41 +912,53 @@ impl AcpThreadView { .child( h_flex() .id(header_id) - .group("disclosure-header") + .group(&card_header_id) + .relative() .w_full() - .justify_between() + .gap_1p5() .opacity(0.8) .hover(|style| style.opacity(1.)) .child( h_flex() - .gap_1p5() - .child( - Icon::new(IconName::ToolBulb) - .size(IconSize::Small) - .color(Color::Muted), - ) + .size_4() + .justify_center() .child( div() - .text_size(self.tool_name_font_size()) - .child("Thinking"), + .group_hover(&card_header_id, |s| s.invisible().w_0()) + .child( + Icon::new(IconName::ToolThink) + .size(IconSize::Small) + .color(Color::Muted), + ), + ) + .child( + h_flex() + .absolute() + .inset_0() + .invisible() + .justify_center() + .group_hover(&card_header_id, |s| s.visible()) + .child( + Disclosure::new(("expand", entry_ix), is_open) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronRight) + .on_click(cx.listener({ + move |this, _event, _window, cx| { + if is_open { + this.expanded_thinking_blocks.remove(&key); + } else { + this.expanded_thinking_blocks.insert(key); + } + cx.notify(); + } + })), + ), ), ) .child( - div().visible_on_hover("disclosure-header").child( - Disclosure::new("thinking-disclosure", is_open) - .opened_icon(IconName::ChevronUp) - .closed_icon(IconName::ChevronDown) - .on_click(cx.listener({ - move |this, _event, _window, cx| { - if is_open { - this.expanded_thinking_blocks.remove(&key); - } else { - this.expanded_thinking_blocks.insert(key); - } - cx.notify(); - } - })), - ), + div() + .text_size(self.tool_name_font_size()) + .child("Thinking"), ) .on_click(cx.listener({ move |this, _event, _window, cx| { @@ -975,6 +989,67 @@ impl AcpThreadView { .into_any_element() } + fn render_tool_call_icon( + &self, + group_name: SharedString, + entry_ix: usize, + is_collapsible: bool, + is_open: bool, + tool_call: &ToolCall, + cx: &Context, + ) -> Div { + let tool_icon = Icon::new(match tool_call.kind { + acp::ToolKind::Read => IconName::ToolRead, + acp::ToolKind::Edit => IconName::ToolPencil, + acp::ToolKind::Delete => IconName::ToolDeleteFile, + acp::ToolKind::Move => IconName::ArrowRightLeft, + acp::ToolKind::Search => IconName::ToolSearch, + acp::ToolKind::Execute => IconName::ToolTerminal, + acp::ToolKind::Think => IconName::ToolThink, + acp::ToolKind::Fetch => IconName::ToolWeb, + acp::ToolKind::Other => IconName::ToolHammer, + }) + .size(IconSize::Small) + .color(Color::Muted); + + if is_collapsible { + h_flex() + .size_4() + .justify_center() + .child( + div() + .group_hover(&group_name, |s| s.invisible().w_0()) + .child(tool_icon), + ) + .child( + h_flex() + .absolute() + .inset_0() + .invisible() + .justify_center() + .group_hover(&group_name, |s| s.visible()) + .child( + Disclosure::new(("expand", entry_ix), is_open) + .opened_icon(IconName::ChevronUp) + .closed_icon(IconName::ChevronRight) + .on_click(cx.listener({ + let id = tool_call.id.clone(); + move |this: &mut Self, _, _, cx: &mut Context| { + if is_open { + this.expanded_tool_calls.remove(&id); + } else { + this.expanded_tool_calls.insert(id.clone()); + } + cx.notify(); + } + })), + ), + ) + } else { + div().child(tool_icon) + } + } + fn render_tool_call( &self, entry_ix: usize, @@ -982,7 +1057,8 @@ impl AcpThreadView { window: &Window, cx: &Context, ) -> Div { - let header_id = SharedString::from(format!("tool-call-header-{}", entry_ix)); + let header_id = SharedString::from(format!("outer-tool-call-header-{}", entry_ix)); + let card_header_id = SharedString::from("inner-tool-call-header"); let status_icon = match &tool_call.status { ToolCallStatus::Allowed { @@ -1031,6 +1107,21 @@ impl AcpThreadView { let is_collapsible = !tool_call.content.is_empty() && !needs_confirmation; let is_open = !is_collapsible || self.expanded_tool_calls.contains(&tool_call.id); + let gradient_color = cx.theme().colors().panel_background; + let gradient_overlay = { + div() + .absolute() + .top_0() + .right_0() + .w_12() + .h_full() + .bg(linear_gradient( + 90., + linear_color_stop(gradient_color, 1.), + linear_color_stop(gradient_color.opacity(0.2), 0.), + )) + }; + v_flex() .when(needs_confirmation, |this| { this.rounded_lg() @@ -1047,43 +1138,38 @@ impl AcpThreadView { .justify_between() .map(|this| { if needs_confirmation { - this.px_2() + this.pl_2() + .pr_1() .py_1() .rounded_t_md() - .bg(self.tool_card_header_bg(cx)) .border_b_1() .border_color(self.tool_card_border_color(cx)) + .bg(self.tool_card_header_bg(cx)) } else { this.opacity(0.8).hover(|style| style.opacity(1.)) } }) .child( h_flex() - .id("tool-call-header") - .overflow_x_scroll() + .group(&card_header_id) + .relative() + .w_full() .map(|this| { - if needs_confirmation { - this.text_xs() + if tool_call.locations.len() == 1 { + this.gap_0() } else { - this.text_size(self.tool_name_font_size()) + this.gap_1p5() } }) - .gap_1p5() - .child( - Icon::new(match tool_call.kind { - acp::ToolKind::Read => IconName::ToolRead, - acp::ToolKind::Edit => IconName::ToolPencil, - acp::ToolKind::Delete => IconName::ToolDeleteFile, - acp::ToolKind::Move => IconName::ArrowRightLeft, - acp::ToolKind::Search => IconName::ToolSearch, - acp::ToolKind::Execute => IconName::ToolTerminal, - acp::ToolKind::Think => IconName::ToolBulb, - acp::ToolKind::Fetch => IconName::ToolWeb, - acp::ToolKind::Other => IconName::ToolHammer, - }) - .size(IconSize::Small) - .color(Color::Muted), - ) + .text_size(self.tool_name_font_size()) + .child(self.render_tool_call_icon( + card_header_id, + entry_ix, + is_collapsible, + is_open, + tool_call, + cx, + )) .child(if tool_call.locations.len() == 1 { let name = tool_call.locations[0] .path @@ -1094,13 +1180,11 @@ impl AcpThreadView { h_flex() .id(("open-tool-call-location", entry_ix)) - .child(name) .w_full() .max_w_full() - .pr_1() - .gap_0p5() - .cursor_pointer() + .px_1p5() .rounded_sm() + .overflow_x_scroll() .opacity(0.8) .hover(|label| { label.opacity(1.).bg(cx @@ -1109,53 +1193,49 @@ impl AcpThreadView { .element_hover .opacity(0.5)) }) + .child(name) .tooltip(Tooltip::text("Jump to File")) .on_click(cx.listener(move |this, _, window, cx| { this.open_tool_call_location(entry_ix, 0, window, cx); })) .into_any_element() } else { - self.render_markdown( - tool_call.label.clone(), - default_markdown_style(needs_confirmation, window, cx), - ) - .into_any() + h_flex() + .id("non-card-label-container") + .w_full() + .relative() + .overflow_hidden() + .child( + h_flex() + .id("non-card-label") + .pr_8() + .w_full() + .overflow_x_scroll() + .child(self.render_markdown( + tool_call.label.clone(), + default_markdown_style( + needs_confirmation, + window, + cx, + ), + )), + ) + .child(gradient_overlay) + .on_click(cx.listener({ + let id = tool_call.id.clone(); + move |this: &mut Self, _, _, cx: &mut Context| { + if is_open { + this.expanded_tool_calls.remove(&id); + } else { + this.expanded_tool_calls.insert(id.clone()); + } + cx.notify(); + } + })) + .into_any() }), ) - .child( - h_flex() - .gap_0p5() - .when(is_collapsible, |this| { - this.child( - Disclosure::new(("expand", entry_ix), is_open) - .opened_icon(IconName::ChevronUp) - .closed_icon(IconName::ChevronDown) - .on_click(cx.listener({ - let id = tool_call.id.clone(); - move |this: &mut Self, _, _, cx: &mut Context| { - if is_open { - this.expanded_tool_calls.remove(&id); - } else { - this.expanded_tool_calls.insert(id.clone()); - } - cx.notify(); - } - })), - ) - }) - .children(status_icon), - ) - .on_click(cx.listener({ - let id = tool_call.id.clone(); - move |this: &mut Self, _, _, cx: &mut Context| { - if is_open { - this.expanded_tool_calls.remove(&id); - } else { - this.expanded_tool_calls.insert(id.clone()); - } - cx.notify(); - } - })), + .children(status_icon), ) .when(is_open, |this| { this.child( @@ -1249,8 +1329,7 @@ impl AcpThreadView { cx: &Context, ) -> Div { h_flex() - .py_1p5() - .px_1p5() + .p_1p5() .gap_1() .justify_end() .when(!empty_content, |this| { @@ -1276,6 +1355,7 @@ impl AcpThreadView { }) .icon_position(IconPosition::Start) .icon_size(IconSize::XSmall) + .label_size(LabelSize::Small) .on_click(cx.listener({ let tool_call_id = tool_call_id.clone(); let option_id = option.id.clone(); @@ -1525,7 +1605,7 @@ impl AcpThreadView { }) }) .when(!changed_buffers.is_empty(), |this| { - this.child(Divider::horizontal()) + this.child(Divider::horizontal().color(DividerColor::Border)) .child(self.render_edits_summary( action_log, &changed_buffers, @@ -1555,6 +1635,7 @@ impl AcpThreadView { { h_flex() .w_full() + .cursor_default() .gap_1() .text_xs() .text_color(cx.theme().colors().text_muted) @@ -1584,7 +1665,7 @@ impl AcpThreadView { let status_label = if stats.pending == 0 { "All Done".to_string() } else if stats.completed == 0 { - format!("{}", plan.entries.len()) + format!("{} Tasks", plan.entries.len()) } else { format!("{}/{}", stats.completed, plan.entries.len()) }; @@ -1698,7 +1779,6 @@ impl AcpThreadView { .child( h_flex() .id("edits-container") - .cursor_pointer() .w_full() .gap_1() .child(Disclosure::new("edits-disclosure", expanded)) @@ -2473,6 +2553,7 @@ impl AcpThreadView { })); h_flex() + .w_full() .mr_1() .pb_2() .px(RESPONSE_PADDING_X) diff --git a/crates/agent_ui/src/active_thread.rs b/crates/agent_ui/src/active_thread.rs index c4d4e72252..71526c8fe1 100644 --- a/crates/agent_ui/src/active_thread.rs +++ b/crates/agent_ui/src/active_thread.rs @@ -2624,7 +2624,7 @@ impl ActiveThread { h_flex() .gap_1p5() .child( - Icon::new(IconName::ToolBulb) + Icon::new(IconName::ToolThink) .size(IconSize::Small) .color(Color::Muted), ) diff --git a/crates/assistant_tools/src/thinking_tool.rs b/crates/assistant_tools/src/thinking_tool.rs index 443c2930be..76c6e6c0ba 100644 --- a/crates/assistant_tools/src/thinking_tool.rs +++ b/crates/assistant_tools/src/thinking_tool.rs @@ -37,7 +37,7 @@ impl Tool for ThinkingTool { } fn icon(&self) -> IconName { - IconName::ToolBulb + IconName::ToolThink } fn input_schema(&self, format: LanguageModelToolSchemaFormat) -> Result { diff --git a/crates/icons/src/icons.rs b/crates/icons/src/icons.rs index a94d89bdc8..12805e62e0 100644 --- a/crates/icons/src/icons.rs +++ b/crates/icons/src/icons.rs @@ -261,7 +261,6 @@ pub enum IconName { TodoComplete, TodoPending, TodoProgress, - ToolBulb, ToolCopy, ToolDeleteFile, ToolDiagnostics, @@ -273,6 +272,7 @@ pub enum IconName { ToolRegex, ToolSearch, ToolTerminal, + ToolThink, ToolWeb, Trash, Triangle, diff --git a/crates/ui/src/components/disclosure.rs b/crates/ui/src/components/disclosure.rs index a1fab02e54..98406cd1e2 100644 --- a/crates/ui/src/components/disclosure.rs +++ b/crates/ui/src/components/disclosure.rs @@ -95,7 +95,7 @@ impl RenderOnce for Disclosure { impl Component for Disclosure { fn scope() -> ComponentScope { - ComponentScope::Navigation + ComponentScope::Input } fn description() -> Option<&'static str> {