Fix scrollbar auto-show for cursors out of sight (#11244)

This PR fixes scrollbar auto-show feature when there're not visible
cursors. The intial behavior was broken in #11147.

The problem is that `selections` only contains visible selections, so
the `if` with `non_visible_cursors |= true` is only visited in rare edge
cases when we have at least part of the selection still visible on the
screen. But when we scroll far enough from the cursor,
`non_visible_cursors` keeps its default `false` value, which is
incorrect.

Release Notes:

- N/A
This commit is contained in:
Andrew Lygin 2024-05-01 21:57:10 +03:00 committed by GitHub
parent 3dc5d48e0c
commit 4b767697af
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -875,10 +875,13 @@ impl EditorElement {
&self, &self,
snapshot: &EditorSnapshot, snapshot: &EditorSnapshot,
cx: &mut WindowContext, cx: &mut WindowContext,
) -> Vec<(Anchor, Hsla)> { ) -> Vec<(DisplayPoint, Hsla)> {
let editor = self.editor.read(cx); let editor = self.editor.read(cx);
let mut cursors = Vec::<(Anchor, Hsla)>::new(); let mut cursors = Vec::new();
let mut skip_local = false; let mut skip_local = false;
let mut add_cursor = |anchor: Anchor, color| {
cursors.push((anchor.to_display_point(&snapshot.display_snapshot), color));
};
// Remote cursors // Remote cursors
if let Some(collaboration_hub) = &editor.collaboration_hub { if let Some(collaboration_hub) = &editor.collaboration_hub {
for remote_selection in snapshot.remote_selections_in_range( for remote_selection in snapshot.remote_selections_in_range(
@ -887,7 +890,7 @@ impl EditorElement {
cx, cx,
) { ) {
let color = Self::get_participant_color(remote_selection.participant_index, cx); let color = Self::get_participant_color(remote_selection.participant_index, cx);
cursors.push((remote_selection.selection.head(), color.cursor)); add_cursor(remote_selection.selection.head(), color.cursor);
if Some(remote_selection.peer_id) == editor.leader_peer_id { if Some(remote_selection.peer_id) == editor.leader_peer_id {
skip_local = true; skip_local = true;
} }
@ -895,11 +898,12 @@ impl EditorElement {
} }
// Local cursors // Local cursors
if !skip_local { if !skip_local {
let color = cx.theme().players().local().cursor;
editor.selections.disjoint.iter().for_each(|selection| { editor.selections.disjoint.iter().for_each(|selection| {
cursors.push((selection.head(), cx.theme().players().local().cursor)); add_cursor(selection.head(), color);
}); });
if let Some(ref selection) = editor.selections.pending_anchor() { if let Some(ref selection) = editor.selections.pending_anchor() {
cursors.push((selection.head(), cx.theme().players().local().cursor)); add_cursor(selection.head(), color);
} }
} }
cursors cursors
@ -920,9 +924,8 @@ impl EditorElement {
em_width: Pixels, em_width: Pixels,
autoscroll_containing_element: bool, autoscroll_containing_element: bool,
cx: &mut WindowContext, cx: &mut WindowContext,
) -> (Vec<CursorLayout>, bool) { ) -> Vec<CursorLayout> {
let mut autoscroll_bounds = None; let mut autoscroll_bounds = None;
let mut non_visible_cursors = false;
let cursor_layouts = self.editor.update(cx, |editor, cx| { let cursor_layouts = self.editor.update(cx, |editor, cx| {
let mut cursors = Vec::new(); let mut cursors = Vec::new();
for (player_color, selections) in selections { for (player_color, selections) in selections {
@ -930,10 +933,6 @@ impl EditorElement {
let cursor_position = selection.head; let cursor_position = selection.head;
let in_range = visible_display_row_range.contains(&cursor_position.row()); let in_range = visible_display_row_range.contains(&cursor_position.row());
if !in_range {
non_visible_cursors |= true;
}
if (selection.is_local && !editor.show_local_cursors(cx)) || !in_range { if (selection.is_local && !editor.show_local_cursors(cx)) || !in_range {
continue; continue;
} }
@ -1041,7 +1040,7 @@ impl EditorElement {
cx.request_autoscroll(bounds); cx.request_autoscroll(bounds);
} }
(cursor_layouts, non_visible_cursors) cursor_layouts
} }
fn layout_scrollbar( fn layout_scrollbar(
@ -2699,15 +2698,10 @@ impl EditorElement {
let cursor_ranges = layout let cursor_ranges = layout
.cursors .cursors
.iter() .iter()
.map(|cursor| { .map(|(point, color)| ColoredRange {
let point = cursor
.0
.to_display_point(&layout.position_map.snapshot.display_snapshot);
ColoredRange {
start: point.row(), start: point.row(),
end: point.row(), end: point.row(),
color: cursor.1, color: *color,
}
}) })
.collect_vec(); .collect_vec();
scrollbar_layout.marker_quads_for_ranges(cursor_ranges, None) scrollbar_layout.marker_quads_for_ranges(cursor_ranges, None)
@ -3788,8 +3782,12 @@ impl Element for EditorElement {
}); });
let cursors = self.collect_cursors(&snapshot, cx); let cursors = self.collect_cursors(&snapshot, cx);
let visible_row_range = start_row..end_row;
let non_visible_cursors = cursors
.iter()
.any(move |c| !visible_row_range.contains(&c.0.row()));
let (visible_cursors, non_visible_cursors) = self.layout_visible_cursors( let visible_cursors = self.layout_visible_cursors(
&snapshot, &snapshot,
&selections, &selections,
start_row..end_row, start_row..end_row,
@ -4044,7 +4042,7 @@ pub struct EditorLayout {
blocks: Vec<BlockLayout>, blocks: Vec<BlockLayout>,
highlighted_ranges: Vec<(Range<DisplayPoint>, Hsla)>, highlighted_ranges: Vec<(Range<DisplayPoint>, Hsla)>,
redacted_ranges: Vec<Range<DisplayPoint>>, redacted_ranges: Vec<Range<DisplayPoint>>,
cursors: Vec<(Anchor, Hsla)>, cursors: Vec<(DisplayPoint, Hsla)>,
visible_cursors: Vec<CursorLayout>, visible_cursors: Vec<CursorLayout>,
selections: Vec<(PlayerColor, Vec<SelectionLayout>)>, selections: Vec<(PlayerColor, Vec<SelectionLayout>)>,
max_row: u32, max_row: u32,