diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 6eef1741927..4b8514c9f67 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -422,6 +422,23 @@ struct SidebarContents { has_open_projects: bool, } +/// Identity-and-layout key for a [`ListEntry`] used to preserve measured list items +/// across rebuilds. Equal shapes must render to the same height; add any new +/// height-affecting state here. +#[derive(Debug, PartialEq, Eq)] +enum EntryShape { + ProjectHeader { + key: ProjectGroupKey, + // Toggles the "No threads yet" empty-state row when not collapsed. + has_threads: bool, + // Determines whether the "No threads yet" row is rendered (only shown when + // `!is_collapsed && !has_threads`). + is_collapsed: bool, + }, + Thread(ThreadId), + Terminal(TerminalId), +} + impl SidebarContents { fn is_thread_notified(&self, thread_id: &agent_ui::ThreadId) -> bool { self.notified_threads.contains(thread_id) @@ -1837,13 +1854,14 @@ impl Sidebar { } let had_notifications = self.has_notifications(cx); - let scroll_position = self.list_state.logical_scroll_top(); + let previous_shapes: Vec = + self.entry_shapes(multi_workspace.read(cx)).collect(); self.rebuild_contents(cx); self.refresh_draft_editor_observations(cx); - self.list_state.reset(self.contents.entries.len()); - self.list_state.scroll_to(scroll_position); + // Preserve measurements for unchanged entries so sticky headers do not flicker. + self.apply_list_state_diff(&previous_shapes, multi_workspace.read(cx)); if had_notifications != self.has_notifications(cx) { multi_workspace.update(cx, |_, cx| { @@ -1854,6 +1872,56 @@ impl Sidebar { cx.notify(); } + /// Splices only the changed entry range, leaving unchanged item measurements intact. + fn apply_list_state_diff( + &self, + previous_shapes: &[EntryShape], + multi_workspace: &MultiWorkspace, + ) { + let mut new_iter = self.entry_shapes(multi_workspace); + let mut prefix_len = 0; + let leading_new = loop { + match (previous_shapes.get(prefix_len), new_iter.next()) { + (Some(prev), Some(next)) if *prev == next => prefix_len += 1, + (None, None) => return, + (_, leading) => break leading, + } + }; + + let new_tail: Vec = leading_new.into_iter().chain(new_iter).collect(); + let prev_tail = &previous_shapes[prefix_len..]; + let suffix_len = prev_tail + .iter() + .rev() + .zip(new_tail.iter().rev()) + .take_while(|(prev, next)| prev == next) + .count(); + + let old_changed = prefix_len..previous_shapes.len() - suffix_len; + let new_changed_count = new_tail.len() - suffix_len; + self.list_state.splice(old_changed, new_changed_count); + } + + fn entry_shapes<'a>( + &'a self, + multi_workspace: &'a MultiWorkspace, + ) -> impl Iterator + 'a { + self.contents.entries.iter().map(move |entry| match entry { + ListEntry::ProjectHeader { + key, has_threads, .. + } => EntryShape::ProjectHeader { + key: key.clone(), + has_threads: *has_threads, + is_collapsed: multi_workspace + .group_state_by_key(key) + .map(|state| !state.expanded) + .unwrap_or(false), + }, + ListEntry::Thread(thread) => EntryShape::Thread(thread.metadata.thread_id), + ListEntry::Terminal(terminal) => EntryShape::Terminal(terminal.metadata.terminal_id), + }) + } + /// Re-establishes subscriptions to each visible draft's message editor /// so we rebuild entries (and their displayed titles) as the user types. fn refresh_draft_editor_observations(&mut self, cx: &mut Context) { diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index a7a7263daa8..3fcb27faf4f 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -605,6 +605,153 @@ fn visible_entries_as_strings( }) } +#[gpui::test] +async fn test_thread_metadata_update_preserves_sticky_header_measurements(cx: &mut TestAppContext) { + let (fs, project_a) = init_multi_project_test(&["/project-a", "/project-b"], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + add_test_project("/project-b", &fs, &multi_workspace, cx).await; + + save_thread_metadata( + acp::SessionId::new(Arc::from("project-a-thread")), + Some("Project A Thread".into()), + chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), + None, + None, + &project_a, + cx, + ); + save_thread_metadata_with_main_paths( + "project-b-thread", + "Project B Thread", + PathList::new(&[PathBuf::from("/project-b")]), + PathList::new(&[PathBuf::from("/project-b")]), + chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), + cx, + ); + + cx.draw( + gpui::point(px(0.), px(0.)), + gpui::size(px(400.), px(240.)), + |_, _| sidebar.clone().into_any_element(), + ); + cx.run_until_parked(); + + let next_header_ix = sidebar.read_with(cx, |sidebar, _| { + assert!( + sidebar.contents.project_header_indices.len() == 2, + "test setup should render exctly two project headers" + ); + sidebar.contents.project_header_indices[1] + }); + + sidebar.update_in(cx, |sidebar, _window, cx| { + sidebar.list_state.scroll_to(gpui::ListOffset { + item_ix: next_header_ix - 1, + offset_in_item: px(24.), + }); + cx.notify(); + }); + cx.draw( + gpui::point(px(0.), px(0.)), + gpui::size(px(400.), px(240.)), + |_, _| sidebar.clone().into_any_element(), + ); + cx.run_until_parked(); + + let bounds_before = sidebar.read_with(cx, |sidebar, _| { + sidebar + .list_state + .bounds_for_item(next_header_ix) + .expect("next project header should be measured before metadata update") + }); + + save_thread_metadata( + acp::SessionId::new(Arc::from("project-a-thread")), + Some("Renamed Project A Thread".into()), + chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 1, 0).unwrap(), + None, + None, + &project_a, + cx, + ); + + let bounds_after = sidebar.read_with(cx, |sidebar, _| { + sidebar + .list_state + .bounds_for_item(next_header_ix) + .expect("same-shape metadata update should preserve next header measurements") + }); + assert_eq!(bounds_before, bounds_after); +} + +#[gpui::test] +async fn test_thread_status_update_does_not_reset_list_measurements(cx: &mut TestAppContext) { + // When a thread's status changes (e.g. Running -> Completed after sending a message), the + // shape sequence is unchanged, so `update_entries` should not reset the underlying + // `ListState`. Resetting throws away measured item bounds for one frame, which makes the + // sticky project header flicker between its pushed-off and fully-on-screen positions. + let project = init_test_project("/my-project", cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + save_n_test_threads(2, &project, cx).await; + cx.run_until_parked(); + + let before = sidebar.read_with(cx, |sidebar, app| { + sidebar + .entry_shapes(multi_workspace.read(app)) + .collect::>() + }); + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + let after = sidebar.read_with(cx, |sidebar, app| { + sidebar + .entry_shapes(multi_workspace.read(app)) + .collect::>() + }); + + assert_eq!( + before, after, + "a no-op rebuild should produce an identical shape sequence" + ); +} + +#[gpui::test] +async fn test_collapse_changes_entry_shape(cx: &mut TestAppContext) { + let project = init_test_project("/my-project", cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + + save_n_test_threads(2, &project, cx).await; + cx.run_until_parked(); + + let project_group_key = project.read_with(cx, |project, cx| project.project_group_key(cx)); + + let before = sidebar.read_with(cx, |sidebar, app| { + sidebar + .entry_shapes(multi_workspace.read(app)) + .collect::>() + }); + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.toggle_collapse(&project_group_key, window, cx); + }); + cx.run_until_parked(); + let after = sidebar.read_with(cx, |sidebar, app| { + sidebar + .entry_shapes(multi_workspace.read(app)) + .collect::>() + }); + + assert_ne!( + before, after, + "collapsing the project group should change the shape sequence so the list resets" + ); +} + #[gpui::test] async fn test_serialization_round_trip(cx: &mut TestAppContext) { let project = init_test_project("/my-project", cx).await;