mirror of
https://github.com/zed-industries/zed.git
synced 2026-05-31 19:05:00 +07:00
agent_ui: Fix expanded message editor jitters while typing (#52612)
Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #52132. *This PR previously also fixes the "expanded message editor not taking up full height" part, but #52545 has already fixed that. Yet it seems to leave a new issue (that was previously not revealed) behind, as follows.* This PR fixes the `Full + ExcludeOverscrollMargin` editor mode (which the agent panel message editor uses in expanded mode), which could jitter while typing because render-time layout and scroll-position updates were clamping against different effective `scroll_beyond_last_line` policies. This PR fixes that inconsistency so the expanded editor stays stable while typing, and adds a regression test covering `ExcludeOverscrollMargin` scroll clamping. https://github.com/user-attachments/assets/86abf04d-c1a1-419b-96d0-8ca097c0acb0 https://github.com/user-attachments/assets/03dbdc3c-f58e-4378-8c6a-4bda1ae425c8 Release Notes: - Fixed the expanded Agent Panel message editor so it no longer jitters while typing. --------- Co-authored-by: MrSubidubi <finn@zed.dev>
This commit is contained in:
parent
501e72dbe7
commit
7f7520b98e
3 changed files with 86 additions and 26 deletions
|
|
@ -49,7 +49,8 @@ use serde_json::{self, json};
|
|||
use settings::{
|
||||
AllLanguageSettingsContent, DelayMs, EditorSettingsContent, GlobalLspSettingsContent,
|
||||
IndentGuideBackgroundColoring, IndentGuideColoring, InlayHintSettingsContent,
|
||||
ProjectSettingsContent, SearchSettingsContent, SettingsContent, SettingsStore,
|
||||
ProjectSettingsContent, ScrollBeyondLastLine, SearchSettingsContent, SettingsContent,
|
||||
SettingsStore,
|
||||
};
|
||||
use std::borrow::Cow;
|
||||
use std::{cell::RefCell, future::Future, rc::Rc, sync::atomic::AtomicBool, time::Instant};
|
||||
|
|
@ -2784,6 +2785,60 @@ async fn test_autoscroll(cx: &mut TestAppContext) {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_exclude_overscroll_margin_clamps_scroll_position(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
update_test_editor_settings(cx, &|settings| {
|
||||
settings.scroll_beyond_last_line = Some(ScrollBeyondLastLine::OnePage);
|
||||
});
|
||||
|
||||
let mut cx = EditorTestContext::new(cx).await;
|
||||
|
||||
let line_height = cx.update_editor(|editor, window, cx| {
|
||||
editor.set_mode(EditorMode::Full {
|
||||
scale_ui_elements_with_buffer_font_size: false,
|
||||
show_active_line_background: false,
|
||||
sizing_behavior: SizingBehavior::ExcludeOverscrollMargin,
|
||||
});
|
||||
editor
|
||||
.style(cx)
|
||||
.text
|
||||
.line_height_in_pixels(window.rem_size())
|
||||
});
|
||||
let window = cx.window;
|
||||
cx.simulate_window_resize(window, size(px(1000.), 6. * line_height));
|
||||
cx.set_state(
|
||||
&r#"
|
||||
ˇone
|
||||
two
|
||||
three
|
||||
four
|
||||
five
|
||||
six
|
||||
seven
|
||||
eight
|
||||
nine
|
||||
ten
|
||||
eleven
|
||||
"#
|
||||
.unindent(),
|
||||
);
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
let snapshot = editor.snapshot(window, cx);
|
||||
let max_scroll_top =
|
||||
(snapshot.max_point().row().as_f64() - editor.visible_line_count().unwrap() + 1.)
|
||||
.max(0.);
|
||||
|
||||
editor.set_scroll_position(gpui::Point::new(0., max_scroll_top + 10.), window, cx);
|
||||
|
||||
assert_eq!(
|
||||
editor.snapshot(window, cx).scroll_position(),
|
||||
gpui::Point::new(0., max_scroll_top)
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_move_page_up_page_down(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
|
|
|||
|
|
@ -9772,26 +9772,14 @@ impl Element for EditorElement {
|
|||
f64::from(visible_bounds.size.height / line_height);
|
||||
|
||||
// The max scroll position for the top of the window
|
||||
let max_scroll_top = if matches!(
|
||||
snapshot.mode,
|
||||
EditorMode::SingleLine
|
||||
| EditorMode::AutoHeight { .. }
|
||||
| EditorMode::Full {
|
||||
sizing_behavior: SizingBehavior::ExcludeOverscrollMargin
|
||||
| SizingBehavior::SizeByContent,
|
||||
..
|
||||
}
|
||||
) {
|
||||
(max_row - height_in_lines + 1.).max(0.)
|
||||
} else {
|
||||
let settings = EditorSettings::get_global(cx);
|
||||
match settings.scroll_beyond_last_line {
|
||||
ScrollBeyondLastLine::OnePage => max_row,
|
||||
ScrollBeyondLastLine::Off => (max_row - height_in_lines + 1.).max(0.),
|
||||
ScrollBeyondLastLine::VerticalScrollMargin => {
|
||||
(max_row - height_in_lines + 1. + settings.vertical_scroll_margin)
|
||||
.max(0.)
|
||||
}
|
||||
let scroll_beyond_last_line = self.editor.read(cx).scroll_beyond_last_line(cx);
|
||||
let max_scroll_top = match scroll_beyond_last_line {
|
||||
ScrollBeyondLastLine::OnePage => max_row,
|
||||
ScrollBeyondLastLine::Off => (max_row - height_in_lines + 1.).max(0.),
|
||||
ScrollBeyondLastLine::VerticalScrollMargin => {
|
||||
let settings = EditorSettings::get_global(cx);
|
||||
(max_row - height_in_lines + 1. + settings.vertical_scroll_margin)
|
||||
.max(0.)
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -10309,6 +10297,7 @@ impl Element for EditorElement {
|
|||
),
|
||||
longest_line_blame_width,
|
||||
EditorSettings::get_global(cx),
|
||||
scroll_beyond_last_line,
|
||||
);
|
||||
|
||||
let mut scroll_width = scrollbar_layout_information.scroll_range.width;
|
||||
|
|
@ -11187,8 +11176,9 @@ impl ScrollbarLayoutInformation {
|
|||
document_size: Size<Pixels>,
|
||||
longest_line_blame_width: Pixels,
|
||||
settings: &EditorSettings,
|
||||
scroll_beyond_last_line: ScrollBeyondLastLine,
|
||||
) -> Self {
|
||||
let vertical_overscroll = match settings.scroll_beyond_last_line {
|
||||
let vertical_overscroll = match scroll_beyond_last_line {
|
||||
ScrollBeyondLastLine::OnePage => editor_bounds.size.height,
|
||||
ScrollBeyondLastLine::Off => glyph_grid_cell.height,
|
||||
ScrollBeyondLastLine::VerticalScrollMargin => {
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ pub(crate) mod scroll_amount;
|
|||
use crate::editor_settings::ScrollBeyondLastLine;
|
||||
use crate::{
|
||||
Anchor, DisplayPoint, DisplayRow, Editor, EditorEvent, EditorMode, EditorSettings,
|
||||
InlayHintRefreshReason, MultiBufferSnapshot, RowExt, ToPoint,
|
||||
InlayHintRefreshReason, MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint,
|
||||
display_map::{DisplaySnapshot, ToDisplayPoint},
|
||||
hover_popover::hide_hover,
|
||||
persistence::EditorDb,
|
||||
|
|
@ -372,6 +372,7 @@ impl ScrollManager {
|
|||
&mut self,
|
||||
scroll_position: gpui::Point<ScrollOffset>,
|
||||
map: &DisplaySnapshot,
|
||||
scroll_beyond_last_line: ScrollBeyondLastLine,
|
||||
local: bool,
|
||||
autoscroll: bool,
|
||||
workspace_id: Option<WorkspaceId>,
|
||||
|
|
@ -379,7 +380,7 @@ impl ScrollManager {
|
|||
cx: &mut Context<Editor>,
|
||||
) -> WasScrolled {
|
||||
let scroll_top = scroll_position.y.max(0.);
|
||||
let scroll_top = match EditorSettings::get_global(cx).scroll_beyond_last_line {
|
||||
let scroll_top = match scroll_beyond_last_line {
|
||||
ScrollBeyondLastLine::OnePage => scroll_top,
|
||||
ScrollBeyondLastLine::Off => {
|
||||
if let Some(height_in_lines) = self.visible_line_count {
|
||||
|
|
@ -400,7 +401,6 @@ impl ScrollManager {
|
|||
}
|
||||
}
|
||||
};
|
||||
|
||||
let scroll_top_row = DisplayRow(scroll_top as u32);
|
||||
let scroll_top_buffer_point = map
|
||||
.clip_point(
|
||||
|
|
@ -639,6 +639,20 @@ impl Editor {
|
|||
self.scroll_manager.vertical_scroll_margin as usize
|
||||
}
|
||||
|
||||
pub(crate) fn scroll_beyond_last_line(&self, cx: &App) -> ScrollBeyondLastLine {
|
||||
match self.mode {
|
||||
EditorMode::Minimap { .. }
|
||||
| EditorMode::Full {
|
||||
sizing_behavior: SizingBehavior::Default,
|
||||
..
|
||||
} => EditorSettings::get_global(cx).scroll_beyond_last_line,
|
||||
|
||||
EditorMode::Full { .. } | EditorMode::SingleLine | EditorMode::AutoHeight { .. } => {
|
||||
ScrollBeyondLastLine::Off
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_vertical_scroll_margin(&mut self, margin_rows: usize, cx: &mut Context<Self>) {
|
||||
self.scroll_manager.vertical_scroll_margin = margin_rows as f64;
|
||||
cx.notify();
|
||||
|
|
@ -776,10 +790,11 @@ impl Editor {
|
|||
} else {
|
||||
scroll_position
|
||||
};
|
||||
|
||||
let scroll_beyond_last_line = self.scroll_beyond_last_line(cx);
|
||||
self.scroll_manager.set_scroll_position(
|
||||
adjusted_position,
|
||||
&display_map,
|
||||
scroll_beyond_last_line,
|
||||
local,
|
||||
autoscroll,
|
||||
workspace_id,
|
||||
|
|
|
|||
Loading…
Reference in a new issue