From c10d18398a54e26d217c47c4e92be3fa0e50aa4b Mon Sep 17 00:00:00 2001 From: Sola Date: Thu, 8 Oct 2026 08:28:10 -0600 Subject: [PATCH 1/2] input: Anchor completion/code-action popups to the current frame's caret The popup position was resolved in render() from the previous frame's paint layout, so a completion menu opened on the keystroke's own frame stayed where the caret was until the next frame. The input now stashes fresh caret geometry (PrepaintCaretGeometry) during prepaint. Popups live inside deferred(), so their prepaint runs after the input's prepaint; a new CaretAnchoredPopup element resolves the anchor there via the shared caret_popup_anchor() and builds the popup content from it, keeping position, hitboxes, accessibility, and width decisions consistent with the resolved position. No follow-up frame is requested; ordinary inputs are unaffected. --- crates/base/src/input/base/element.rs | 35 ++++- crates/base/src/input/base/state.rs | 36 +++++ crates/component/src/input/mod.rs | 12 +- .../src/input/popovers/caret_anchored.rs | 129 ++++++++++++++++ .../src/input/popovers/code_action_menu.rs | 75 +++++---- .../src/input/popovers/completion_menu.rs | 142 +++++++++--------- crates/component/src/input/popovers/mod.rs | 2 + crates/kit/tests/input/completions.rs | 81 ++++++++++ 8 files changed, 391 insertions(+), 121 deletions(-) create mode 100644 crates/component/src/input/popovers/caret_anchored.rs diff --git a/crates/base/src/input/base/element.rs b/crates/base/src/input/base/element.rs index 6076b1a6df..f5a6a236e9 100644 --- a/crates/base/src/input/base/element.rs +++ b/crates/base/src/input/base/element.rs @@ -21,7 +21,7 @@ use crate::{ }; use super::{ - InputBaseState, RangeDecorationStyle, TextDecoration, + InputBaseState, PrepaintCaretGeometry, RangeDecorationStyle, TextDecoration, layout::{LastLayout, WhitespaceIndicators}, mode::LayoutMode, }; @@ -2862,6 +2862,39 @@ impl Element for TextElement { .find(|info| info.is_active) .map(|info| info.bounds); + // Stash the fresh caret geometry for caret-anchored popups. Their + // deferred prepaint runs after this prepaint but before paint, so + // they anchor to the current frame's caret instead of the previous + // frame's. Only the positioning geometry is stashed here; the rest + // of the paint-time state updates stay where they are. + let caret_geometry = last_layout.cursor_bounds.map(|cursor_bounds| { + let mut scroll_offset = cursor_scroll_offset; + // Mirror update_scroll_offset's clamping with the fresh values, + // so the stash matches what paint will publish. + let state = self.state.read(cx); + let safe_y_range = (-scroll_size.height + input_bounds.size.height).min(px(0.))..px(0.); + scroll_offset.y = if state.is_single_line() { + px(0.) + } else { + scroll_offset.y.clamp(safe_y_range.start, safe_y_range.end) + }; + scroll_offset.x = clamp_horizontal_scroll_offset( + scroll_offset.x, + scroll_size.width, + input_bounds.size.width, + last_layout.text_align, + ); + PrepaintCaretGeometry { + cursor_bounds, + line_height, + input_bounds, + scroll_offset, + } + }); + self.state.update(cx, |state, _| { + state.prepaint_caret_geometry = caret_geometry; + }); + let search_match_paths = self.layout_search_matches(&last_layout, &mut bounds, cx); let selection_paths = self.layout_selections(&last_layout, &mut bounds, window, cx); let hover_highlight_path = self.layout_hover_highlight(&last_layout, &mut bounds, cx); diff --git a/crates/base/src/input/base/state.rs b/crates/base/src/input/base/state.rs index ae7f425a10..4e55c30588 100644 --- a/crates/base/src/input/base/state.rs +++ b/crates/base/src/input/base/state.rs @@ -338,6 +338,26 @@ struct PasteTarget { selections: Vec, } +/// Caret positioning geometry from the input's latest prepaint. +/// +/// Unlike `last_layout` (published at paint time), this is stashed during +/// prepaint so caret-anchored popups can position themselves from the +/// current frame's geometry in their deferred prepaint, instead of the +/// previous frame's paint. +#[derive(Clone, Copy, Debug)] +#[doc(hidden)] +pub struct PrepaintCaretGeometry { + /// The active caret's bounds from the fresh prepaint layout. + pub cursor_bounds: Bounds, + /// Line height from the fresh prepaint layout. + pub line_height: Pixels, + /// The input container bounds from the fresh prepaint. + pub input_bounds: Bounds, + /// The scroll offset that will be in effect after paint (clamped like + /// `update_scroll_offset` clamps it). + pub scroll_offset: Point, +} + /// The shared text-editing engine behind [`crate::input::InputState`], /// [`crate::input::TextareaState`] and [`crate::input::EditorState`]. /// @@ -397,6 +417,12 @@ pub struct InputBaseState { /// The marked range is the temporary insert text on IME typing. pub(super) ime_marked_range: Option, pub(super) last_layout: Option, + /// Fresh caret geometry stashed during prepaint for caret-anchored popups. + /// + /// Popups are positioned in their deferred prepaint, which runs after the + /// input's prepaint but before its paint, so they read this instead of + /// the previous frame's `last_layout`. + pub(crate) prepaint_caret_geometry: Option, pub(super) last_cursor: Option, /// The input container bounds pub(super) input_bounds: Bounds, @@ -570,6 +596,15 @@ impl InputBaseState { Some((layout.cursor_bounds?, layout.line_height)) } + /// Caret geometry from the latest prepaint, for caret-anchored popups. + /// + /// Popups positioned in their deferred prepaint read this to anchor to + /// the current frame's caret instead of the previous frame's paint. + #[doc(hidden)] + pub fn prepaint_caret_geometry(&self) -> Option { + self.prepaint_caret_geometry + } + pub fn input_bounds(&self) -> Bounds { self.input_bounds } @@ -780,6 +815,7 @@ impl InputBaseState { number_max: None, mode: LayoutMode::default(), last_layout: None, + prepaint_caret_geometry: None, last_bounds: None, last_selected_range: None, column_select_start: None, diff --git a/crates/component/src/input/mod.rs b/crates/component/src/input/mod.rs index 4ef229f8f9..3f84757a8a 100644 --- a/crates/component/src/input/mod.rs +++ b/crates/component/src/input/mod.rs @@ -31,12 +31,12 @@ pub use gpui_base::input::{ IndentInline, InputEdit, InputEvent, InputHighlighter, InputHighlighterFactory, InputState, Lsp, MaskPattern, MoveDown, MoveEnd, MoveHome, MoveLeft, MovePageDown, MovePageUp, MoveRight, MoveToEnd, MoveToEndOfLine, MoveToNextWord, MoveToPreviousWord, MoveToStart, MoveToStartOfLine, - MoveUp, Outdent, OutdentInline, Paste, Point, RangeDecoration, RangeDecorationCollection, - RangeDecorationStyle, Redo, Replace, Rope, RopeExt, RopeLines, Search, SelectAll, SelectToEnd, - SelectToEndOfLine, SelectToNextWordEnd, SelectToPreviousWordStart, SelectToStart, - SelectToStartOfLine, Selection, ShowCharacterPalette, ShowCompletions, ShowDocumentHandler, - TabSize, TextDecoration, TextDecorationCollection, TextareaState, ToggleCodeActions, Undo, - WrappingIndent, + MoveUp, Outdent, OutdentInline, Paste, Point, PrepaintCaretGeometry, RangeDecoration, + RangeDecorationCollection, RangeDecorationStyle, Redo, Replace, Rope, RopeExt, RopeLines, + Search, SelectAll, SelectToEnd, SelectToEndOfLine, SelectToNextWordEnd, + SelectToPreviousWordStart, SelectToStart, SelectToStartOfLine, Selection, ShowCharacterPalette, + ShowCompletions, ShowDocumentHandler, TabSize, TextDecoration, TextDecorationCollection, + TextareaState, ToggleCodeActions, Undo, WrappingIndent, }; pub use gpui_base::input::{EditorMode, InputMode, InputModeKind, TextareaMode}; #[doc(hidden)] diff --git a/crates/component/src/input/popovers/caret_anchored.rs b/crates/component/src/input/popovers/caret_anchored.rs new file mode 100644 index 0000000000..c4ffbff0ff --- /dev/null +++ b/crates/component/src/input/popovers/caret_anchored.rs @@ -0,0 +1,129 @@ +//! Caret-anchored popups positioned from the input's current-frame geometry. +//! +//! The input stashes fresh caret geometry in its prepaint +//! ([`PrepaintCaretGeometry`]). Popups live inside `deferred()`, so their +//! prepaint runs after the input's prepaint but before its paint; resolving +//! the anchor there puts the popup at the current frame's caret instead of +//! the previous frame's. + +use gpui::{ + AnyElement, App, AvailableSpace, Bounds, Element, ElementId, GlobalElementId, + InspectorElementId, IntoElement, LayoutId, Pixels, Point, Size, Style, WeakEntity, Window, px, +}; + +use crate::input::{EditorState, PrepaintCaretGeometry}; + +/// Computes the caret-anchored popup origin from fresh prepaint geometry. +/// +/// Shared by the completion and code-action menus so both anchor to the +/// same point: just below the caret, nudged left. The result is in window +/// coordinates, matching where the caret itself is painted. +pub(crate) fn caret_popup_anchor(geometry: PrepaintCaretGeometry) -> Point { + Point::new( + geometry.cursor_bounds.origin.x - px(4.), + geometry.cursor_bounds.origin.y + geometry.scroll_offset.y + geometry.line_height + px(4.), + ) +} + +/// A popup positioned at the caret from the input's current-frame prepaint. +/// +/// The content is built in prepaint (not render) from the resolved anchor, +/// so position, hitboxes, accessibility bounds, and width/layout decisions +/// are all consistent with the current frame's caret. Always use inside +/// `deferred()` so the prepaint runs after the input's prepaint. +pub(crate) struct CaretAnchoredPopup { + editor: WeakEntity, + build: Box, &mut Window, &mut App) -> AnyElement>, + content: Option, +} + +impl CaretAnchoredPopup { + pub(crate) fn new( + editor: WeakEntity, + build: impl Fn(Point, &mut Window, &mut App) -> AnyElement + 'static, + ) -> Self { + Self { + editor, + build: Box::new(build), + content: None, + } + } +} + +impl Element for CaretAnchoredPopup { + type RequestLayoutState = (); + type PrepaintState = (); + + fn id(&self) -> Option { + None + } + + fn source_location(&self) -> Option<&'static core::panic::Location<'static>> { + None + } + + fn request_layout( + &mut self, + _id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + // The real layout happens in prepaint, once the anchor is resolved. + let layout_id = window.request_layout(Style::default(), [], cx); + (layout_id, ()) + } + + fn prepaint( + &mut self, + _id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + _bounds: Bounds, + _request_layout: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) { + let Some(editor) = self.editor.upgrade() else { + return; + }; + let Some(geometry) = editor.read(cx).prepaint_caret_geometry() else { + return; + }; + let anchor = caret_popup_anchor(geometry); + let mut content = (self.build)(anchor, window, cx); + let viewport = window.viewport_size(); + content.layout_as_root( + Size { + width: AvailableSpace::Definite(viewport.width), + height: AvailableSpace::Definite(viewport.height), + }, + window, + cx, + ); + content.prepaint_at(anchor, window, cx); + self.content = Some(content); + } + + fn paint( + &mut self, + _id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + _bounds: Bounds, + _request_layout: &mut Self::RequestLayoutState, + _prepaint: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + if let Some(content) = self.content.as_mut() { + content.paint(window, cx); + } + } +} + +impl IntoElement for CaretAnchoredPopup { + type Element = Self; + + fn into_element(self) -> Self::Element { + self + } +} diff --git a/crates/component/src/input/popovers/code_action_menu.rs b/crates/component/src/input/popovers/code_action_menu.rs index 0f2220fc15..521a5bc65d 100644 --- a/crates/component/src/input/popovers/code_action_menu.rs +++ b/crates/component/src/input/popovers/code_action_menu.rs @@ -2,9 +2,9 @@ use std::rc::Rc; use gpui::{ Action, AnyElement, App, AppContext, Context, DismissEvent, Empty, Entity, EventEmitter, - Half as _, InteractiveElement as _, IntoElement, ParentElement, Pixels, Point, Render, - RenderOnce, Styled, StyledText, Subscription, WeakEntity, Window, deferred, div, - prelude::FluentBuilder, px, relative, + Half as _, InteractiveElement as _, IntoElement, ParentElement, Pixels, Render, RenderOnce, + Styled, StyledText, Subscription, WeakEntity, Window, deferred, div, prelude::FluentBuilder, + px, relative, }; pub(crate) use gpui_base::input::CodeActionItem; @@ -13,7 +13,10 @@ const MAX_MENU_HEIGHT: Pixels = px(480.); use crate::{ ActiveTheme, IndexPath, Selectable, actions, h_flex, - input::{self, EditorState, popovers::editor_popover}, + input::{ + self, EditorState, + popovers::{CaretAnchoredPopup, editor_popover}, + }, list::{List, ListDelegate, ListEvent, ListState}, }; @@ -278,26 +281,10 @@ impl CodeActionMenu { cx.notify(); } - - fn origin(&self, cx: &App) -> Option> { - let state = self.state.upgrade()?; - let state = state.read(cx); - let Some((cursor_bounds, line_height)) = state.cursor_layout() else { - return None; - }; - let cursor_origin = cursor_bounds.origin; - - let scroll_origin = state.scroll_offset(); - - Some( - scroll_origin + cursor_origin - state.input_bounds().origin - + Point::new(-px(4.), line_height + px(4.)), - ) - } } impl Render for CodeActionMenu { - fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { if !self.open { return Empty.into_any_element(); } @@ -307,24 +294,34 @@ impl Render for CodeActionMenu { return Empty.into_any_element(); } - let Some(pos) = self.origin(cx) else { - return Empty.into_any_element(); - }; - - let max_width = MAX_MENU_WIDTH.min(window.bounds().size.width - pos.x); - - deferred( - editor_popover("code-action-menu", cx) - .absolute() - .left(pos.x) - .top(pos.y) - .max_w(max_width) - .min_w(px(120.)) - .child(List::new(&self.list).max_h(MAX_MENU_HEIGHT)) - .on_mouse_down_out(cx.listener(|this, _, _, cx| { - this.hide(cx); - })), - ) + // The anchor is resolved in the popup's deferred prepaint from the + // input's current-frame geometry; the content below is built there. + let list = self.list.clone(); + let menu = cx.weak_entity(); + + deferred(CaretAnchoredPopup::new( + self.state.clone(), + move |anchor, window, cx| { + let max_width = MAX_MENU_WIDTH.min(window.bounds().size.width - anchor.x); + + div() + .child( + editor_popover("code-action-menu", cx) + .max_w(max_width) + .min_w(px(120.)) + .child(List::new(&list).max_h(MAX_MENU_HEIGHT)), + ) + .on_mouse_down_out({ + let menu = menu.clone(); + move |_, _, cx: &mut App| { + if let Some(menu) = menu.upgrade() { + menu.update(cx, |menu, cx| menu.hide(cx)); + } + } + }) + .into_any_element() + }, + )) .into_any_element() } } diff --git a/crates/component/src/input/popovers/completion_menu.rs b/crates/component/src/input/popovers/completion_menu.rs index 4b0ef55725..fd19eac83b 100644 --- a/crates/component/src/input/popovers/completion_menu.rs +++ b/crates/component/src/input/popovers/completion_menu.rs @@ -2,10 +2,11 @@ use std::rc::Rc; use gpui::{ Action, AnyElement, App, AppContext, Context, DismissEvent, Empty, Entity, EventEmitter, - Half as _, HighlightStyle, InteractiveElement as _, IntoElement, ParentElement, Pixels, Point, - Render, RenderOnce, SharedString, Styled, StyledText, Subscription, WeakEntity, Window, - deferred, div, prelude::FluentBuilder, px, relative, + Half as _, HighlightStyle, InteractiveElement as _, IntoElement, ParentElement, Pixels, Render, + RenderOnce, SharedString, Styled, StyledText, Subscription, WeakEntity, Window, deferred, div, + prelude::FluentBuilder, px, relative, }; +use gpui_base::TestSupportExt as _; use lsp_types::CompletionItem; const MAX_MENU_HEIGHT: Pixels = px(240.); @@ -15,7 +16,7 @@ use crate::{ ActiveTheme, IndexPath, Selectable, actions, h_flex, input::{ self, EditorState, - popovers::{editor_popover, render_markdown}, + popovers::{CaretAnchoredPopup, editor_popover, render_markdown}, }, label::Label, list::{List, ListDelegate, ListEvent, ListState}, @@ -342,26 +343,10 @@ impl CompletionMenu { cx.notify(); } - - fn origin(&self, cx: &App) -> Option> { - let editor = self.editor.upgrade()?; - let editor = editor.read(cx); - let Some((cursor_bounds, line_height)) = editor.cursor_layout() else { - return None; - }; - let cursor_origin = cursor_bounds.origin; - - let scroll_origin = editor.scroll_offset(); - - Some( - scroll_origin + cursor_origin - editor.input_bounds().origin - + Point::new(-px(4.), line_height + px(4.)), - ) - } } impl Render for CompletionMenu { - fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { if !self.open { return Empty.into_any_element(); } @@ -371,65 +356,72 @@ impl Render for CompletionMenu { return Empty.into_any_element(); } - let Some(pos) = self.origin(cx) else { - return Empty.into_any_element(); - }; - - let selected_documentation = self - .list - .read(cx) - .delegate() - .selected_item() - .and_then(|item| item.documentation.clone()); - let Some(editor) = self.editor.upgrade() else { return Empty.into_any_element(); }; + // The anchor is resolved in the popup's deferred prepaint from the + // input's current-frame geometry; the content below is built there. let configured_max = editor.read(cx).lsp().completion_menu.max_width; - let max_width = configured_max.min(window.bounds().size.width - pos.x); - let abs_pos = editor.read(cx).input_bounds().origin + pos; - let vertical_layout = - abs_pos.x + configured_max + POPOVER_GAP + configured_max + POPOVER_GAP - > window.bounds().size.width; - - deferred( - div() - .absolute() - .left(pos.x) - .top(pos.y) - .flex() - .flex_row() - .gap(POPOVER_GAP) - .items_start() - .when(vertical_layout, |this| this.flex_col()) - .child( - editor_popover("completion-menu", cx) - .max_w(max_width) - .min_w(px(120.)) - .child(List::new(&self.list).max_h(MAX_MENU_HEIGHT)), - ) - .when_some(selected_documentation, |this, documentation| { - let mut doc = match documentation { - lsp_types::Documentation::String(s) => s.clone(), - lsp_types::Documentation::MarkupContent(mc) => mc.value.clone(), - }; - if vertical_layout { - doc = doc.split("\n").next().unwrap_or_default().to_string(); - } - - this.child( - div().child( - editor_popover("completion-menu", cx) - .w(configured_max) - .px_2() - .child(render_markdown("doc", doc, window, cx)), - ), + let list = self.list.clone(); + let menu = cx.weak_entity(); + + deferred(CaretAnchoredPopup::new( + self.editor.clone(), + move |anchor, window, cx| { + // `anchor` is in window coordinates, from the current frame. + let max_width = configured_max.min(window.bounds().size.width - anchor.x); + let vertical_layout = + anchor.x + configured_max + POPOVER_GAP + configured_max + POPOVER_GAP + > window.bounds().size.width; + + let selected_documentation = list + .read(cx) + .delegate() + .selected_item() + .and_then(|item| item.documentation.clone()); + + div() + .flex() + .flex_row() + .gap(POPOVER_GAP) + .items_start() + .when(vertical_layout, |this| this.flex_col()) + .child( + editor_popover("completion-menu", cx) + .test_support() + .max_w(max_width) + .min_w(px(120.)) + .child(List::new(&list).max_h(MAX_MENU_HEIGHT)), ) - }) - .on_mouse_down_out(cx.listener(|this, _, _, cx| { - this.dismiss(cx); - })), - ) + .when_some(selected_documentation, |this, documentation| { + let mut doc = match documentation { + lsp_types::Documentation::String(s) => s.clone(), + lsp_types::Documentation::MarkupContent(mc) => mc.value.clone(), + }; + if vertical_layout { + doc = doc.split("\n").next().unwrap_or_default().to_string(); + } + + this.child( + div().child( + editor_popover("completion-menu", cx) + .w(configured_max) + .px_2() + .child(render_markdown("doc", doc, window, cx)), + ), + ) + }) + .on_mouse_down_out({ + let menu = menu.clone(); + move |_, _, cx: &mut App| { + if let Some(menu) = menu.upgrade() { + menu.update(cx, |menu, cx| menu.dismiss(cx)); + } + } + }) + .into_any_element() + }, + )) .into_any_element() } } diff --git a/crates/component/src/input/popovers/mod.rs b/crates/component/src/input/popovers/mod.rs index 2773f966b1..07697db494 100644 --- a/crates/component/src/input/popovers/mod.rs +++ b/crates/component/src/input/popovers/mod.rs @@ -1,8 +1,10 @@ +mod caret_anchored; mod code_action_menu; mod completion_menu; mod diagnostic_popover; mod hover_popover; +pub(crate) use caret_anchored::*; pub(crate) use code_action_menu::*; pub(crate) use completion_menu::*; pub(crate) use diagnostic_popover::*; diff --git a/crates/kit/tests/input/completions.rs b/crates/kit/tests/input/completions.rs index ff2d95db1e..1ba8548dc9 100644 --- a/crates/kit/tests/input/completions.rs +++ b/crates/kit/tests/input/completions.rs @@ -643,6 +643,87 @@ fn continued_typing_refreshes_provider_filtered_suggestions(cx: &mut TestAppCont fixture.assert_editor("private", cx); } +#[gpui_kit::test] +fn completion_popup_follows_caret_on_first_frame(cx: &mut TestAppContext) { + // The popup anchors to the caret from the input's current-frame prepaint + // geometry, so it must sit at the new caret position on the very frame + // the caret moves — not one frame later. + let fixture = Fixture::new(cx); + fixture.start_completion(cx); + + let before = cx + .update_window(fixture.handle.into(), |_, window, _| { + window.find("completion-menu").bounds() + }) + .unwrap(); + + // Type a character and render exactly one frame. + cx.update_window(fixture.handle.into(), |_, window, cx| { + window.input("r", cx); + }) + .unwrap(); + cx.update_window(fixture.handle.into(), |_, window, cx| { + window.render_frame(cx); + }) + .unwrap(); + cx.run_until_parked(); + + let after = cx + .update_window(fixture.handle.into(), |_, window, _| { + window.find("completion-menu").bounds() + }) + .unwrap(); + + assert!( + after.origin.x > before.origin.x, + "popup must follow the caret right on the first frame (before {before:?}, after {after:?})" + ); + // Same line, so y must not move. + assert_eq!(after.origin.y, before.origin.y); + + // And it must sit at the freshly laid-out caret, not just have moved. + let expected_x = cx.update(|cx| { + let geometry = fixture + .state + .read(cx) + .prepaint_caret_geometry() + .expect("prepaint stashes caret geometry"); + geometry.cursor_bounds.origin.x - px(4.) + }); + assert!( + (after.origin.x - expected_x).abs() < px(2.), + "popup at {after:?} must match fresh caret x {expected_x:?}" + ); +} + +#[gpui_kit::test] +fn ordinary_input_schedules_no_followup_frame(cx: &mut TestAppContext) { + // The caret-anchored popup fix must not cost ordinary inputs an extra + // frame: geometry changes notify during paint, but nothing may schedule + // a follow-up frame. + let fixture = Fixture::new(cx); + // Focus the ordinary input and type. + cx.update_window(fixture.handle.into(), |_, window, cx| { + window.click("other", cx); + }) + .unwrap(); + cx.update_window(fixture.handle.into(), |_, window, cx| { + window.input("hello", cx); + }) + .unwrap(); + fixture.settle(cx); + + let callbacks = cx + .update_window(fixture.handle.into(), |_, window, cx| { + window.simulate_next_frame(cx) + }) + .unwrap(); + assert_eq!( + callbacks, 0, + "an ordinary input must not schedule a follow-up frame" + ); +} + #[gpui_kit::test] fn typing_over_a_selection_measures_the_prefix_from_where_the_text_begins(cx: &mut TestAppContext) { let fixture = Fixture::new(cx); From b32df2dea822055a454b025fdbd889af37b719a9 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 9 Oct 2026 19:14:23 +0800 Subject: [PATCH 2/2] input: Fix caret popup dismissal and first-frame regression coverage Keep the code-action popup hitbox sized to its contents, encapsulate prepaint caret geometry, and verify completion positioning on the first frame including wrapping and scrolling. AI-assisted change using Codex. --- crates/base/src/input/base/element.rs | 7 +- crates/base/src/input/base/state.rs | 44 +++++- .../src/input/popovers/caret_anchored.rs | 7 +- .../src/input/popovers/code_action_menu.rs | 5 + crates/kit/tests/input/completions.rs | 133 ++++++++++++++---- 5 files changed, 158 insertions(+), 38 deletions(-) diff --git a/crates/base/src/input/base/element.rs b/crates/base/src/input/base/element.rs index f5a6a236e9..2e23301e68 100644 --- a/crates/base/src/input/base/element.rs +++ b/crates/base/src/input/base/element.rs @@ -2884,12 +2884,7 @@ impl Element for TextElement { input_bounds.size.width, last_layout.text_align, ); - PrepaintCaretGeometry { - cursor_bounds, - line_height, - input_bounds, - scroll_offset, - } + PrepaintCaretGeometry::new(cursor_bounds, line_height, input_bounds, scroll_offset) }); self.state.update(cx, |state, _| { state.prepaint_caret_geometry = caret_geometry; diff --git a/crates/base/src/input/base/state.rs b/crates/base/src/input/base/state.rs index 4e55c30588..f5e7d5a350 100644 --- a/crates/base/src/input/base/state.rs +++ b/crates/base/src/input/base/state.rs @@ -347,15 +347,47 @@ struct PasteTarget { #[derive(Clone, Copy, Debug)] #[doc(hidden)] pub struct PrepaintCaretGeometry { + cursor_bounds: Bounds, + line_height: Pixels, + input_bounds: Bounds, + scroll_offset: Point, +} + +impl PrepaintCaretGeometry { + pub(super) fn new( + cursor_bounds: Bounds, + line_height: Pixels, + input_bounds: Bounds, + scroll_offset: Point, + ) -> Self { + Self { + cursor_bounds, + line_height, + input_bounds, + scroll_offset, + } + } + /// The active caret's bounds from the fresh prepaint layout. - pub cursor_bounds: Bounds, + pub fn cursor_bounds(&self) -> Bounds { + self.cursor_bounds + } + /// Line height from the fresh prepaint layout. - pub line_height: Pixels, + pub fn line_height(&self) -> Pixels { + self.line_height + } + /// The input container bounds from the fresh prepaint. - pub input_bounds: Bounds, - /// The scroll offset that will be in effect after paint (clamped like - /// `update_scroll_offset` clamps it). - pub scroll_offset: Point, + pub fn input_bounds(&self) -> Bounds { + self.input_bounds + } + + /// The scroll offset that will be in effect after paint, clamped like + /// `InputBaseState::update_scroll_offset` clamps it. + pub fn scroll_offset(&self) -> Point { + self.scroll_offset + } } /// The shared text-editing engine behind [`crate::input::InputState`], diff --git a/crates/component/src/input/popovers/caret_anchored.rs b/crates/component/src/input/popovers/caret_anchored.rs index c4ffbff0ff..86ae26b350 100644 --- a/crates/component/src/input/popovers/caret_anchored.rs +++ b/crates/component/src/input/popovers/caret_anchored.rs @@ -20,8 +20,11 @@ use crate::input::{EditorState, PrepaintCaretGeometry}; /// coordinates, matching where the caret itself is painted. pub(crate) fn caret_popup_anchor(geometry: PrepaintCaretGeometry) -> Point { Point::new( - geometry.cursor_bounds.origin.x - px(4.), - geometry.cursor_bounds.origin.y + geometry.scroll_offset.y + geometry.line_height + px(4.), + geometry.cursor_bounds().origin.x - px(4.), + geometry.cursor_bounds().origin.y + + geometry.scroll_offset().y + + geometry.line_height() + + px(4.), ) } diff --git a/crates/component/src/input/popovers/code_action_menu.rs b/crates/component/src/input/popovers/code_action_menu.rs index 521a5bc65d..a8680994d8 100644 --- a/crates/component/src/input/popovers/code_action_menu.rs +++ b/crates/component/src/input/popovers/code_action_menu.rs @@ -6,6 +6,7 @@ use gpui::{ Styled, StyledText, Subscription, WeakEntity, Window, deferred, div, prelude::FluentBuilder, px, relative, }; +use gpui_base::TestSupportExt as _; pub(crate) use gpui_base::input::CodeActionItem; const MAX_MENU_WIDTH: Pixels = px(320.); @@ -304,9 +305,13 @@ impl Render for CodeActionMenu { move |anchor, window, cx| { let max_width = MAX_MENU_WIDTH.min(window.bounds().size.width - anchor.x); + // A flex root measures to its contents; a block root would + // stretch its outside-click hitbox across the viewport. div() + .flex() .child( editor_popover("code-action-menu", cx) + .test_support() .max_w(max_width) .min_w(px(120.)) .child(List::new(&list).max_h(MAX_MENU_HEIGHT)), diff --git a/crates/kit/tests/input/completions.rs b/crates/kit/tests/input/completions.rs index 1ba8548dc9..9ce6a2ce7e 100644 --- a/crates/kit/tests/input/completions.rs +++ b/crates/kit/tests/input/completions.rs @@ -1,6 +1,6 @@ //! Completion workflows through the styled editor, its real popup, and native input. -//! The popup has no TestWindowExt observation; text, focus, and provider requests -//! are the public evidence, with acceptance proving that menu actions were routed. +//! Popup bounds verify caret anchoring; text, focus, and provider requests +//! verify the workflow, with acceptance proving that menu actions were routed. use std::{ cell::{Cell, RefCell}, @@ -10,16 +10,16 @@ use std::{ }; use gpui_kit::{ - App, AppContext, Context, Entity, Result, SharedString, Subscription, Task, TestAppContext, - Window, WindowHandle, + App, AppContext, Bounds, Context, Entity, Keystroke, MouseButton, Pixels, Result, SharedString, + Subscription, Task, TestAppContext, Window, WindowHandle, component::input::{ CodeActionProvider, CompletionProvider, Editor, EditorState, Input, InputEvent, InputState, Rope, ShowCompletions, }, - div, + div, point, prelude::*, px, size, - test::TestWindowExt, + test::{TestEventExt, TestWindowExt}, }; use lsp_types::{ CodeAction, CompletionContext, CompletionItem, CompletionResponse, CompletionTextEdit, @@ -265,6 +265,23 @@ impl Fixture { self.settle(cx); } + fn completion_after_first_frame(&self, text: &str, cx: &mut TestAppContext) -> Bounds { + // Dispatch native input without the test helper's automatic draw. The + // provider must finish before rendering the updated completion menu. + cx.update_window(self.handle.into(), |_, window, cx| { + let mut key = Keystroke::parse(text).expect("valid test keystroke"); + key.key_char = Some(text.into()); + window.dispatch_keystroke(key, cx); + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(self.handle.into(), |_, window, cx| { + window.render_frame(cx); + window.find("completion-menu").bounds() + }) + .unwrap() + } + fn set_value(&self, value: &str, cx: &mut TestAppContext) { cx.update_window(self.handle.into(), |_, window, cx| { self.state @@ -657,22 +674,7 @@ fn completion_popup_follows_caret_on_first_frame(cx: &mut TestAppContext) { }) .unwrap(); - // Type a character and render exactly one frame. - cx.update_window(fixture.handle.into(), |_, window, cx| { - window.input("r", cx); - }) - .unwrap(); - cx.update_window(fixture.handle.into(), |_, window, cx| { - window.render_frame(cx); - }) - .unwrap(); - cx.run_until_parked(); - - let after = cx - .update_window(fixture.handle.into(), |_, window, _| { - window.find("completion-menu").bounds() - }) - .unwrap(); + let after = fixture.completion_after_first_frame("r", cx); assert!( after.origin.x > before.origin.x, @@ -688,7 +690,7 @@ fn completion_popup_follows_caret_on_first_frame(cx: &mut TestAppContext) { .read(cx) .prepaint_caret_geometry() .expect("prepaint stashes caret geometry"); - geometry.cursor_bounds.origin.x - px(4.) + geometry.cursor_bounds().origin.x - px(4.) }); assert!( (after.origin.x - expected_x).abs() < px(2.), @@ -711,7 +713,6 @@ fn ordinary_input_schedules_no_followup_frame(cx: &mut TestAppContext) { window.input("hello", cx); }) .unwrap(); - fixture.settle(cx); let callbacks = cx .update_window(fixture.handle.into(), |_, window, cx| { @@ -724,6 +725,64 @@ fn ordinary_input_schedules_no_followup_frame(cx: &mut TestAppContext) { ); } +struct CaretSuggestions; + +impl CompletionProvider for CaretSuggestions { + fn completions( + &self, + _: &Rope, + _: usize, + _: CompletionContext, + _: &mut Window, + _: &mut App, + ) -> Task> { + Task::ready(Ok(CompletionResponse::Array(vec![CompletionItem { + label: "print".into(), + ..Default::default() + }]))) + } + + fn is_completion_trigger(&self, _: usize, _: &str, _: &mut App) -> bool { + true + } +} + +#[gpui_kit::test] +fn completion_popup_anchors_to_wrapped_and_scrolled_caret_on_first_frame(cx: &mut TestAppContext) { + for (text, scrolled) in [ + ("p".repeat(240), false), + (format!("{}p", "p\n".repeat(40)), true), + ] { + let fixture = Fixture::new(cx); + fixture.state.update(cx, |state, _| { + state.lsp_mut().completion_provider = Some(Rc::new(CaretSuggestions)); + }); + fixture.set_value(&text, cx); + fixture.select(text.len()..text.len(), cx); + fixture.show_completions(cx); + + let popup = fixture.completion_after_first_frame("r", cx); + fixture.state.read_with(cx, |state, _| { + // Compare with the geometry published by the completed input paint, + // independently of the popup's prepaint snapshot. + let (caret, line_height) = state.cursor_layout().expect("painted caret"); + let scroll = state.scroll_offset(); + assert!(caret.origin.y > state.input_bounds().origin.y + line_height); + if scrolled { + assert!( + scroll.y < px(0.), + "fixture must exercise vertical scrolling" + ); + } + assert!((popup.origin.x - (caret.origin.x - px(4.))).abs() < px(2.)); + assert!( + (popup.origin.y - (caret.origin.y + scroll.y + line_height + px(4.))).abs() + < px(2.) + ); + }); + } +} + #[gpui_kit::test] fn typing_over_a_selection_measures_the_prefix_from_where_the_text_begins(cx: &mut TestAppContext) { let fixture = Fixture::new(cx); @@ -1193,6 +1252,32 @@ fn code_action_fixture(cx: &mut TestAppContext, providers: Vec>) -> fixture } +#[gpui_kit::test] +fn clicking_beside_code_action_menu_dismisses_it(cx: &mut TestAppContext) { + let provider = Actions::new("First action", false); + let fixture = code_action_fixture(cx, vec![provider.clone()]); + fixture.press(CODE_ACTIONS, cx); + + cx.update_window(fixture.handle.into(), |_, window, cx| { + let popup = window.find("code-action-menu").bounds(); + let outside = point(popup.right() + px(24.), popup.center().y); + assert!( + outside.x < window.viewport_size().width, + "popup {popup:?}, viewport {:?}", + window.viewport_size() + ); + window.pointer_down(outside, MouseButton::Left, cx); + window.pointer_up(outside, MouseButton::Left, cx); + assert!(window.try_find("code-action-menu").is_none()); + }) + .unwrap(); + fixture.settle(cx); + fixture.state.read_with(cx, |state, _| { + assert!(!state.code_action_menu_state().open); + }); + assert!(provider.performed.borrow().is_empty()); +} + #[gpui_kit::test] fn code_action_selection_routes_to_its_provider_with_selected_range(cx: &mut TestAppContext) { let first = Actions::new("First action", false);