From dae5fcd8aa59ae772e355174ac216ddaf72b08a8 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Mon, 24 Nov 2025 17:37:59 +0800 Subject: [PATCH] editor: Improve scroll to support position at the edge of the editor on click. (#1672) But if we use `Up`, `Down` to move the cursor, it still will keep 3 lines. https://github.com/user-attachments/assets/4310a4cc-3829-4190-9be9-e7159c688884 --- crates/ui/src/input/lsp/definitions.rs | 6 +-- crates/ui/src/input/movement.rs | 44 ++++++++++++++------ crates/ui/src/input/popovers/context_menu.rs | 12 +++--- crates/ui/src/input/search.rs | 22 ++++++---- crates/ui/src/input/state.rs | 33 +++++++++++---- 5 files changed, 78 insertions(+), 39 deletions(-) diff --git a/crates/ui/src/input/lsp/definitions.rs b/crates/ui/src/input/lsp/definitions.rs index 0a519529..9536603c 100644 --- a/crates/ui/src/input/lsp/definitions.rs +++ b/crates/ui/src/input/lsp/definitions.rs @@ -1,13 +1,13 @@ use anyhow::Result; use gpui::{ - px, App, Context, HighlightStyle, Hitbox, MouseDownEvent, Task, UnderlineStyle, Window, + App, Context, HighlightStyle, Hitbox, MouseDownEvent, Task, UnderlineStyle, Window, px, }; use ropey::Rope; use std::{ops::Range, rc::Rc}; use crate::{ - input::{element::TextElement, GoToDefinition, InputState, RopeExt}, ActiveTheme, + input::{GoToDefinition, InputState, RopeExt, element::TextElement}, }; /// Definition provider @@ -172,7 +172,7 @@ impl InputState { let start = self.text.position_to_offset(&target_range.start); let end = self.text.position_to_offset(&target_range.end); - self.move_to(start, cx); + self.move_to(start, None, cx); self.select_to(end, cx); } } diff --git a/crates/ui/src/input/movement.rs b/crates/ui/src/input/movement.rs index 27dd01fc..76eb969d 100644 --- a/crates/ui/src/input/movement.rs +++ b/crates/ui/src/input/movement.rs @@ -5,6 +5,12 @@ use crate::input::{ MoveToEnd, MoveToNextWord, MoveToPreviousWord, MoveToStart, MoveUp, RopeExt as _, }; +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum MoveDirection { + Up, + Down, +} + impl InputState { /// Called after moving the cursor. Updates preferred_column if we know where the cursor now is. pub(super) fn update_preferred_column(&mut self) { @@ -33,10 +39,15 @@ impl InputState { /// The offset is the UTF-8 offset. /// /// Ensure the offset use self.next_boundary or self.previous_boundary to get the correct offset. - pub(crate) fn move_to(&mut self, offset: usize, cx: &mut Context) { + pub(crate) fn move_to( + &mut self, + offset: usize, + direction: Option, + cx: &mut Context, + ) { let offset = offset.clamp(0, self.text.len()); self.selected_range = (offset..offset).into(); - self.scroll_to(offset, cx); + self.scroll_to(offset, direction, cx); self.pause_blink_cursor(cx); self.update_preferred_column(); self.hide_context_menu(cx); @@ -93,7 +104,12 @@ impl InputState { } self.pause_blink_cursor(cx); - self.move_to(new_offset, cx); + let direction = if move_lines < 0 { + MoveDirection::Up + } else { + MoveDirection::Down + }; + self.move_to(new_offset, Some(direction), cx); // Set back the preferred_column self.preferred_column = was_preferred_column; cx.notify(); @@ -102,18 +118,18 @@ impl InputState { pub(super) fn left(&mut self, _: &MoveLeft, _: &mut Window, cx: &mut Context) { self.pause_blink_cursor(cx); if self.selected_range.is_empty() { - self.move_to(self.previous_boundary(self.cursor()), cx); + self.move_to(self.previous_boundary(self.cursor()), None, cx); } else { - self.move_to(self.selected_range.start, cx) + self.move_to(self.selected_range.start, None, cx) } } pub(super) fn right(&mut self, _: &MoveRight, _: &mut Window, cx: &mut Context) { self.pause_blink_cursor(cx); if self.selected_range.is_empty() { - self.move_to(self.next_boundary(self.selected_range.end), cx); + self.move_to(self.next_boundary(self.selected_range.end), None, cx); } else { - self.move_to(self.selected_range.end, cx) + self.move_to(self.selected_range.end, None, cx) } } @@ -129,6 +145,7 @@ impl InputState { if !self.selected_range.is_empty() { self.move_to( self.previous_boundary(self.selected_range.start.saturating_sub(1)), + Some(MoveDirection::Up), cx, ); } @@ -148,6 +165,7 @@ impl InputState { if !self.selected_range.is_empty() { self.move_to( self.next_boundary(self.selected_range.end.saturating_sub(1)), + Some(MoveDirection::Down), cx, ); } @@ -190,13 +208,13 @@ impl InputState { pub(super) fn home(&mut self, _: &MoveHome, _: &mut Window, cx: &mut Context) { self.pause_blink_cursor(cx); let offset = self.start_of_line(); - self.move_to(offset, cx); + self.move_to(offset, Some(MoveDirection::Up), cx); } pub(super) fn end(&mut self, _: &MoveEnd, _: &mut Window, cx: &mut Context) { self.pause_blink_cursor(cx); let offset = self.end_of_line(); - self.move_to(offset, cx); + self.move_to(offset, Some(MoveDirection::Down), cx); } pub(super) fn move_to_start( @@ -205,11 +223,11 @@ impl InputState { _: &mut Window, cx: &mut Context, ) { - self.move_to(0, cx); + self.move_to(0, None, cx); } pub(super) fn move_to_end(&mut self, _: &MoveToEnd, _: &mut Window, cx: &mut Context) { - self.move_to(self.text.len(), cx); + self.move_to(self.text.len(), None, cx); } pub(super) fn move_to_previous_word( @@ -219,7 +237,7 @@ impl InputState { cx: &mut Context, ) { let offset = self.previous_start_of_word(); - self.move_to(offset, cx); + self.move_to(offset, None, cx); } pub(super) fn move_to_next_word( @@ -229,6 +247,6 @@ impl InputState { cx: &mut Context, ) { let offset = self.next_end_of_word(); - self.move_to(offset, cx); + self.move_to(offset, None, cx); } } diff --git a/crates/ui/src/input/popovers/context_menu.rs b/crates/ui/src/input/popovers/context_menu.rs index 72ce7fa5..dd9011b3 100644 --- a/crates/ui/src/input/popovers/context_menu.rs +++ b/crates/ui/src/input/popovers/context_menu.rs @@ -1,14 +1,14 @@ use gpui::{ - anchored, deferred, div, prelude::FluentBuilder as _, px, App, AppContext as _, Context, - Corner, DismissEvent, Entity, IntoElement, MouseDownEvent, ParentElement as _, Pixels, Point, - Render, Styled, Subscription, Window, + App, AppContext as _, Context, Corner, DismissEvent, Entity, IntoElement, MouseDownEvent, + ParentElement as _, Pixels, Point, Render, Styled, Subscription, Window, anchored, deferred, + div, prelude::FluentBuilder as _, px, }; use rust_i18n::t; use crate::{ - input::{self, popovers::ContextMenu, InputState}, - menu::PopupMenu, ActiveTheme as _, + input::{self, InputState, popovers::ContextMenu}, + menu::PopupMenu, }; /// Context menu for mouse right clicks. @@ -31,7 +31,7 @@ impl InputState { ) { // Show Mouse context menu if !self.selected_range.contains(offset) { - self.move_to(offset, cx); + self.move_to(offset, None, cx); } self.context_menu = Some(ContextMenu::MouseContext(self.mouse_context_menu.clone())); diff --git a/crates/ui/src/input/search.rs b/crates/ui/src/input/search.rs index 11ada752..531d419d 100644 --- a/crates/ui/src/input/search.rs +++ b/crates/ui/src/input/search.rs @@ -3,19 +3,23 @@ use rust_i18n::t; use std::{ops::Range, rc::Rc}; use gpui::{ - actions, canvas, div, prelude::FluentBuilder as _, App, AppContext as _, Context, Empty, - Entity, FocusHandle, Focusable, Half, InteractiveElement as _, IntoElement, KeyBinding, - ParentElement as _, Pixels, Render, Styled, Subscription, Window, + App, AppContext as _, Context, Empty, Entity, FocusHandle, Focusable, Half, + InteractiveElement as _, IntoElement, KeyBinding, ParentElement as _, Pixels, Render, Styled, + Subscription, Window, actions, canvas, div, prelude::FluentBuilder as _, }; use ropey::Rope; use crate::{ + ActiveTheme, Disableable, IconName, Selectable, Sizable, actions::SelectUp, button::{Button, ButtonVariants}, h_flex, - input::{Enter, Escape, IndentInline, Input, InputEvent, InputState, RopeExt as _, Search}, + input::{ + Enter, Escape, IndentInline, Input, InputEvent, InputState, RopeExt as _, Search, + movement::MoveDirection, + }, label::Label, - v_flex, ActiveTheme, Disableable, IconName, Selectable, Sizable, + v_flex, }; const CONTEXT: &'static str = "SearchPanel"; @@ -309,7 +313,7 @@ impl SearchPanel { fn prev(&mut self, _: &mut Window, cx: &mut Context) { if let Some(range) = self.matcher.next_back() { self.editor.update(cx, |state, cx| { - state.scroll_to(range.start, cx); + state.scroll_to(range.start, Some(MoveDirection::Up), cx); }); } } @@ -317,7 +321,7 @@ impl SearchPanel { fn next(&mut self, _: &mut Window, cx: &mut Context) { if let Some(range) = self.matcher.next() { self.editor.update(cx, |state, cx| { - state.scroll_to(range.end, cx); + state.scroll_to(range.end, Some(MoveDirection::Down), cx); }); } } @@ -346,7 +350,7 @@ impl SearchPanel { cx.update(|window, cx| { text_state.update(cx, |state, cx| { let range_utf16 = state.range_to_utf16(&range); - state.scroll_to(next_range.end, cx); + state.scroll_to(next_range.end, Some(MoveDirection::Down), cx); state.replace_text_in_range_silent( Some(range_utf16), new_text.as_str(), @@ -383,7 +387,7 @@ impl SearchPanel { window, cx, ); - state.scroll_to(0, cx); + state.scroll_to(0, Some(MoveDirection::Down), cx); }); }) }) diff --git a/crates/ui/src/input/state.rs b/crates/ui/src/input/state.rs index 80a3dacf..8254f164 100644 --- a/crates/ui/src/input/state.rs +++ b/crates/ui/src/input/state.rs @@ -23,6 +23,7 @@ use super::{ mask_pattern::MaskPattern, mode::InputMode, number_input, text_wrapper::TextWrapper, }; use crate::actions::{SelectDown, SelectLeft, SelectRight, SelectUp}; +use crate::input::movement::MoveDirection; use crate::input::{ HoverDefinition, Lsp, Position, element::RIGHT_MARGIN, @@ -825,7 +826,7 @@ impl InputState { let position: Position = position.into(); let offset = self.text.position_to_offset(&position); - self.move_to(offset, cx); + self.move_to(offset, None, cx); self.update_preferred_column(); self.focus(window, cx); } @@ -1178,7 +1179,7 @@ impl InputState { pub(super) fn clean(&mut self, window: &mut Window, cx: &mut Context) { self.replace_text("", window, cx); self.selected_range = (0..0).into(); - self.scroll_to(0, cx); + self.scroll_to(0, None, cx); } pub(super) fn escape(&mut self, action: &Escape, window: &mut Window, cx: &mut Context) { @@ -1233,7 +1234,7 @@ impl InputState { if event.modifiers.shift { self.select_to(offset, cx); } else { - self.move_to(offset, cx) + self.move_to(offset, None, cx) } } @@ -1331,7 +1332,15 @@ impl InputState { cx.notify(); } - pub(crate) fn scroll_to(&mut self, offset: usize, cx: &mut Context) { + /// Scroll to make the given offset visible. + /// + /// If `direction` is Some, will keep edges at the same side. + pub(crate) fn scroll_to( + &mut self, + offset: usize, + direction: Option, + cx: &mut Context, + ) { let Some(last_layout) = self.last_layout.as_ref() else { return; }; @@ -1340,6 +1349,7 @@ impl InputState { }; let mut scroll_offset = self.scroll_handle.offset(); + let was_offset = scroll_offset; let line_height = last_layout.line_height; let point = self.text.offset_to_point(offset); @@ -1375,19 +1385,26 @@ impl InputState { // Check if row_offset_y is out of the viewport // If row offset is not in the viewport, scroll to make it visible - let edge_height = if self.mode.is_code_editor() { + let edge_height = if direction.is_some() && self.mode.is_code_editor() { 3 * line_height } else { line_height }; - if row_offset_y - edge_height < -scroll_offset.y { + if row_offset_y - edge_height + line_height < -scroll_offset.y { // Scroll up - scroll_offset.y = -row_offset_y + edge_height; + scroll_offset.y = -row_offset_y + edge_height - line_height; } else if row_offset_y + edge_height > -scroll_offset.y + bounds.size.height { // Scroll down scroll_offset.y = -(row_offset_y - bounds.size.height + edge_height); } + // Avoid necessary scroll, when it was already in the correct position. + if direction == Some(MoveDirection::Up) { + scroll_offset.y = scroll_offset.y.max(was_offset.y); + } else if direction == Some(MoveDirection::Down) { + scroll_offset.y = scroll_offset.y.min(was_offset.y); + } + scroll_offset.x = scroll_offset.x.min(px(0.)); scroll_offset.y = scroll_offset.y.min(px(0.)); self.deferred_scroll_offset = Some(scroll_offset); @@ -1431,7 +1448,7 @@ impl InputState { } self.replace_text_in_range_silent(None, &new_text, window, cx); - self.scroll_to(self.cursor(), cx); + self.scroll_to(self.cursor(), None, cx); } }