agent: Show delete thread icon buttons on hover/focus (#30370)

This PR's main goal is to show the delete thread button when the list
item is either focused or hovered. In order to do that, we ended up
refactoring (i.e., merging) the `PastThread` and `PastContext` elements
into a single `HistoryElementEntry` that already matches to the entry
type (i.e., context or thread).

Release Notes:

- agent: Simplify the UI by showing the delete thread icon button only
on hover or focus.

---------

Co-authored-by: Agus Zubiaga <hi@aguz.me>

crates/agent/src/agent_panel.rs

@@ -55,10 +55,10 @@ use zed_llm_client::UsageLimit;
 use crate::active_thread::{self, ActiveThread, ActiveThreadEvent};
 use crate::agent_configuration::{AgentConfiguration, AssistantConfigurationEvent};
 use crate::agent_diff::AgentDiff;
-use crate::history_store::{HistoryEntry, HistoryStore, RecentEntry};
+use crate::history_store::{HistoryStore, RecentEntry};
 use crate::message_editor::{MessageEditor, MessageEditorEvent};
 use crate::thread::{Thread, ThreadError, ThreadId, TokenUsageRatio};
-use crate::thread_history::{EntryTimeFormat, PastContext, PastThread, ThreadHistory};
+use crate::thread_history::{HistoryEntryElement, ThreadHistory};
 use crate::thread_store::ThreadStore;
 use crate::ui::AgentOnboardingModal;
 use crate::{
@@ -356,6 +356,7 @@ pub struct AgentPanel {
     previous_view: Option<ActiveView>,
     history_store: Entity<HistoryStore>,
     history: Entity<ThreadHistory>,
+    hovered_recent_history_item: Option<usize>,
     assistant_dropdown_menu_handle: PopoverMenuHandle<ContextMenu>,
     assistant_navigation_menu_handle: PopoverMenuHandle<ContextMenu>,
     assistant_navigation_menu: Option<Entity<ContextMenu>>,
@@ -696,6 +697,7 @@ impl AgentPanel {
             previous_view: None,
             history_store: history_store.clone(),
             history: cx.new(|cx| ThreadHistory::new(weak_self, history_store, window, cx)),
+            hovered_recent_history_item: None,
             assistant_dropdown_menu_handle: PopoverMenuHandle::default(),
             assistant_navigation_menu_handle: PopoverMenuHandle::default(),
             assistant_navigation_menu: None,
@@ -2212,7 +2214,7 @@ impl AgentPanel {
                             .border_b_1()
                             .border_color(cx.theme().colors().border_variant)
                             .child(
-                                Label::new("Past Interactions")
+                                Label::new("Recent")
                                     .size(LabelSize::Small)
                                     .color(Color::Muted),
                             )
@@ -2237,18 +2239,20 @@ impl AgentPanel {
                         v_flex()
                             .gap_1()
                             .children(
-                                recent_history.into_iter().map(|entry| {
+                                recent_history.into_iter().enumerate().map(|(index, entry)| {
                                     // TODO: Add keyboard navigation.
-                                    match entry {
+                                    let is_hovered = self.hovered_recent_history_item == Some(index);
-                                        HistoryEntry::Thread(thread) => {
+                                    HistoryEntryElement::new(entry.clone(), cx.entity().downgrade())
-                                            PastThread::new(thread, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime)
+                                        .hovered(is_hovered)
+                                        .on_hover(cx.listener(move |this, is_hovered, _window, cx| {
+                                            if *is_hovered {
+                                                this.hovered_recent_history_item = Some(index);
+                                            } else if this.hovered_recent_history_item == Some(index) {
+                                                this.hovered_recent_history_item = None;
+                                            }
+                                            cx.notify();
+                                        }))
                                         .into_any_element()
-                                        }
-                                        HistoryEntry::Context(context) => {
-                                            PastContext::new(context, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime)
-                                                .into_any_element()
-                                        }
-                                    }
                                 }),
                             )
                     )

crates/agent/src/thread_history.rs

@@ -2,12 +2,11 @@ use std::fmt::Display;
 use std::ops::Range;
 use std::sync::Arc;
 
-use assistant_context_editor::SavedContextMetadata;
 use chrono::{Datelike as _, Local, NaiveDate, TimeDelta};
 use editor::{Editor, EditorEvent};
 use fuzzy::{StringMatch, StringMatchCandidate};
 use gpui::{
-    App, Empty, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task,
+    App, ClickEvent, Empty, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task,
     UniformListScrollHandle, WeakEntity, Window, uniform_list,
 };
 use time::{OffsetDateTime, UtcOffset};
@@ -18,7 +17,6 @@ use ui::{
 use util::ResultExt;
 
 use crate::history_store::{HistoryEntry, HistoryStore};
-use crate::thread_store::SerializedThreadMetadata;
 use crate::{AgentPanel, RemoveSelectedThread};
 
 pub struct ThreadHistory {
@@ -26,11 +24,12 @@ pub struct ThreadHistory {
     history_store: Entity<HistoryStore>,
     scroll_handle: UniformListScrollHandle,
     selected_index: usize,
+    hovered_index: Option<usize>,
     search_editor: Entity<Editor>,
     all_entries: Arc<Vec<HistoryEntry>>,
     // When the search is empty, we display date separators between history entries
     // This vector contains an enum of either a separator or an actual entry
-    separated_items: Vec<HistoryListItem>,
+    separated_items: Vec<ListItemType>,
     // Maps entry indexes to list item indexes
     separated_item_indexes: Vec<u32>,
     _separated_items_task: Option<Task<()>>,
@@ -52,7 +51,7 @@ enum SearchState {
     },
 }
 
-enum HistoryListItem {
+enum ListItemType {
     BucketSeparator(TimeBucket),
     Entry {
         index: usize,
@@ -60,11 +59,11 @@ enum HistoryListItem {
     },
 }
 
-impl HistoryListItem {
+impl ListItemType {
     fn entry_index(&self) -> Option<usize> {
         match self {
-            HistoryListItem::BucketSeparator(_) => None,
+            ListItemType::BucketSeparator(_) => None,
-            HistoryListItem::Entry { index, .. } => Some(*index),
+            ListItemType::Entry { index, .. } => Some(*index),
         }
     }
 }
@@ -102,6 +101,7 @@ impl ThreadHistory {
             history_store,
             scroll_handle,
             selected_index: 0,
+            hovered_index: None,
             search_state: SearchState::Empty,
             all_entries: Default::default(),
             separated_items: Default::default(),
@@ -160,11 +160,11 @@ impl ThreadHistory {
 
                 if Some(entry_bucket) != bucket {
                     bucket = Some(entry_bucket);
-                    items.push(HistoryListItem::BucketSeparator(entry_bucket));
+                    items.push(ListItemType::BucketSeparator(entry_bucket));
                 }
 
                 indexes.push(items.len() as u32);
-                items.push(HistoryListItem::Entry {
+                items.push(ListItemType::Entry {
                     index,
                     format: entry_bucket.into(),
                 });
@@ -467,7 +467,7 @@ impl ThreadHistory {
                 .map(|(ix, m)| {
                     self.render_list_item(
                         Some(range_start + ix),
-                        &HistoryListItem::Entry {
+                        &ListItemType::Entry {
                             index: m.candidate_id,
                             format: EntryTimeFormat::DateAndTime,
                         },
@@ -485,25 +485,36 @@ impl ThreadHistory {
     fn render_list_item(
         &self,
         list_entry_ix: Option<usize>,
-        item: &HistoryListItem,
+        item: &ListItemType,
         highlight_positions: Vec<usize>,
-        cx: &App,
+        cx: &Context<Self>,
     ) -> AnyElement {
         match item {
-            HistoryListItem::Entry { index, format } => match self.all_entries.get(*index) {
+            ListItemType::Entry { index, format } => match self.all_entries.get(*index) {
                 Some(entry) => h_flex()
                     .w_full()
                     .pb_1()
-                    .child(self.render_history_entry(
+                    .child(
-                        entry,
+                        HistoryEntryElement::new(entry.clone(), self.agent_panel.clone())
-                        list_entry_ix == Some(self.selected_index),
+                            .highlight_positions(highlight_positions)
-                        highlight_positions,
+                            .timestamp_format(*format)
-                        *format,
+                            .selected(list_entry_ix == Some(self.selected_index))
-                    ))
+                            .hovered(list_entry_ix == self.hovered_index)
+                            .on_hover(cx.listener(move |this, is_hovered, _window, cx| {
+                                if *is_hovered {
+                                    this.hovered_index = list_entry_ix;
+                                } else if this.hovered_index == list_entry_ix {
+                                    this.hovered_index = None;
+                                }
+
+                                cx.notify();
+                            }))
+                            .into_any_element(),
+                    )
                     .into_any(),
                 None => Empty.into_any_element(),
             },
-            HistoryListItem::BucketSeparator(bucket) => div()
+            ListItemType::BucketSeparator(bucket) => div()
                 .px(DynamicSpacing::Base06.rems(cx))
                 .pt_2()
                 .pb_1()
@@ -515,33 +526,6 @@ impl ThreadHistory {
                 .into_any_element(),
         }
     }
-
-    fn render_history_entry(
-        &self,
-        entry: &HistoryEntry,
-        is_active: bool,
-        highlight_positions: Vec<usize>,
-        format: EntryTimeFormat,
-    ) -> AnyElement {
-        match entry {
-            HistoryEntry::Thread(thread) => PastThread::new(
-                thread.clone(),
-                self.agent_panel.clone(),
-                is_active,
-                highlight_positions,
-                format,
-            )
-            .into_any_element(),
-            HistoryEntry::Context(context) => PastContext::new(
-                context.clone(),
-                self.agent_panel.clone(),
-                is_active,
-                highlight_positions,
-                format,
-            )
-            .into_any_element(),
-        }
-    }
 }
 
 impl Focusable for ThreadHistory {
@@ -623,62 +607,97 @@ impl Render for ThreadHistory {
 }
 
 #[derive(IntoElement)]
-pub struct PastThread {
+pub struct HistoryEntryElement {
-    thread: SerializedThreadMetadata,
+    entry: HistoryEntry,
     agent_panel: WeakEntity<AgentPanel>,
     selected: bool,
+    hovered: bool,
     highlight_positions: Vec<usize>,
     timestamp_format: EntryTimeFormat,
+    on_hover: Box<dyn Fn(&bool, &mut Window, &mut App) + 'static>,
 }
 
-impl PastThread {
+impl HistoryEntryElement {
-    pub fn new(
+    pub fn new(entry: HistoryEntry, agent_panel: WeakEntity<AgentPanel>) -> Self {
-        thread: SerializedThreadMetadata,
-        agent_panel: WeakEntity<AgentPanel>,
-        selected: bool,
-        highlight_positions: Vec<usize>,
-        timestamp_format: EntryTimeFormat,
-    ) -> Self {
         Self {
-            thread,
+            entry,
             agent_panel,
-            selected,
+            selected: false,
-            highlight_positions,
+            hovered: false,
-            timestamp_format,
+            highlight_positions: vec![],
+            timestamp_format: EntryTimeFormat::DateAndTime,
+            on_hover: Box::new(|_, _, _| {}),
         }
     }
+
+    pub fn selected(mut self, selected: bool) -> Self {
+        self.selected = selected;
+        self
+    }
+
+    pub fn hovered(mut self, hovered: bool) -> Self {
+        self.hovered = hovered;
+        self
+    }
+
+    pub fn highlight_positions(mut self, positions: Vec<usize>) -> Self {
+        self.highlight_positions = positions;
+        self
+    }
+
+    pub fn on_hover(mut self, on_hover: impl Fn(&bool, &mut Window, &mut App) + 'static) -> Self {
+        self.on_hover = Box::new(on_hover);
+        self
+    }
+
+    pub fn timestamp_format(mut self, format: EntryTimeFormat) -> Self {
+        self.timestamp_format = format;
+        self
+    }
 }
 
-impl RenderOnce for PastThread {
+impl RenderOnce for HistoryEntryElement {
     fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement {
-        let summary = self.thread.summary;
+        let (id, summary, timestamp) = match &self.entry {
+            HistoryEntry::Thread(thread) => (
+                thread.id.to_string(),
+                thread.summary.clone(),
+                thread.updated_at.timestamp(),
+            ),
+            HistoryEntry::Context(context) => (
+                context.path.to_string_lossy().to_string(),
+                context.title.clone().into(),
+                context.mtime.timestamp(),
+            ),
+        };
 
-        let thread_timestamp = self.timestamp_format.format_timestamp(
+        let thread_timestamp =
-            &self.agent_panel,
+            self.timestamp_format
-            self.thread.updated_at.timestamp(),
+                .format_timestamp(&self.agent_panel, timestamp, cx);
-            cx,
-        );
 
-        ListItem::new(SharedString::from(self.thread.id.to_string()))
+        ListItem::new(SharedString::from(id))
             .rounded()
             .toggle_state(self.selected)
             .spacing(ListItemSpacing::Sparse)
             .start_slot(
-                div().max_w_4_5().child(
+                h_flex()
+                    .w_full()
+                    .gap_2()
+                    .justify_between()
+                    .child(
                         HighlightedLabel::new(summary, self.highlight_positions)
                             .size(LabelSize::Small)
                             .truncate(),
-                ),
                     )
-            .end_slot(
-                h_flex()
-                    .gap_1p5()
                     .child(
                         Label::new(thread_timestamp)
                             .color(Color::Muted)
                             .size(LabelSize::XSmall),
+                    ),
             )
-                    .child(
+            .on_hover(self.on_hover)
+            .end_slot::<IconButton>(if self.hovered || self.selected {
+                Some(
                     IconButton::new("delete", IconName::TrashAlt)
                         .shape(IconButtonShape::Square)
                         .icon_size(IconSize::XSmall)
@@ -688,122 +707,69 @@ impl RenderOnce for PastThread {
                         })
                         .on_click({
                             let agent_panel = self.agent_panel.clone();
-                                let id = self.thread.id.clone();
+
-                                move |_event, _window, cx| {
+                            let f: Box<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static> =
+                                match &self.entry {
+                                    HistoryEntry::Thread(thread) => {
+                                        let id = thread.id.clone();
+
+                                        Box::new(move |_event, _window, cx| {
                                             agent_panel
                                                 .update(cx, |this, cx| {
-                                            this.delete_thread(&id, cx).detach_and_log_err(cx);
+                                                    this.delete_thread(&id, cx)
-                                        })
-                                        .ok();
-                                }
-                            }),
-                    ),
-            )
-            .on_click({
-                let agent_panel = self.agent_panel.clone();
-                let id = self.thread.id.clone();
-                move |_event, window, cx| {
-                    agent_panel
-                        .update(cx, |this, cx| {
-                            this.open_thread_by_id(&id, window, cx)
                                                         .detach_and_log_err(cx);
                                                 })
                                                 .ok();
-                }
                                         })
                                     }
-}
+                                    HistoryEntry::Context(context) => {
+                                        let path = context.path.clone();
 
-#[derive(IntoElement)]
+                                        Box::new(move |_event, _window, cx| {
-pub struct PastContext {
-    context: SavedContextMetadata,
-    agent_panel: WeakEntity<AgentPanel>,
-    selected: bool,
-    highlight_positions: Vec<usize>,
-    timestamp_format: EntryTimeFormat,
-}
-
-impl PastContext {
-    pub fn new(
-        context: SavedContextMetadata,
-        agent_panel: WeakEntity<AgentPanel>,
-        selected: bool,
-        highlight_positions: Vec<usize>,
-        timestamp_format: EntryTimeFormat,
-    ) -> Self {
-        Self {
-            context,
-            agent_panel,
-            selected,
-            highlight_positions,
-            timestamp_format,
-        }
-    }
-}
-
-impl RenderOnce for PastContext {
-    fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement {
-        let summary = self.context.title;
-        let context_timestamp = self.timestamp_format.format_timestamp(
-            &self.agent_panel,
-            self.context.mtime.timestamp(),
-            cx,
-        );
-
-        ListItem::new(SharedString::from(
-            self.context.path.to_string_lossy().to_string(),
-        ))
-        .rounded()
-        .toggle_state(self.selected)
-        .spacing(ListItemSpacing::Sparse)
-        .start_slot(
-            div().max_w_4_5().child(
-                HighlightedLabel::new(summary, self.highlight_positions)
-                    .size(LabelSize::Small)
-                    .truncate(),
-            ),
-        )
-        .end_slot(
-            h_flex()
-                .gap_1p5()
-                .child(
-                    Label::new(context_timestamp)
-                        .color(Color::Muted)
-                        .size(LabelSize::XSmall),
-                )
-                .child(
-                    IconButton::new("delete", IconName::TrashAlt)
-                        .shape(IconButtonShape::Square)
-                        .icon_size(IconSize::XSmall)
-                        .icon_color(Color::Muted)
-                        .tooltip(move |window, cx| {
-                            Tooltip::for_action("Delete", &RemoveSelectedThread, window, cx)
-                        })
-                        .on_click({
-                            let agent_panel = self.agent_panel.clone();
-                            let path = self.context.path.clone();
-                            move |_event, _window, cx| {
                                             agent_panel
                                                 .update(cx, |this, cx| {
                                                     this.delete_context(path.clone(), cx)
                                                         .detach_and_log_err(cx);
                                                 })
                                                 .ok();
+                                        })
                                     }
+                                };
+                            f
                         }),
-                ),
                 )
+            } else {
+                None
+            })
             .on_click({
                 let agent_panel = self.agent_panel.clone();
-            let path = self.context.path.clone();
+
-            move |_event, window, cx| {
+                let f: Box<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static> = match &self.entry
+                {
+                    HistoryEntry::Thread(thread) => {
+                        let id = thread.id.clone();
+                        Box::new(move |_event, window, cx| {
+                            agent_panel
+                                .update(cx, |this, cx| {
+                                    this.open_thread_by_id(&id, window, cx)
+                                        .detach_and_log_err(cx);
+                                })
+                                .ok();
+                        })
+                    }
+                    HistoryEntry::Context(context) => {
+                        let path = context.path.clone();
+                        Box::new(move |_event, window, cx| {
                             agent_panel
                                 .update(cx, |this, cx| {
                                     this.open_saved_prompt_editor(path.clone(), window, cx)
                                         .detach_and_log_err(cx);
                                 })
                                 .ok();
+                        })
                     }
+                };
+                f
             })
     }
 }

crates/ui/src/components/list/list_item.rs

@@ -33,6 +33,7 @@ pub struct ListItem {
     toggle: Option<bool>,
     inset: bool,
     on_click: Option<Box<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>,
+    on_hover: Option<Box<dyn Fn(&bool, &mut Window, &mut App) + 'static>>,
     on_toggle: Option<Arc<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>>,
     tooltip: Option<Box<dyn Fn(&mut Window, &mut App) -> AnyView + 'static>>,
     on_secondary_mouse_down: Option<Box<dyn Fn(&MouseDownEvent, &mut Window, &mut App) + 'static>>,
@@ -63,6 +64,7 @@ impl ListItem {
             on_click: None,
             on_secondary_mouse_down: None,
             on_toggle: None,
+            on_hover: None,
             tooltip: None,
             children: SmallVec::new(),
             selectable: true,
@@ -102,6 +104,11 @@ impl ListItem {
         self
     }
 
+    pub fn on_hover(mut self, handler: impl Fn(&bool, &mut Window, &mut App) + 'static) -> Self {
+        self.on_hover = Some(Box::new(handler));
+        self
+    }
+
     pub fn on_secondary_mouse_down(
         mut self,
         handler: impl Fn(&MouseDownEvent, &mut Window, &mut App) + 'static,
@@ -233,6 +240,7 @@ impl RenderOnce for ListItem {
                     })
             })
             .when(self.rounded, |this| this.rounded_sm())
+            .when_some(self.on_hover, |this, on_hover| this.on_hover(on_hover))
             .child(
                 h_flex()
                     .id("inner_list_item")