Fix autoscroll in the new assistant (#10928)

Antonio Scandurra created 2 years ago

This removes the manual calls to `scroll_to_reveal_item` in the new
assistant, as they are superseded by the new autoscrolling behavior of
the `List` when the editor requests one.

Release Notes:

- N/A

Change summary

crates/assistant2/src/assistant2.rs | 24 --------------
crates/gpui/src/elements/list.rs    | 49 ++++++++++++++++++++++++++----
2 files changed, 43 insertions(+), 30 deletions(-)

Detailed changes

crates/assistant2/src/assistant2.rs

@@ -6,7 +6,7 @@ use anyhow::{Context, Result};
 use assistant_tooling::{ToolFunctionCall, ToolRegistry};
 use client::{proto, Client};
 use completion_provider::*;
-use editor::{Editor, EditorEvent};
+use editor::Editor;
 use feature_flags::FeatureFlagAppExt as _;
 use futures::{channel::oneshot, future::join_all, Future, FutureExt, StreamExt};
 use gpui::{
@@ -426,31 +426,10 @@ impl AssistantChat {
             }
             editor
         });
-        let _subscription = cx.subscribe(&body, move |this, editor, event, cx| match event {
-            EditorEvent::SelectionsChanged { .. } => {
-                if editor.read(cx).is_focused(cx) {
-                    let (message_ix, _message) = this
-                        .messages
-                        .iter()
-                        .enumerate()
-                        .find_map(|(ix, message)| match message {
-                            ChatMessage::User(user_message) if user_message.id == id => {
-                                Some((ix, user_message))
-                            }
-                            _ => None,
-                        })
-                        .expect("user message not found");
-
-                    this.list_state.scroll_to_reveal_item(message_ix);
-                }
-            }
-            _ => {}
-        });
         let message = ChatMessage::User(UserMessage {
             id,
             body,
             contexts: Vec::new(),
-            _subscription,
         });
         self.push_message(message, cx);
     }
@@ -733,7 +712,6 @@ struct UserMessage {
     id: MessageId,
     body: View<Editor>,
     contexts: Vec<AssistantContext>,
-    _subscription: gpui::Subscription,
 }
 
 struct AssistantMessage {

crates/gpui/src/elements/list.rs

@@ -608,6 +608,7 @@ impl StateInner {
         &mut self,
         bounds: Bounds<Pixels>,
         padding: Edges<Pixels>,
+        autoscroll: bool,
         cx: &mut ElementContext,
     ) -> Result<LayoutItemsResponse, ListOffset> {
         cx.transact(|cx| {
@@ -627,11 +628,45 @@ impl StateInner {
                     });
 
                     if let Some(autoscroll_bounds) = cx.take_autoscroll() {
-                        if bounds.intersect(&autoscroll_bounds) != autoscroll_bounds {
-                            return Err(ListOffset {
-                                item_ix: item.index,
-                                offset_in_item: autoscroll_bounds.origin.y - item_origin.y,
-                            });
+                        if autoscroll {
+                            if autoscroll_bounds.top() < bounds.top() {
+                                return Err(ListOffset {
+                                    item_ix: item.index,
+                                    offset_in_item: autoscroll_bounds.top() - item_origin.y,
+                                });
+                            } else if autoscroll_bounds.bottom() > bounds.bottom() {
+                                let mut cursor = self.items.cursor::<Count>();
+                                cursor.seek(&Count(item.index), Bias::Right, &());
+                                let mut height = bounds.size.height - padding.top - padding.bottom;
+
+                                // Account for the height of the element down until the autoscroll bottom.
+                                height -= autoscroll_bounds.bottom() - item_origin.y;
+
+                                // Keep decreasing the scroll top until we fill all the available space.
+                                while height > Pixels::ZERO {
+                                    cursor.prev(&());
+                                    let Some(item) = cursor.item() else { break };
+
+                                    let size = item.size().unwrap_or_else(|| {
+                                        let mut item = (self.render_item)(cursor.start().0, cx);
+                                        let item_available_size = size(
+                                            bounds.size.width.into(),
+                                            AvailableSpace::MinContent,
+                                        );
+                                        item.layout_as_root(item_available_size, cx)
+                                    });
+                                    height -= size.height;
+                                }
+
+                                return Err(ListOffset {
+                                    item_ix: cursor.start().0,
+                                    offset_in_item: if height < Pixels::ZERO {
+                                        -height
+                                    } else {
+                                        Pixels::ZERO
+                                    },
+                                });
+                            }
                         }
                     }
 
@@ -762,11 +797,11 @@ impl Element for List {
         }
 
         let padding = style.padding.to_pixels(bounds.size.into(), cx.rem_size());
-        let layout = match state.prepaint_items(bounds, padding, cx) {
+        let layout = match state.prepaint_items(bounds, padding, true, cx) {
             Ok(layout) => layout,
             Err(autoscroll_request) => {
                 state.logical_scroll_top = Some(autoscroll_request);
-                state.prepaint_items(bounds, padding, cx).unwrap()
+                state.prepaint_items(bounds, padding, false, cx).unwrap()
             }
         };