From 906bff792cb2222098ec6463d6df74bcbcdf4ccb Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Fri, 29 May 2026 20:41:25 +0530 Subject: [PATCH] agent_ui: Show permission popover when inline prompt is above the viewport (#58081) Follow up to https://github.com/zed-industries/zed/pull/57632, uses changes from https://github.com/zed-industries/zed/pull/58061 Previously the floating permission popover only appeared when the inline permission prompt was scrolled below the viewport. It now also appears when the prompt is scrolled above the viewport, with the scroll button pointing in the right direction. Release Notes: - Fixed the agent permission popover not appearing when the inline prompt was scrolled above the viewport. --- crates/agent_ui/src/conversation_view.rs | 77 ++++++++++++++++++- .../src/conversation_view/thread_view.rs | 19 ++--- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 568b94c124..c707fc9f62 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -8113,9 +8113,17 @@ pub(crate) mod tests { async fn test_permission_row_hidden_when_inline_bounds_unavailable(cx: &mut TestAppContext) { init_test(cx); - let (_view, thread_view, _entry_ix, cx) = + let (_view, thread_view, entry_ix, cx) = setup_pending_permission_thread("perm-no-bounds", cx).await; + // Pin the scroll top to the entry so it isn't treated as above the + // viewport, forcing the unmeasured-bounds path we want to exercise. + thread_view.read_with(cx, |view, _cx| { + view.list_state.scroll_to(ListOffset { + item_ix: entry_ix, + offset_in_item: px(0.0), + }); + }); thread_view.update_in(cx, |view, window, cx| { assert!( view.render_main_agent_awaiting_permission(window, cx) @@ -8176,8 +8184,8 @@ pub(crate) mod tests { let (_view, thread_view, entry_ix, cx) = setup_pending_permission_thread("perm-scroll", cx).await; - // Start off-screen below the viewport — row visible because the item - // has bounds that do not intersect the viewport. + // Start off-screen below the viewport. The row is visible because the + // item has bounds that do not intersect the viewport. draw_thread_list_at( &thread_view, ListOffset { @@ -8221,6 +8229,69 @@ pub(crate) mod tests { }); } + #[gpui::test] + async fn test_permission_row_shown_when_inline_prompt_is_above_viewport( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let (_view, thread_view, entry_ix, cx) = + setup_pending_permission_thread("perm-above", cx).await; + + let thread = thread_view.read_with(cx, |view, _cx| view.thread.clone()); + thread.update(cx, |thread, cx| { + let result = thread.handle_session_update( + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new( + "More content".into(), + )), + cx, + ); + assert!( + result.is_ok(), + "following assistant message should be accepted" + ); + }); + + draw_thread_list_at( + &thread_view, + ListOffset { + item_ix: entry_ix + 1, + offset_in_item: px(0.0), + }, + cx, + ); + thread_view.read_with(cx, |view, _cx| { + assert!( + entry_ix < view.list_state.logical_scroll_top().item_ix, + "The tool call entry should be above the logical scroll top" + ); + }); + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_some(), + "Floating row should be visible when the inline prompt is above the viewport" + ); + }); + + // Scrolling up to the entry brings it back into view. + draw_thread_list_at( + &thread_view, + ListOffset { + item_ix: entry_ix, + offset_in_item: px(0.0), + }, + cx, + ); + thread_view.update_in(cx, |view, window, cx| { + assert!( + view.render_main_agent_awaiting_permission(window, cx) + .is_none(), + "Floating row should disappear after scrolling brings the inline prompt into view" + ); + }); + } + #[gpui::test] async fn test_permission_row_disappears_when_authorized(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index a90fef49bd..32969d76d3 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -3047,15 +3047,6 @@ impl ThreadView { ) } - /// Returns true when the entry has been measured and sits entirely below - /// the current viewport. - fn entry_is_below_viewport(&self, entry_ix: usize) -> bool { - let viewport_bounds = self.list_state.viewport_bounds(); - self.list_state - .bounds_for_item(entry_ix) - .is_some_and(|entry_bounds| entry_bounds.top() >= viewport_bounds.bottom()) - } - pub(crate) fn render_main_agent_awaiting_permission( &self, window: &Window, @@ -3073,9 +3064,13 @@ impl ThreadView { let thread = self.thread.read(cx); let (entry_ix, tool_call) = thread.tool_call(&tool_call_id)?; - if !self.entry_is_below_viewport(entry_ix) { + let scroll_icon = if self.list_state.item_is_above_viewport(entry_ix)? { + IconName::ArrowUp + } else if self.list_state.item_is_below_viewport(entry_ix)? { + IconName::ArrowDown + } else { return None; - } + }; let focus_handle = self.focus_handle(cx); @@ -3118,7 +3113,7 @@ impl ThreadView { Button::new("main-agent-permission-scroll-to", "Scroll") .label_size(LabelSize::Small) .end_icon( - Icon::new(IconName::ArrowDown) + Icon::new(scroll_icon) .size(IconSize::XSmall) .color(Color::Default), )