From 083f06322d10b0187da39ab1de0fe0ca4e4f94cd Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 7 Nov 2024 14:36:29 +0200 Subject: [PATCH] Prefer revealing items in the middle of the list for outline and project panels (#20349) Closes https://github.com/zed-industries/zed/issues/18255 Zed does not scroll always, but only if the item is out of sight, this is preserved for now. Otherwise, if the item is out of sight, project and outline panels + the syntax tree view now attempt to scroll it into the middle, if there's enough elements above and below. Release Notes: - Improved revealing items for outline and project panels (now center of the list is preferred) --- crates/editor/src/editor.rs | 28 +++++++----- crates/gpui/src/elements/uniform_list.rs | 45 ++++++++++++++++--- crates/language_tools/src/syntax_tree_view.rs | 7 +-- crates/outline_panel/src/outline_panel.rs | 5 ++- crates/picker/src/picker.rs | 8 ++-- crates/project_panel/src/project_panel.rs | 9 ++-- 6 files changed, 75 insertions(+), 27 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 345f436958b688da081c79510150db78713a0c22..adc4c96f986a0355fd1e6abfe52b0ae1acb783ee 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -76,8 +76,8 @@ use gpui::{ ClipboardItem, Context, DispatchPhase, ElementId, EventEmitter, FocusHandle, FocusOutEvent, FocusableView, FontId, FontWeight, HighlightStyle, Hsla, InteractiveText, KeyContext, ListSizingBehavior, Model, ModelContext, MouseButton, PaintQuad, ParentElement, Pixels, Render, - SharedString, Size, StrikethroughStyle, Styled, StyledText, Subscription, Task, TextStyle, - TextStyleRefinement, UTF16Selection, UnderlineStyle, UniformListScrollHandle, View, + ScrollStrategy, SharedString, Size, StrikethroughStyle, Styled, StyledText, Subscription, Task, + TextStyle, TextStyleRefinement, UTF16Selection, UnderlineStyle, UniformListScrollHandle, View, ViewContext, ViewInputHandler, VisualContext, WeakFocusHandle, WeakView, WindowContext, }; use highlight_matching_bracket::refresh_matching_bracket_highlights; @@ -1016,7 +1016,8 @@ impl CompletionsMenu { cx: &mut ViewContext, ) { self.selected_item = 0; - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); self.attempt_resolve_selected_completion_documentation(provider, cx); cx.notify(); } @@ -1031,7 +1032,8 @@ impl CompletionsMenu { } else { self.selected_item = self.matches.len() - 1; } - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); self.attempt_resolve_selected_completion_documentation(provider, cx); cx.notify(); } @@ -1046,7 +1048,8 @@ impl CompletionsMenu { } else { self.selected_item = 0; } - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); self.attempt_resolve_selected_completion_documentation(provider, cx); cx.notify(); } @@ -1057,7 +1060,8 @@ impl CompletionsMenu { cx: &mut ViewContext, ) { self.selected_item = self.matches.len() - 1; - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); self.attempt_resolve_selected_completion_documentation(provider, cx); cx.notify(); } @@ -1538,7 +1542,8 @@ struct CodeActionsMenu { impl CodeActionsMenu { fn select_first(&mut self, cx: &mut ViewContext) { self.selected_item = 0; - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); cx.notify() } @@ -1548,7 +1553,8 @@ impl CodeActionsMenu { } else { self.selected_item = self.actions.len() - 1; } - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); cx.notify(); } @@ -1558,13 +1564,15 @@ impl CodeActionsMenu { } else { self.selected_item = 0; } - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); cx.notify(); } fn select_last(&mut self, cx: &mut ViewContext) { self.selected_item = self.actions.len() - 1; - self.scroll_handle.scroll_to_item(self.selected_item); + self.scroll_handle + .scroll_to_item(self.selected_item, ScrollStrategy::Top); cx.notify() } diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 2379ee9f8123e726ffb01e77858bb4feea5e92ea..703d9bebe64e5614ed1f0ddde3a148138418eedd 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -88,11 +88,22 @@ pub struct UniformListFrameState { #[derive(Clone, Debug, Default)] pub struct UniformListScrollHandle(pub Rc>); +/// Where to place the element scrolled to. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ScrollStrategy { + /// Place the element at the top of the list's viewport. + Top, + /// Attempt to place the element in the middle of the list's viewport. + /// May not be possible if there's not enough list items above the item scrolled to: + /// in this case, the element will be placed at the closest possible position. + Center, +} + #[derive(Clone, Debug, Default)] #[allow(missing_docs)] pub struct UniformListScrollState { pub base_handle: ScrollHandle, - pub deferred_scroll_to_item: Option, + pub deferred_scroll_to_item: Option<(usize, ScrollStrategy)>, /// Size of the item, captured during last layout. pub last_item_size: Option, } @@ -118,14 +129,16 @@ impl UniformListScrollHandle { } /// Scroll the list to the given item index. - pub fn scroll_to_item(&self, ix: usize) { - self.0.borrow_mut().deferred_scroll_to_item = Some(ix); + pub fn scroll_to_item(&self, ix: usize, strategy: ScrollStrategy) { + self.0.borrow_mut().deferred_scroll_to_item = Some((ix, strategy)); } /// Get the index of the topmost visible child. + #[cfg(any(test, feature = "test-support"))] pub fn logical_scroll_top_index(&self) -> usize { let this = self.0.borrow(); this.deferred_scroll_to_item + .map(|(ix, _)| ix) .unwrap_or_else(|| this.base_handle.logical_scroll_top().0) } } @@ -273,18 +286,40 @@ impl Element for UniformList { scroll_offset.x = Pixels::ZERO; } - if let Some(ix) = shared_scroll_to_item { + if let Some((ix, scroll_strategy)) = shared_scroll_to_item { let list_height = padded_bounds.size.height; let mut updated_scroll_offset = shared_scroll_offset.borrow_mut(); let item_top = item_height * ix + padding.top; let item_bottom = item_top + item_height; let scroll_top = -updated_scroll_offset.y; + let mut scrolled_to_top = false; if item_top < scroll_top + padding.top { + scrolled_to_top = true; updated_scroll_offset.y = -(item_top) + padding.top; } else if item_bottom > scroll_top + list_height - padding.bottom { + scrolled_to_top = true; updated_scroll_offset.y = -(item_bottom - list_height) - padding.bottom; } - scroll_offset = *updated_scroll_offset; + + match scroll_strategy { + ScrollStrategy::Top => {} + ScrollStrategy::Center => { + if scrolled_to_top { + let item_center = item_top + item_height / 2.0; + let target_scroll_top = item_center - list_height / 2.0; + + if item_top < scroll_top + || item_bottom > scroll_top + list_height + { + updated_scroll_offset.y = -target_scroll_top + .max(Pixels::ZERO) + .min(content_height - list_height) + .max(Pixels::ZERO); + } + } + } + } + scroll_offset = *updated_scroll_offset } let first_visible_element_ix = diff --git a/crates/language_tools/src/syntax_tree_view.rs b/crates/language_tools/src/syntax_tree_view.rs index b9c960c9c3dfa4788eafdbaebde35f712a063acc..a0eb479e6d15521f7c2a7be4ad902d236ff06ca6 100644 --- a/crates/language_tools/src/syntax_tree_view.rs +++ b/crates/language_tools/src/syntax_tree_view.rs @@ -2,8 +2,8 @@ use editor::{scroll::Autoscroll, Anchor, Editor, ExcerptId}; use gpui::{ actions, div, rems, uniform_list, AppContext, Div, EventEmitter, FocusHandle, FocusableView, Hsla, InteractiveElement, IntoElement, Model, MouseButton, MouseDownEvent, MouseMoveEvent, - ParentElement, Render, SharedString, Styled, UniformListScrollHandle, View, ViewContext, - VisualContext, WeakView, WindowContext, + ParentElement, Render, ScrollStrategy, SharedString, Styled, UniformListScrollHandle, View, + ViewContext, VisualContext, WeakView, WindowContext, }; use language::{Buffer, OwnedSyntaxLayer}; use std::{mem, ops::Range}; @@ -199,7 +199,8 @@ impl SyntaxTreeView { let descendant_ix = cursor.descendant_index(); self.selected_descendant_ix = Some(descendant_ix); - self.list_scroll_handle.scroll_to_item(descendant_ix); + self.list_scroll_handle + .scroll_to_item(descendant_ix, ScrollStrategy::Center); cx.notify(); Some(()) diff --git a/crates/outline_panel/src/outline_panel.rs b/crates/outline_panel/src/outline_panel.rs index 4b75a03b1a4575cbb9c737b7744754042ed8a8b1..350519906f775e7a77f3d40d773c3c4d31640ba6 100644 --- a/crates/outline_panel/src/outline_panel.rs +++ b/crates/outline_panel/src/outline_panel.rs @@ -27,7 +27,7 @@ use gpui::{ AnyElement, AppContext, AssetSource, AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div, ElementId, EventEmitter, FocusHandle, FocusableView, HighlightStyle, InteractiveElement, IntoElement, KeyContext, ListHorizontalSizingBehavior, ListSizingBehavior, Model, MouseButton, - MouseDownEvent, ParentElement, Pixels, Point, Render, SharedString, Stateful, + MouseDownEvent, ParentElement, Pixels, Point, Render, ScrollStrategy, SharedString, Stateful, StatefulInteractiveElement as _, Styled, Subscription, Task, UniformListScrollHandle, View, ViewContext, VisualContext, WeakView, WindowContext, }; @@ -1078,7 +1078,8 @@ impl OutlinePanel { .iter() .position(|cached_entry| &cached_entry.entry == selected_entry); if let Some(index) = index { - self.scroll_handle.scroll_to_item(index); + self.scroll_handle + .scroll_to_item(index, ScrollStrategy::Center); cx.notify(); } } diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 5ebbcd3330333e4ba7dc549051639c03127d1d24..119c412b48ccefe57bdc7228cf604ed6ba84c0c1 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -3,8 +3,8 @@ use editor::{scroll::Autoscroll, Editor}; use gpui::{ actions, div, impl_actions, list, prelude::*, uniform_list, AnyElement, AppContext, ClickEvent, DismissEvent, EventEmitter, FocusHandle, FocusableView, Length, ListSizingBehavior, ListState, - MouseButton, MouseUpEvent, Render, Task, UniformListScrollHandle, View, ViewContext, - WindowContext, + MouseButton, MouseUpEvent, Render, ScrollStrategy, Task, UniformListScrollHandle, View, + ViewContext, WindowContext, }; use head::Head; use serde::Deserialize; @@ -495,7 +495,9 @@ impl Picker { fn scroll_to_item_index(&mut self, ix: usize) { match &mut self.element_container { ElementContainer::List(state) => state.scroll_to_reveal_item(ix), - ElementContainer::UniformList(scroll_handle) => scroll_handle.scroll_to_item(ix), + ElementContainer::UniformList(scroll_handle) => { + scroll_handle.scroll_to_item(ix, ScrollStrategy::Top) + } } } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index a7d826076c026e24ee5deea7b5e6ade88f45fc71..27532a5d98800889153b47af7bb3d40949653771 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -20,9 +20,9 @@ use gpui::{ AnyElement, AppContext, AssetSource, AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div, DragMoveEvent, EventEmitter, ExternalPaths, FocusHandle, FocusableView, InteractiveElement, KeyContext, ListHorizontalSizingBehavior, ListSizingBehavior, Model, - MouseButton, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, Stateful, - Styled, Subscription, Task, UniformListScrollHandle, View, ViewContext, VisualContext as _, - WeakView, WindowContext, + MouseButton, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, ScrollStrategy, + Stateful, Styled, Subscription, Task, UniformListScrollHandle, View, ViewContext, + VisualContext as _, WeakView, WindowContext, }; use indexmap::IndexMap; use menu::{Confirm, SelectFirst, SelectLast, SelectNext, SelectPrev}; @@ -1356,7 +1356,8 @@ impl ProjectPanel { fn autoscroll(&mut self, cx: &mut ViewContext) { if let Some((_, _, index)) = self.selection.and_then(|s| self.index_for_selection(s)) { - self.scroll_handle.scroll_to_item(index); + self.scroll_handle + .scroll_to_item(index, ScrollStrategy::Center); cx.notify(); } }