diff --git a/crates/fresh-editor/src/app/mouse_input.rs b/crates/fresh-editor/src/app/mouse_input.rs index 22c31a57b8..07f5b6cf70 100644 --- a/crates/fresh-editor/src/app/mouse_input.rs +++ b/crates/fresh-editor/src/app/mouse_input.rs @@ -12,7 +12,7 @@ use crate::input::keybindings::Action; use crate::model::event::{ContainerId, CursorId, LeafId, SplitDirection}; use crate::services::plugins::hooks::HookArgs; use crate::view::popup_mouse::{popup_areas_to_layout_info, PopupHitTester}; -use crate::view::prompt::PromptType; +use crate::view::prompt::{PromptType, MAX_VISIBLE_SUGGESTIONS}; use crate::view::ui::tabs::TabHit; use anyhow::Result as AnyhowResult; use ratatui::layout::Rect; @@ -23,6 +23,33 @@ fn in_rect(col: u16, row: u16, rect: Rect) -> bool { col >= rect.x && col < rect.x + rect.width && row >= rect.y && row < rect.y + rect.height } +/// Map a screen row on a suggestion list's scrollbar track to the prompt +/// scroll offset that puts the thumb's top on exactly that row. +/// +/// Shared by the press and the drag-follow-up so the thumb tracks the cursor +/// identically in both. `ScrollbarState::click_to_offset` is deliberately +/// *not* used here: it divides by the whole track height instead of by the +/// thumb's actual travel, so it lands the thumb a row above the row the user +/// pointed at and can never reach the bottom of the track. +/// [`ScrollbarState::offset_for_thumb_top`] is the real inverse of the thumb +/// geometry the renderer draws. +/// +/// Rows above/below the track clamp to its ends rather than being rejected, +/// so a fast drag doesn't drop the thumb. +fn prompt_scrollbar_offset_for_row( + total: usize, + visible: usize, + scroll_offset: usize, + sb_rect: Rect, + row: u16, +) -> usize { + use crate::view::ui::scrollbar::ScrollbarState; + let clamped_row = row.clamp(sb_rect.y, sb_rect.y + sb_rect.height.saturating_sub(1)); + let track_row = clamped_row.saturating_sub(sb_rect.y) as usize; + ScrollbarState::new(total, visible, scroll_offset) + .offset_for_thumb_top(sb_rect.height as usize, track_row) +} + /// Where a screen cell lands inside a floating widget panel. See /// [`Editor::probe_floating_widget`]. struct FloatingWidgetProbe { @@ -544,7 +571,9 @@ impl Editor { // under the pointer — the preview when over it, otherwise the // result list (without moving the selection). See issue #2119. } else if self.handle_prompt_scroll(delta) { - // bottom-anchored prompt consumed the scroll (moves selection) + // bottom-anchored prompt dropdown consumed the scroll: it scrolls + // its list without moving the selection, like every other wheel + // surface in the editor. } else if self.is_file_open_active() && self.is_mouse_over_file_browser(col, row) && self.handle_file_open_scroll(delta) @@ -598,7 +627,8 @@ impl Editor { /// result list *without* moving the selection. /// /// Bottom-anchored prompts (command palette, file finder) are left to - /// `handle_prompt_scroll`, which keeps their wheel-moves-selection UX. + /// `handle_prompt_scroll`, which scrolls their dropdown the same + /// selection-preserving way. fn handle_overlay_prompt_scroll(&mut self, col: u16, row: u16, delta: i32) -> bool { if !self.overlay_prompt_active() { return false; @@ -1953,13 +1983,10 @@ impl Editor { Some(self.handle_action(Action::PromptConfirm)) } - /// Click/drag on the floating-overlay prompt's scrollbar - /// (issue #1796). Reuses - /// `view::ui::scrollbar::ScrollbarState::click_to_offset` for - /// the same math the popup-scrollbar handler uses, so thumb - /// behaviour is consistent across the editor. + /// Click/drag on a suggestion-list scrollbar: the floating-overlay + /// prompt's (issue #1796) and the bottom-anchored dropdown's + /// (issues #623 / #1593), which share `suggestions_scrollbar_rect`. fn handle_click_prompt_scrollbar(&mut self, col: u16, row: u16) -> Option> { - use crate::view::ui::scrollbar::ScrollbarState; let sb_rect = self.active_chrome().suggestions_scrollbar_rect?; if col < sb_rect.x || col >= sb_rect.x + sb_rect.width @@ -1981,12 +2008,19 @@ impl Editor { .windows .get_mut(&active_window_id) .and_then(|w| w.prompt.as_mut())?; - let visible = suggestions_area_visible.unwrap_or(prompt.suggestions.len().min(10)); - let total = prompt.suggestions.len(); - let track_height = sb_rect.height as usize; - let click_row = row.saturating_sub(sb_rect.y) as usize; - let state = ScrollbarState::new(total, visible, prompt.scroll_offset); - prompt.scroll_offset = state.click_to_offset(track_height, click_row); + let visible = suggestions_area_visible + .unwrap_or_else(|| prompt.suggestions.len().min(MAX_VISIBLE_SUGGESTIONS)); + prompt.scroll_offset = prompt_scrollbar_offset_for_row( + prompt.suggestions.len(), + visible, + prompt.scroll_offset, + sb_rect, + row, + ); + // Latch manual scroll so the renderer's keep-selection-visible + // pass doesn't immediately yank the offset back to the selection + // (same latch the wheel uses; released when the selection moves). + prompt.manual_scroll = true; // Hand off to the drag follow-up so subsequent mouse moves // keep tracking the thumb. self.active_window_mut() @@ -2976,7 +3010,6 @@ impl Editor { .mouse_state .dragging_prompt_scrollbar { - use crate::view::ui::scrollbar::ScrollbarState; // Snapshot chrome rects up front so the prompt borrow on // active_window_mut() doesn't conflict. let sb_rect = self.active_chrome().suggestions_scrollbar_rect; @@ -2989,17 +3022,18 @@ impl Editor { .get_mut(&active_window_id) .and_then(|w| w.prompt.as_mut()), ) { - let visible = suggestions_area_visible.unwrap_or(prompt.suggestions.len().min(10)); - let total = prompt.suggestions.len(); - let track_height = sb_rect.height as usize; - // Allow dragging slightly past the top/bottom; clamp - // here rather than rejecting so the thumb keeps up - // with a fast mouse. - let clamped_row = - row.clamp(sb_rect.y, sb_rect.y + sb_rect.height.saturating_sub(1)); - let click_row = clamped_row.saturating_sub(sb_rect.y) as usize; - let state = ScrollbarState::new(total, visible, prompt.scroll_offset); - prompt.scroll_offset = state.click_to_offset(track_height, click_row); + let visible = suggestions_area_visible + .unwrap_or_else(|| prompt.suggestions.len().min(MAX_VISIBLE_SUGGESTIONS)); + prompt.scroll_offset = prompt_scrollbar_offset_for_row( + prompt.suggestions.len(), + visible, + prompt.scroll_offset, + sb_rect, + row, + ); + // Keep the manual-scroll latch through the drag so the + // renderer doesn't pull the offset back to the selection. + prompt.manual_scroll = true; } return Ok(()); } diff --git a/crates/fresh-editor/src/app/prompt_lifecycle.rs b/crates/fresh-editor/src/app/prompt_lifecycle.rs index efdeeda8af..67dcb6a631 100644 --- a/crates/fresh-editor/src/app/prompt_lifecycle.rs +++ b/crates/fresh-editor/src/app/prompt_lifecycle.rs @@ -15,7 +15,7 @@ use crate::input::keybindings::KeyContext; use crate::input::quick_open::{BufferInfo, QuickOpenContext}; use crate::services::async_bridge::AsyncMessage; use crate::services::plugins::PluginManager; -use crate::view::prompt::{Prompt, PromptType}; +use crate::view::prompt::{Prompt, PromptType, MAX_VISIBLE_SUGGESTIONS}; use super::file_open; use super::window::Window; @@ -967,37 +967,34 @@ impl Editor { } } - /// Handle mouse wheel scroll in prompt with suggestions. + /// Handle mouse wheel scroll over a prompt's suggestion list (command + /// palette, Select Locale, quick open, every other bottom-anchored + /// dropdown). + /// + /// The wheel scrolls the **view only** — it never moves the selection. + /// That is the editor-wide rule (and what VS Code does): the highlighted + /// entry may scroll out of sight, and pressing Enter still commits it. + /// Wheeling used to walk `selected_suggestion` instead, which also + /// rewrote the prompt input under the user and — once the scrollbar + /// could latch the offset — made the list jump, because the wheel + /// released the latch and the renderer snapped the view back to a + /// selection that had never visibly moved. + /// /// Returns true if scroll was handled, false if no prompt is active or has no suggestions. pub fn handle_prompt_scroll(&mut self, delta: i32) -> bool { + // Scroll by what the renderer actually drew (`suggestions_area` is + // `(inner_rect, scroll_start_idx, visible_count, total_count)`), so + // the offset can't run past the end of the list. Read before + // borrowing the prompt: `active_window_mut()` is a method call, so + // the compiler can't see the two are disjoint sub-fields. + let visible_rows = self.active_chrome().suggestions_area.map(|(_, _, v, _)| v); if let Some(ref mut prompt) = self.active_window_mut().prompt { if prompt.suggestions.is_empty() { return false; } - - let current = prompt.selected_suggestion.unwrap_or(0); - let len = prompt.suggestions.len(); - - // Calculate new position based on scroll direction - // delta < 0 = scroll up, delta > 0 = scroll down - let new_selected = if delta < 0 { - // Scroll up - move selection up (decrease index) - current.saturating_sub((-delta) as usize) - } else { - // Scroll down - move selection down (increase index) - (current + delta as usize).min(len.saturating_sub(1)) - }; - - prompt.selected_suggestion = Some(new_selected); - - // Update input to match selected suggestion for non-plugin prompts - if !matches!(prompt.prompt_type, PromptType::Plugin { .. }) { - if let Some(suggestion) = prompt.suggestions.get(new_selected) { - prompt.input = suggestion.get_value().to_string(); - prompt.cursor_pos = prompt.input.len(); - } - } - + let visible = visible_rows + .unwrap_or_else(|| prompt.suggestions.len().min(MAX_VISIBLE_SUGGESTIONS)); + prompt.scroll_results(delta, visible); return true; } false diff --git a/crates/fresh-editor/src/app/render.rs b/crates/fresh-editor/src/app/render.rs index 90c8e71fb1..848fd56c2c 100644 --- a/crates/fresh-editor/src/app/render.rs +++ b/crates/fresh-editor/src/app/render.rs @@ -748,6 +748,7 @@ impl Editor { // Initialize popup/suggestion layout state (rendered after status bar below) self.active_chrome_mut().suggestions_area = None; self.active_chrome_mut().suggestions_outer_area = None; + self.active_chrome_mut().suggestions_scrollbar_rect = None; self.active_chrome_mut().prompt_results_area = None; self.active_chrome_mut().prompt_preview_area = None; self.active_window_mut().file_browser_layout = None; @@ -2453,9 +2454,12 @@ impl Editor { return; } - let suggestion_count = prompt.suggestions.len().min(10); let is_quick_open = prompt.prompt_type == crate::view::prompt::PromptType::QuickOpen; let hints_height: u16 = if is_quick_open { 1 } else { 0 }; + let suggestion_count = prompt + .suggestions + .len() + .min(crate::view::prompt::MAX_VISIBLE_SUGGESTIONS); let height = suggestion_count as u16 + 2 + hints_height; let suggestions_area = ratatui::layout::Rect { @@ -2478,9 +2482,14 @@ impl Editor { } // Adjust the prompt's scroll position to keep the selected item - // visible, scrolling the minimum amount required. + // visible, scrolling the minimum amount required — unless the user + // has scrolled the list with the scrollbar, in which case pulling + // the offset back would undo their scroll (same reasoning as the + // overlay prompt, issue #2119). if let Some(prompt) = self.active_window_mut().prompt.as_mut() { - prompt.ensure_selected_visible(); + if !prompt.manual_scroll { + prompt.ensure_selected_visible_within(suggestion_count); + } } let Some(prompt) = &self.active_window().prompt else { return; @@ -2500,6 +2509,19 @@ impl Editor { if chrome.suggestions_area.is_some() { chrome.suggestions_outer_area = Some(suggestions_area); } + // When the list overflows, the renderer drew a scrollbar over the + // popup's right border; record its rect so the shared prompt- + // scrollbar mouse handlers (click-to-jump, thumb drag) work here + // exactly like in the overlay prompt (issue #623 / #1593). + chrome.suggestions_scrollbar_rect = + new_suggestions_area.and_then(|(inner, _, visible, total)| { + (total > visible).then_some(ratatui::layout::Rect { + x: inner.x + inner.width, + y: inner.y, + width: 1, + height: inner.height, + }) + }); // The quick-open hints row is chrome drawn into cells; the web renders // no hints, so in `suppress_chrome_cells` mode we skip it entirely diff --git a/crates/fresh-editor/src/view/popup.rs b/crates/fresh-editor/src/view/popup.rs index 8ba3bda8a3..7b663e324d 100644 --- a/crates/fresh-editor/src/view/popup.rs +++ b/crates/fresh-editor/src/view/popup.rs @@ -586,8 +586,15 @@ impl Popup { } } - /// Scroll by a delta amount (positive = down, negative = up) - /// Used for mouse wheel scrolling + /// Scroll by a delta amount (positive = down, negative = up). + /// + /// Used for mouse-wheel scrolling and for the scrollbar handlers, so it + /// moves the **view only**: a `List` popup's selection stays on whatever + /// entry it was on, even when that entry scrolls off-screen. Dragging the + /// selection along with the viewport made the wheel silently retarget + /// what Enter would commit; the keyboard paths (`select_next`, + /// `page_down`, …) are the ones that move the selection, and they scroll + /// the view to follow it. pub fn scroll_by(&mut self, delta: i32) { let content_len = self.wrapped_item_count(); let visible = self.visible_height(); @@ -600,18 +607,6 @@ impl Popup { // Scroll down self.scroll_offset = (self.scroll_offset + delta as usize).min(max_scroll); } - - // For list popups, adjust selection to stay visible - if let PopupContent::List { items, selected } = &mut self.content { - let visible_start = self.scroll_offset; - let visible_end = (self.scroll_offset + visible).min(items.len()); - - if *selected < visible_start { - *selected = visible_start; - } else if *selected >= visible_end { - *selected = visible_end.saturating_sub(1); - } - } } /// Get the total number of items/lines in the popup diff --git a/crates/fresh-editor/src/view/prompt.rs b/crates/fresh-editor/src/view/prompt.rs index 920fe901e9..d7e458e0be 100644 --- a/crates/fresh-editor/src/view/prompt.rs +++ b/crates/fresh-editor/src/view/prompt.rs @@ -296,9 +296,17 @@ pub struct Prompt { pub status: String, } -/// Maximum number of suggestion rows shown at once. Mirrors the cap used by -/// `SuggestionsRenderer` so `Prompt::ensure_selected_visible` can compute the -/// viewport size without inspecting render state. +/// Maximum number of suggestion rows a bottom-anchored dropdown shows at once. +/// The palette is a transient overlay drawn over the document, so it stays +/// deliberately short rather than growing with the terminal — on an 80x24 +/// terminal a height-sized dropdown would cover all but one line of the +/// buffer, and on a split layout it would hide the other pane entirely. +/// Overflow is communicated by the scrollbar the renderer draws over the +/// popup's right border (issues #623 / #1593) instead of by more rows. +/// +/// Renderers with their own geometry (the floating Live Grep overlay, which +/// can be 30+ rows tall) pass their actual height to +/// [`Prompt::ensure_selected_visible_within`] rather than using this cap. pub const MAX_VISIBLE_SUGGESTIONS: usize = 10; impl Prompt { @@ -674,28 +682,15 @@ impl Prompt { self.manual_scroll = false; } - /// Adjust `scroll_offset` so that `selected_suggestion` is inside the - /// viewport, scrolling the minimum amount required. A selection that's - /// already on-screen leaves the viewport untouched — this is what stops - /// a click on a near-bottom item from snapping the list upward and - /// recentering under the cursor (issue #1660). - /// - /// Uses the bottom-popup default cap (`MAX_VISIBLE_SUGGESTIONS`). - /// Callers rendering into a different-sized area (e.g. the - /// floating Live Grep overlay, where the suggestion list can be - /// 30+ rows tall) should call - /// [`ensure_selected_visible_within`] with the actual height - /// instead — otherwise the scroll moves prematurely once the - /// selection passes the 10th row even though the rest of the - /// list is still visible on-screen. - pub fn ensure_selected_visible(&mut self) { - self.ensure_selected_visible_within(MAX_VISIBLE_SUGGESTIONS); - } - - /// Like [`ensure_selected_visible`] but with an explicit - /// `visible_count` argument, so renderers in differently-sized - /// frames don't all share the bottom-popup `MAX_VISIBLE_SUGGESTIONS` - /// assumption. + /// Adjust `scroll_offset` so that `selected_suggestion` is inside a + /// viewport of `visible_count` rows, scrolling the minimum amount + /// required. A selection that's already on-screen leaves the viewport + /// untouched — this is what stops a click on a near-bottom item from + /// snapping the list upward and recentering under the cursor (issue + /// #1660). Callers pass the actual rendered height of their list + /// (bottom-anchored popup and floating Live Grep overlay alike), so + /// the scroll only moves when the selection genuinely leaves the + /// visible window. pub fn ensure_selected_visible_within(&mut self, visible_count: usize) { let total = self.suggestions.len(); let visible = total.min(visible_count.max(1)); diff --git a/crates/fresh-editor/src/view/ui/scrollbar.rs b/crates/fresh-editor/src/view/ui/scrollbar.rs index 1b085ddd2f..520e6da5eb 100644 --- a/crates/fresh-editor/src/view/ui/scrollbar.rs +++ b/crates/fresh-editor/src/view/ui/scrollbar.rs @@ -113,8 +113,26 @@ impl ScrollbarState { return 0; } let clamped = target_thumb_top.min(max_thumb_top); - let ratio = clamped as f64 / max_thumb_top as f64; - ((ratio * max_scroll as f64).round() as usize).min(max_scroll) + // `thumb_geometry` *floors* offset → thumb row, so a rounding + // division here lands the thumb one row above the target whenever the + // exact quotient has a fractional part. Take the smallest offset that + // reaches the row (ceiling division) instead, and keep the offset + // below it as a candidate: when there are fewer scroll positions than + // track rows, not every row is reachable and the nearest one wins. + let hi = (clamped * max_scroll) + .div_ceil(max_thumb_top) + .min(max_scroll); + let lo = hi.saturating_sub(1); + let thumb_top_of = |offset: usize| { + Self::new(self.total_items, self.visible_items, offset) + .thumb_geometry(track_height) + .0 + }; + if thumb_top_of(lo).abs_diff(clamped) < thumb_top_of(hi).abs_diff(clamped) { + lo + } else { + hi + } } /// Compute the scroll offset for a drag that preserves the cursor's @@ -505,29 +523,72 @@ mod tests { #[test] fn test_offset_for_thumb_top_round_trip() { // For every reachable thumb row, `offset_for_thumb_top` must - // produce an offset whose rendered thumb top matches that row — - // i.e. it really is the inverse of `thumb_geometry`. + // produce an offset whose rendered thumb top matches that row + // *exactly* — it really is the inverse of `thumb_geometry`. The + // last two cases are the prompt-dropdown shape (10 visible rows on + // a 10-row track), where a rounding inverse used to land the thumb + // a row above the row the user clicked. let cases = [ (200_usize, 50_usize, 20_usize), (1000, 30, 25), (50, 10, 15), + (30, 10, 10), + (14, 10, 10), ]; for (total, visible, track) in cases { let probe = ScrollbarState::new(total, visible, 0); let (_, thumb_size) = probe.thumb_geometry(track); let max_thumb_top = track.saturating_sub(thumb_size); + let max_scroll = total - visible; + // Every row is reachable only when there are at least as many + // scroll positions as thumb rows; otherwise the mapping can + // only pick the nearest reachable row (asserted below). + assert!( + max_thumb_top <= max_scroll, + "case (total={total} visible={visible} track={track}) is not exactly invertible" + ); for target in 0..=max_thumb_top { let offset = probe.offset_for_thumb_top(track, target); let placed = ScrollbarState::new(total, visible, offset); let (got_top, _) = placed.thumb_geometry(track); - assert!( - got_top.abs_diff(target) <= 1, + assert_eq!( + got_top, target, "thumb landed at {got_top}, expected {target} (total={total} visible={visible} track={track})" ); } } } + #[test] + fn test_offset_for_thumb_top_picks_nearest_when_rows_unreachable() { + // 11 items in a 10-row viewport: only two scroll positions exist, + // so most track rows can't be hit. The mapping must still pick the + // closest reachable thumb row rather than always rounding down. + let total = 11; + let visible = 10; + let track = 10; + let probe = ScrollbarState::new(total, visible, 0); + let (_, thumb_size) = probe.thumb_geometry(track); + let max_thumb_top = track - thumb_size; + for target in 0..=max_thumb_top { + let offset = probe.offset_for_thumb_top(track, target); + let (got_top, _) = ScrollbarState::new(total, visible, offset).thumb_geometry(track); + let best = (0..=(total - visible)) + .map(|o| { + ScrollbarState::new(total, visible, o) + .thumb_geometry(track) + .0 + }) + .min_by_key(|top| top.abs_diff(target)) + .unwrap(); + assert_eq!( + got_top.abs_diff(target), + best.abs_diff(target), + "target row {target}: landed at {got_top}, nearest reachable was {best}" + ); + } + } + #[test] fn test_offset_for_thumb_top_clamps_to_max() { let state = ScrollbarState::new(200, 50, 0); diff --git a/crates/fresh-editor/src/view/ui/status_bar.rs b/crates/fresh-editor/src/view/ui/status_bar.rs index 62d32a910a..494ead63af 100644 --- a/crates/fresh-editor/src/view/ui/status_bar.rs +++ b/crates/fresh-editor/src/view/ui/status_bar.rs @@ -778,6 +778,22 @@ fn format_cursor_position_compact(line: usize, col: usize, line_count: usize) -> } } +/// Horizontal scroll (in display cells) for a single-line input rendered +/// in a viewport `width` cells wide, so the cursor at display column +/// `cursor_cells` is always visible (issue #2876). Returns 0 while the +/// cursor fits; otherwise scrolls just enough that the cursor lands on +/// the viewport's last column. +/// +/// Invariant (for `width > 0`): `scroll <= cursor_cells` and +/// `cursor_cells - scroll < width`. +pub(crate) fn input_hscroll(cursor_cells: usize, width: usize) -> usize { + if width == 0 { + 0 + } else { + cursor_cells.saturating_sub(width - 1) + } +} + /// Renders the status bar and prompt/minibuffer pub struct StatusBarRenderer; @@ -808,8 +824,9 @@ impl StatusBarRenderer { ) { let base_style = Style::default().fg(theme.prompt_fg).bg(theme.prompt_bg); - // Create spans for the prompt - let mut spans = vec![Span::styled(prompt.message.clone(), base_style)]; + // Create spans for the input (the message/label is rendered + // separately so it stays anchored while the input scrolls). + let mut spans = Vec::new(); // If there's a selection, split the input into parts if let Some((sel_start, sel_end)) = prompt.selection_range() { @@ -841,20 +858,66 @@ impl StatusBarRenderer { spans.push(Span::styled(prompt.input.clone(), base_style)); } - let line = Line::from(spans); - let prompt_line = Paragraph::new(line).style(base_style); + Self::render_prompt_label_and_input( + frame, + area, + vec![Span::styled(prompt.message.clone(), base_style)], + spans, + base_style, + str_width(&prompt.input[..prompt.cursor_pos.min(prompt.input.len())]), + ); + } - frame.render_widget(prompt_line, area); + /// Shared tail of the prompt-line renderers: paint the label spans + /// anchored at the left edge, then the input spans in the remaining + /// columns with a horizontal scroll that keeps the cursor visible + /// (issue #2876), and always place the terminal cursor on the input. + /// + /// `cursor_cells` is the display width of the input up to the cursor. + fn render_prompt_label_and_input( + frame: &mut Frame, + area: Rect, + label_spans: Vec>, + input_spans: Vec>, + base_style: Style, + cursor_cells: usize, + ) { + // Label, clipped to the area. Use display width (not byte length) + // for proper handling of double-width CJK and zero-width + // combining characters. + let label_cells: usize = label_spans.iter().map(|s| str_width(&s.content)).sum(); + let label_cols = (label_cells.min(area.width as usize)) as u16; + let label_area = Rect { + x: area.x, + y: area.y, + width: label_cols, + height: area.height, + }; + frame.render_widget( + Paragraph::new(Line::from(label_spans)).style(base_style), + label_area, + ); + + // Input, horizontally scrolled so the cursor never leaves the + // viewport: with text longer than the box the line scrolls left + // and the cursor rides the last column. + let input_area = Rect { + x: area.x + label_cols, + y: area.y, + width: area.width - label_cols, + height: area.height, + }; + let scroll = input_hscroll(cursor_cells, input_area.width as usize); + frame.render_widget( + Paragraph::new(Line::from(input_spans)) + .style(base_style) + .scroll((0, scroll as u16)), + input_area, + ); - // Set cursor position in the prompt - // Use display width (not byte length) for proper handling of: - // - Double-width CJK characters - // - Zero-width combining characters (Thai diacritics, etc.) - let message_width = str_width(&prompt.message); - let input_width_before_cursor = str_width(&prompt.input[..prompt.cursor_pos]); - let cursor_x = (message_width + input_width_before_cursor) as u16; - if cursor_x < area.width { - frame.set_cursor_position((area.x + cursor_x, area.y)); + if input_area.width > 0 { + // `input_hscroll` guarantees cursor_cells - scroll < width. + frame.set_cursor_position((input_area.x + (cursor_cells - scroll) as u16, area.y)); } } @@ -937,34 +1000,18 @@ impl StatusBarRenderer { spans.push(Span::styled(path_display, dir_style)); } - // User input (the filename part) - normal color - spans.push(Span::styled(prompt.input.clone(), base_style)); - - let line = Line::from(spans); - let prompt_line = Paragraph::new(line).style(base_style); - - frame.render_widget(prompt_line, area); - - // Set cursor position in the prompt - // Use display width for proper handling of Unicode characters - // We need to calculate the visual width of: "Open: " + dir_display + input[..cursor_pos] - let prefix_width = str_width(&open_prompt); - let dir_display_width = if truncated.truncated { - let suffix_with_slash = if truncated.suffix.ends_with('/') { - &truncated.suffix - } else { - // We already added "/" in the suffix_with_slash above, so approximate - &truncated.suffix - }; - str_width(&truncated.prefix) + str_width("/[...]") + str_width(suffix_with_slash) + 1 - } else { - str_width(&truncated.suffix) + 1 // +1 for trailing slash - }; - let input_width_before_cursor = str_width(&prompt.input[..prompt.cursor_pos]); - let cursor_x = (prefix_width + dir_display_width + input_width_before_cursor) as u16; - if cursor_x < area.width { - frame.set_cursor_position((area.x + cursor_x, area.y)); - } + // The label here is the whole prefix (message + colorized dir path); + // the user input (the filename part) scrolls after it so the cursor + // stays visible even when the typed name overflows the line. + let input_spans = vec![Span::styled(prompt.input.clone(), base_style)]; + Self::render_prompt_label_and_input( + frame, + area, + spans, + input_spans, + base_style, + str_width(&prompt.input[..prompt.cursor_pos.min(prompt.input.len())]), + ); } /// Render a single element to its text representation. @@ -2962,4 +3009,112 @@ mod tests { let line_start = buf.line_start_offset(1).unwrap(); assert_eq!(cursor_column(&mut buf, line_start), 0); } + + /// Invariant of the prompt-input horizontal scroll window (issue #2876): + /// for any viewport width > 0 the cursor always lands inside the + /// viewport (`cursor - scroll < width`), scrolling never overshoots the + /// cursor, and no scrolling happens while the cursor already fits. + #[test] + fn test_input_hscroll_keeps_cursor_visible() { + for width in 1usize..=120 { + for cursor in 0usize..=200 { + let scroll = input_hscroll(cursor, width); + assert!(scroll <= cursor, "scroll {scroll} > cursor {cursor}"); + assert!( + cursor - scroll < width, + "cursor not visible: cursor={cursor} scroll={scroll} width={width}" + ); + if cursor < width { + assert_eq!( + scroll, 0, + "no scroll needed at cursor={cursor} width={width}" + ); + } + } + } + // Degenerate zero-width viewport must not underflow. + assert_eq!(input_hscroll(50, 0), 0); + } + + /// Reproducer for issue #2876 at the renderer level: with input wider + /// than the prompt line, the tail must scroll into view and the + /// terminal cursor must always be placed (the old renderer clipped the + /// paragraph at the right edge and skipped `set_cursor_position` + /// whenever the cursor's logical column was past it). + #[test] + fn test_render_prompt_scrolls_long_input_and_places_cursor() { + use ratatui::backend::{Backend, TestBackend}; + use ratatui::Terminal; + + let theme = + crate::view::theme::Theme::load_builtin(crate::view::theme::THEME_DARK).unwrap(); + let mut prompt = Prompt::new( + "Search: ".to_string(), + crate::view::prompt::PromptType::Search, + ); + // 100 chars in an 80-column line: 8 label cols + 72 input cols. + let input: String = ('a'..='z').cycle().take(100).collect(); + prompt.input = input.clone(); + prompt.cursor_pos = prompt.input.len(); + + let width: u16 = 80; + let backend = TestBackend::new(width, 1); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|frame| { + let area = Rect::new(0, 0, width, 1); + StatusBarRenderer::render_prompt(frame, area, &prompt, &theme); + }) + .unwrap(); + + // The label stays anchored and the *tail* of the input is visible. + let buffer = terminal.backend().buffer().clone(); + let row: String = (0..width).map(|x| buffer[(x, 0)].symbol()).collect(); + assert!( + row.starts_with("Search: "), + "label must stay visible: {row:?}" + ); + // Scroll = 100 - 71 = 29 cells, so chars 29.. are visible and the + // last column is left free for the cursor. + let tail: String = input.chars().skip(100 - 71).collect(); + assert!( + row.trim_end().ends_with(&tail), + "tail of the input must scroll into view: {row:?}" + ); + // Cursor rides the last column instead of being skipped. + let cursor = terminal.backend_mut().get_cursor_position().unwrap(); + assert_eq!((cursor.x, cursor.y), (width - 1, 0)); + + // Move the cursor 15 chars left: still visible (pinned to the last + // column), and the window follows it leftward. + prompt.cursor_pos -= 15; + terminal + .draw(|frame| { + let area = Rect::new(0, 0, width, 1); + StatusBarRenderer::render_prompt(frame, area, &prompt, &theme); + }) + .unwrap(); + let buffer = terminal.backend().buffer().clone(); + let row: String = (0..width).map(|x| buffer[(x, 0)].symbol()).collect(); + let shifted_window: String = input.chars().skip(85 - 71).take(72).collect(); + assert!( + row.ends_with(&shifted_window[shifted_window.len() - 20..]), + "window must shift with the cursor: {row:?}" + ); + let cursor = terminal.backend_mut().get_cursor_position().unwrap(); + assert_eq!((cursor.x, cursor.y), (width - 1, 0)); + + // With a short input nothing scrolls and the cursor sits right + // after the typed text. + prompt.input = "abc".to_string(); + prompt.cursor_pos = 3; + terminal + .draw(|frame| { + let area = Rect::new(0, 0, width, 1); + StatusBarRenderer::render_prompt(frame, area, &prompt, &theme); + }) + .unwrap(); + let cursor = terminal.backend_mut().get_cursor_position().unwrap(); + assert_eq!((cursor.x, cursor.y), (8 + 3, 0)); + } } diff --git a/crates/fresh-editor/src/view/ui/suggestions.rs b/crates/fresh-editor/src/view/ui/suggestions.rs index 46cae38c51..4a4a8912df 100644 --- a/crates/fresh-editor/src/view/ui/suggestions.rs +++ b/crates/fresh-editor/src/view/ui/suggestions.rs @@ -3,6 +3,7 @@ use crate::input::commands::{CommandSource, Suggestion}; use crate::primitives::display_width::{char_width, str_width}; use crate::view::prompt::Prompt; +use crate::view::ui::scrollbar::{render_scrollbar, ScrollbarColors, ScrollbarState}; use ratatui::layout::Rect; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; @@ -72,9 +73,10 @@ impl SuggestionsRenderer { // The scroll position is owned by the Prompt itself and only adjusted // when the selection moves out of the viewport (see - // `Prompt::ensure_selected_visible`, called once before render). This - // keeps a stable list under the cursor so a click near the bottom - // doesn't trigger a recenter that shifts items mid-double-click. + // `Prompt::ensure_selected_visible_within`, called once before + // render). This keeps a stable list under the cursor so a click near + // the bottom doesn't trigger a recenter that shifts items + // mid-double-click. let (start_idx, end_idx) = visible_range(prompt, visible_count); let visible_suggestions = &prompt.suggestions[start_idx..end_idx]; let layout = ColumnLayout::compute(visible_suggestions, available_width); @@ -108,6 +110,27 @@ impl SuggestionsRenderer { if draw { let paragraph = Paragraph::new(lines).block(block); frame.render_widget(paragraph, area); + + // When the list overflows the viewport, draw a scrollbar over + // the right border so the user can tell more entries exist and + // how far through them the viewport is (issues #623 / #1593). + // The borderless variant (floating overlay) carves its own + // scrollbar lane next to the list instead. + if with_border && prompt.suggestions.len() > visible_count && inner_area.height > 0 { + let scrollbar_area = Rect { + x: inner_area.x + inner_area.width, + y: inner_area.y, + width: 1, + height: inner_area.height, + }; + let state = ScrollbarState::new(prompt.suggestions.len(), visible_count, start_idx); + render_scrollbar( + frame, + scrollbar_area, + &state, + &ScrollbarColors::from_theme(theme), + ); + } } // Return area info for mouse hit testing @@ -832,6 +855,59 @@ mod tests { ); } + /// When the suggestion list overflows the popup, a scrollbar must be + /// drawn over the right border so the user can tell more entries exist + /// (issues #623 / #1593). The scrollbar paints background-colored + /// cells, so the border column's `│` glyphs disappear on those rows. + #[test] + fn overflowing_list_draws_scrollbar_on_right_border() { + let suggestions: Vec = (0..30) + .map(|i| Suggestion::new(format!("Command {i:02}"))) + .collect(); + let mut prompt = palette_prompt(suggestions); + prompt.selected_suggestion = Some(0); + + // 12 rows: 10 inner rows for 30 suggestions -> overflow. + let width: u16 = 60; + let rows = render_rows(&prompt, width, 12); + let border_col = (width - 1) as usize; + let scrollbar_cells = (1..11) + .filter(|&y| { + let cell: String = rows[y].chars().nth(border_col).unwrap().to_string(); + cell != "│" + }) + .count(); + assert_eq!( + scrollbar_cells, + 10, + "all inner border rows must be covered by the scrollbar track/thumb:\n{}", + rows.join("\n") + ); + } + + /// A list that fits entirely in the popup keeps its plain right border — + /// no scrollbar noise. + #[test] + fn fitting_list_keeps_plain_right_border() { + let suggestions: Vec = (0..5) + .map(|i| Suggestion::new(format!("Command {i}"))) + .collect(); + let prompt = palette_prompt(suggestions); + + let width: u16 = 60; + let rows = render_rows(&prompt, width, 12); + let border_col = (width - 1) as usize; + for y in 1..11 { + let cell: String = rows[y].chars().nth(border_col).unwrap().to_string(); + assert_eq!( + cell, + "│", + "row {y} must keep the plain border when the list fits:\n{}", + rows.join("\n") + ); + } + } + /// Test that truncation produces valid UTF-8 output #[test] fn test_truncation_preserves_valid_utf8() { diff --git a/crates/fresh-editor/tests/common/snapshots/e2e_tests__common__visual_testing__Comprehensive UI B__state_b.snap b/crates/fresh-editor/tests/common/snapshots/e2e_tests__common__visual_testing__Comprehensive UI B__state_b.snap index e917684b36..40f73f788f 100644 --- a/crates/fresh-editor/tests/common/snapshots/e2e_tests__common__visual_testing__Comprehensive UI B__state_b.snap +++ b/crates/fresh-editor/tests/common/snapshots/e2e_tests__common__visual_testing__Comprehensive UI B__state_b.snap @@ -19,16 +19,16 @@ expression: "&screen_text" ~ ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ -│ Plugin Demo: Open Help Open the editor help page (uses built-in action) welcome│ -│ Show Signature Help Show function parameter hints builtin│ -│ Shell Command (Replace) Alt+Shift+| Run shell command on buffer/selection, replace con... builtin│ -│ Search and Replace in Project Alt+A Search and replace text across all git-tracked files search_replace│ -│ Show Manual F1 Open the help manual builtin│ -│ Git Log: Close Close the git log panel git_log│ -│ Toggle Inlay Hints Show or hide LSP inlay hints (type hints, paramete... builtin│ -│ Start/Restart LSP Server Start or restart the LSP server for the current la... builtin│ -│ Copy File Path Copy the absolute path of the current buffer's fil... builtin│ -│ Git Blame: Close Close the git blame panel git_blame│ +│ Plugin Demo: Open Help Open the editor help page (uses built-in action) welcome +│ Show Signature Help Show function parameter hints builtin +│ Shell Command (Replace) Alt+Shift+| Run shell command on buffer/selection, replace con... builtin +│ Search and Replace in Project Alt+A Search and replace text across all git-tracked files search_replace +│ Show Manual F1 Open the help manual builtin +│ Git Log: Close Close the git log panel git_log +│ Toggle Inlay Hints Show or hide LSP inlay hints (type hints, paramete... builtin +│ Start/Restart LSP Server Start or restart the LSP server for the current la... builtin +│ Copy File Path Copy the absolute path of the current buffer's fil... builtin +│ Git Blame: Close Close the git blame panel git_blame └──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ file | >command | :line | #buffer >help diff --git a/crates/fresh-editor/tests/e2e/issue_2876_prompt_input_hscroll.rs b/crates/fresh-editor/tests/e2e/issue_2876_prompt_input_hscroll.rs new file mode 100644 index 0000000000..7712ec4975 --- /dev/null +++ b/crates/fresh-editor/tests/e2e/issue_2876_prompt_input_hscroll.rs @@ -0,0 +1,58 @@ +//! Reproducer for issue #2876: prompt-line inputs (search prompt, open-file +//! prompt, palette input) had no horizontal scrolling. With text longer than +//! the line, the paragraph was clipped at the right edge and +//! `set_cursor_position` was skipped entirely once the cursor's logical +//! column passed the width — the cursor vanished and the tail of the input +//! was unviewable, even though the internal cursor kept moving. +//! +//! With the fix, the input scrolls horizontally so the cursor is always +//! visible (pinned to the last column while past the right edge), and the +//! terminal cursor is always placed. + +use crate::common::harness::EditorTestHarness; +use crossterm::event::{KeyCode, KeyModifiers}; + +#[test] +fn test_search_prompt_long_input_scrolls_and_keeps_cursor_visible() { + let mut harness = EditorTestHarness::new(80, 24).unwrap(); + + // Open the search prompt and type 100 characters — far wider than the + // 80-column line. The last 10 form a unique marker so the visible + // window is observable on screen. + harness + .send_key(KeyCode::Char('f'), KeyModifiers::CONTROL) + .unwrap(); + harness.render().unwrap(); + let input = format!("{}TAILMARKER", "x".repeat(90)); + harness.type_text(&input).unwrap(); + + // The tail of the input must be scrolled into view (the old renderer + // showed only the first ~72 chars and never the tail) … + let cursor = harness.render_observing_cursor().unwrap(); + harness.assert_screen_contains("TAILMARKER"); + // … and the hardware cursor must be visible, riding the last column of + // the prompt line (the old renderer skipped `set_cursor_position`, so + // the frame ended with the cursor hidden). + assert_eq!( + cursor, + Some((79, 23)), + "cursor must be visible at the right edge of the prompt line" + ); + + // Press Left 15 times: the cursor is still inside the overflowing + // region, so it must stay visible while the window scrolls back with + // it (the old renderer showed no cursor until ~28 presses). + for _ in 0..15 { + harness.send_key(KeyCode::Left, KeyModifiers::NONE).unwrap(); + } + let cursor = harness.render_observing_cursor().unwrap(); + assert_eq!( + cursor, + Some((79, 23)), + "cursor must stay visible while moving left through clipped text" + ); + // The window now ends just after the cursor (char 85 of 100), so the + // tail marker (chars 90..100) must have scrolled off the right edge — + // proof the window follows the cursor rather than staying pinned. + harness.assert_screen_not_contains("TAILMARKER"); +} diff --git a/crates/fresh-editor/tests/e2e/issue_623_prompt_dropdown_scrollbar.rs b/crates/fresh-editor/tests/e2e/issue_623_prompt_dropdown_scrollbar.rs new file mode 100644 index 0000000000..a7134070e6 --- /dev/null +++ b/crates/fresh-editor/tests/e2e/issue_623_prompt_dropdown_scrollbar.rs @@ -0,0 +1,491 @@ +//! Reproducers for issue #623 (a suggestion dropdown that scrolls, but gives +//! no sign that entries are hidden — "at first glance, I thought only these") +//! and the remaining item of issue #1593 (the command palette rendered no +//! scrollbar at all). Both lists go through the same `SuggestionsRenderer`, +//! so one fix covers both: when the list overflows the dropdown, the shared +//! scrollbar widget is drawn over the popup's right border, and it responds +//! to clicks like every other scrollbar in the editor. + +use crate::common::harness::EditorTestHarness; +use crossterm::event::{KeyCode, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; + +/// The scrollbar sits on the popup's right border, i.e. the last column. +const SCROLLBAR_COL: u16 = 99; + +/// The popup's suggestion rows: full-width popup rows start with the `│` +/// left border glyph (the top/bottom borders start with `┌`/`└`). +fn suggestion_rows(harness: &EditorTestHarness, height: u16) -> Vec { + (0..height) + .filter(|&y| harness.get_row_text(y).starts_with('│')) + .collect() +} + +/// Where the scrollbar thumb is drawn, as `(top, size)` in rows relative to +/// the top of the track. Read from the rendered cells only — the thumb is a +/// run of background-coloured cells inside the track. +fn thumb_span(harness: &EditorTestHarness, rows: &[u16]) -> (usize, usize) { + let thumb: Vec = rows + .iter() + .enumerate() + .filter(|(_, &y)| harness.is_scrollbar_thumb_at(SCROLLBAR_COL, y)) + .map(|(i, _)| i) + .collect(); + assert!( + !thumb.is_empty(), + "expected a scrollbar thumb on the popup border:\n{}", + harness.screen_to_string() + ); + let top = thumb[0]; + assert_eq!( + thumb.last().copied(), + Some(top + thumb.len() - 1), + "thumb rows must be contiguous, got {thumb:?}" + ); + (top, thumb.len()) +} + +/// A dropdown row's text, with runs of padding collapsed: the column widths +/// are computed from the *visible* entries, so scrolling reflows them and +/// only the words identify the entry. +fn entry_text(harness: &EditorTestHarness, y: u16) -> String { + harness + .get_row_text(y) + .split_whitespace() + .collect::>() + .join(" ") +} + +/// Text of the highlighted suggestion row, or `None` when the selection has +/// scrolled out of the viewport. The selection is a background colour rather +/// than a glyph, so it's found as the one row whose background differs from +/// the rest of the list. +fn selected_row_text(harness: &EditorTestHarness, rows: &[u16]) -> Option { + use std::collections::HashMap; + let bg_at = |y: u16| harness.get_cell_style(2, y).and_then(|s| s.bg); + let mut counts: HashMap<_, usize> = HashMap::new(); + for &y in rows { + *counts.entry(bg_at(y)).or_default() += 1; + } + let unselected = counts.into_iter().max_by_key(|&(_, n)| n)?.0; + rows.iter() + .copied() + .find(|&y| bg_at(y) != unselected) + .map(|y| entry_text(harness, y)) +} + +/// Open a prompt whose suggestion list overflows the 10-row dropdown, and +/// return its suggestion rows. `command` is run through the palette; passing +/// `None` leaves the palette itself open. +fn open_overflowing_dropdown(harness: &mut EditorTestHarness, command: Option<&str>) -> Vec { + match command { + Some(name) => harness.run_palette_command(name).unwrap(), + None => harness + .send_key(KeyCode::Char('p'), KeyModifiers::CONTROL) + .unwrap(), + } + harness.render().unwrap(); + let rows = suggestion_rows(harness, 24); + assert_eq!( + rows.len(), + 10, + "dropdown must be full for the list to overflow:\n{}", + harness.screen_to_string() + ); + rows +} + +/// Issue #1593: the command palette lists far more commands than fit, so +/// every suggestion row must carry a scrollbar cell on the popup's right +/// border. Before the fix that column was a plain `│` frame line, so nothing +/// signalled that the list continued. +#[test] +fn test_command_palette_overflow_draws_scrollbar() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + + harness + .send_key(KeyCode::Char('p'), KeyModifiers::CONTROL) + .unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("Add Cursor Above"); + + let rows = suggestion_rows(&harness, 24); + assert!(!rows.is_empty(), "command palette must render suggestions"); + + let scrollbar_col = 99; + let scrollbar_cells = rows + .iter() + .filter(|&&y| { + harness.is_scrollbar_thumb_at(scrollbar_col, y) + || harness.is_scrollbar_track_at(scrollbar_col, y) + }) + .count(); + assert_eq!( + scrollbar_cells, + rows.len(), + "every suggestion row must carry a scrollbar track/thumb cell on the right border" + ); +} + +/// Issue #623 verbatim: the Select Locale dropdown shows 10 of the 14 +/// locales. It scrolled even before the fix — what was missing was any +/// indication that more entries existed, which is now the right-border +/// scrollbar. +#[test] +fn test_select_locale_dropdown_shows_scrollbar_for_hidden_entries() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + + harness + .send_key(KeyCode::Char('p'), KeyModifiers::CONTROL) + .unwrap(); + harness.type_text("Select Locale").unwrap(); + harness + .send_key(KeyCode::Enter, KeyModifiers::NONE) + .unwrap(); + harness.render().unwrap(); + + // The locale list is open: 14 locales, 10 rows of room, so 4 are hidden. + let rows = suggestion_rows(&harness, 24); + assert_eq!( + rows.len(), + 10, + "locale dropdown must fill its 10 rows with the 14 locales" + ); + + let scrollbar_col = 99; + let scrollbar_cells = rows + .iter() + .filter(|&&y| { + harness.is_scrollbar_thumb_at(scrollbar_col, y) + || harness.is_scrollbar_track_at(scrollbar_col, y) + }) + .count(); + assert_eq!( + scrollbar_cells, + rows.len(), + "the hidden locales must be signalled by a scrollbar on the right border" + ); +} + +/// A list that fits the dropdown keeps its plain `│` right border — the +/// indicator only appears when entries are actually hidden. +#[test] +fn test_short_suggestion_list_shows_no_scrollbar() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + + harness + .send_key(KeyCode::Char('p'), KeyModifiers::CONTROL) + .unwrap(); + harness.type_text("Toggle Line Wrap").unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("Toggle Line Wrap"); + + let rows = suggestion_rows(&harness, 24); + assert!( + !rows.is_empty() && rows.len() < 10, + "filtered list should fit the dropdown, got {} rows", + rows.len() + ); + for &y in &rows { + assert!( + harness.get_row_text(y).trim_end().ends_with('│'), + "row {y} must keep its plain right border when the list fits" + ); + } +} + +/// Issue #1593 asked for a palette scrollbar that behaves like the editor's: +/// clicking low on the track must jump the list there and keep it there +/// (the renderer's keep-selection-visible pass must not yank it back). +#[test] +fn test_clicking_palette_scrollbar_scrolls_the_list() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + + harness + .send_key(KeyCode::Char('p'), KeyModifiers::CONTROL) + .unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("Add Cursor Above"); + + let rows = suggestion_rows(&harness, 24); + let bottom_row = *rows.last().expect("popup must have suggestion rows"); + + // Click the bottom of the scrollbar track: the list must jump towards + // the end, so the alphabetically-first command scrolls out of view. + harness.mouse_click(99, bottom_row).unwrap(); + harness.render().unwrap(); + + harness.assert_screen_not_contains("Add Cursor Above"); +} + +/// Clicking track row R must leave the thumb's top *on* row R — the thumb +/// goes where you point it. The mapping used to divide the click position by +/// the full track height instead of by the thumb's actual travel, so every +/// click landed the thumb a row (or more) above the row clicked, and the +/// bottom row of the track could never reach the end of the list. +#[test] +fn test_clicking_palette_scrollbar_lands_thumb_on_the_clicked_row() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, None); + + let (_, thumb_size) = thumb_span(&harness, &rows); + // Rows below `max_thumb_top` can't hold the thumb's top — the thumb + // would hang off the end of the track — so they clamp to the bottom. + let max_thumb_top = rows.len() - thumb_size; + + for target in 0..=max_thumb_top { + harness.mouse_click(SCROLLBAR_COL, rows[target]).unwrap(); + let (top, size) = thumb_span(&harness, &rows); + assert_eq!( + top, + target, + "clicking track row {target} put the thumb at row {top}:\n{}", + harness.screen_to_string() + ); + assert_eq!( + size, thumb_size, + "thumb size must not change while scrolling" + ); + } + + // Both extremes, explicitly: the last row of the track scrolls to the + // very end of the list, the first row back to the very start. + harness + .mouse_click(SCROLLBAR_COL, *rows.last().unwrap()) + .unwrap(); + let (top, size) = thumb_span(&harness, &rows); + assert_eq!( + top + size, + rows.len(), + "clicking the last track row must park the thumb at the bottom:\n{}", + harness.screen_to_string() + ); + harness.assert_screen_not_contains("Add Cursor Above"); + + harness.mouse_click(SCROLLBAR_COL, rows[0]).unwrap(); + assert_eq!( + thumb_span(&harness, &rows).0, + 0, + "clicking the first track row must return to the top of the list" + ); + harness.assert_screen_contains("Add Cursor Above"); +} + +/// The drag follow-up uses the same mapping as the press, so the thumb keeps +/// tracking the cursor row-for-row while the button is held. Driven as one +/// continuous gesture — press once, walk the cursor down the track and back +/// up — so it exercises the drag handler rather than a series of clicks. +#[test] +fn test_dragging_palette_scrollbar_tracks_the_cursor_row() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, None); + + let (_, thumb_size) = thumb_span(&harness, &rows); + let max_thumb_top = rows.len() - thumb_size; + + let press = |h: &mut EditorTestHarness, y: u16| { + h.send_mouse(MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: SCROLLBAR_COL, + row: y, + modifiers: KeyModifiers::empty(), + }) + .unwrap(); + }; + let drag_to = |h: &mut EditorTestHarness, y: u16| { + h.send_mouse(MouseEvent { + kind: MouseEventKind::Drag(MouseButton::Left), + column: SCROLLBAR_COL, + row: y, + modifiers: KeyModifiers::empty(), + }) + .unwrap(); + h.render().unwrap(); + }; + let release = |h: &mut EditorTestHarness, y: u16| { + h.send_mouse(MouseEvent { + kind: MouseEventKind::Up(MouseButton::Left), + column: SCROLLBAR_COL, + row: y, + modifiers: KeyModifiers::empty(), + }) + .unwrap(); + h.render().unwrap(); + }; + + press(&mut harness, rows[0]); + let sweep: Vec = (0..=max_thumb_top) + .chain((0..max_thumb_top).rev()) + .collect(); + for target in sweep { + drag_to(&mut harness, rows[target]); + assert_eq!( + thumb_span(&harness, &rows).0, + target, + "dragging to track row {target} left the thumb elsewhere:\n{}", + harness.screen_to_string() + ); + } + release(&mut harness, rows[0]); +} + +/// The mouse wheel scrolls the VIEW only: it must never move the selection. +/// +/// Wheeling used to walk `selected_suggestion` instead, which rewrote the +/// prompt input under the user and — once a scrollbar click could pin the +/// viewport — made the list visibly jump, because the wheel released that +/// pin and the renderer snapped back to a selection that had not moved on +/// screen. Asserted on the palette, which is the surface the report used. +#[test] +fn test_wheel_over_palette_scrolls_view_without_moving_selection() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, None); + + // Put the selection a few rows down so it stays on screen after the + // wheel — that's what makes "the same entry is still highlighted" an + // observation rather than a vacuous absence. + for _ in 0..3 { + harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + } + harness.render().unwrap(); + let selected_before = selected_row_text(&harness, &rows).expect("an entry must be highlighted"); + let first_before = entry_text(&harness, rows[0]); + + harness.mouse_scroll_down(50, rows[5]).unwrap(); + + assert_ne!( + entry_text(&harness, rows[0]), + first_before, + "the wheel must scroll the list:\n{}", + harness.screen_to_string() + ); + assert_eq!( + selected_row_text(&harness, &rows).as_deref(), + Some(selected_before.as_str()), + "the wheel must leave the selection on the same entry:\n{}", + harness.screen_to_string() + ); + + // Scrolling back restores the view, selection still untouched. + harness.mouse_scroll_up(50, rows[5]).unwrap(); + assert_eq!(entry_text(&harness, rows[0]), first_before); + assert_eq!( + selected_row_text(&harness, &rows).as_deref(), + Some(selected_before.as_str()) + ); +} + +/// Same rule on the Select Locale picker (issue #623's own dropdown): a +/// different `PromptType`, the same shared suggestions renderer. +#[test] +fn test_wheel_over_select_locale_does_not_move_selection() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, Some("Select Locale")); + + for _ in 0..2 { + harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + } + harness.render().unwrap(); + let selected_before = selected_row_text(&harness, &rows).expect("a locale must be highlighted"); + let first_before = entry_text(&harness, rows[0]); + + harness.mouse_scroll_down(50, rows[5]).unwrap(); + + assert_ne!( + entry_text(&harness, rows[0]), + first_before, + "the wheel must scroll the locale list:\n{}", + harness.screen_to_string() + ); + assert_eq!( + selected_row_text(&harness, &rows).as_deref(), + Some(selected_before.as_str()), + "the wheel must leave the selected locale alone:\n{}", + harness.screen_to_string() + ); +} + +/// And on a third prompt-driven list — Set Language, whose suggestions come +/// from the grammar catalogue rather than from the command registry. +#[test] +fn test_wheel_over_set_language_does_not_move_selection() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, Some("Set Language")); + + for _ in 0..3 { + harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + } + harness.render().unwrap(); + let selected_before = + selected_row_text(&harness, &rows).expect("a language must be highlighted"); + let first_before = entry_text(&harness, rows[0]); + + harness.mouse_scroll_down(50, rows[5]).unwrap(); + + assert_ne!( + entry_text(&harness, rows[0]), + first_before, + "the wheel must scroll the language list:\n{}", + harness.screen_to_string() + ); + assert_eq!( + selected_row_text(&harness, &rows).as_deref(), + Some(selected_before.as_str()), + "the wheel must leave the selected language alone:\n{}", + harness.screen_to_string() + ); +} + +/// The wheel may scroll the selection clean off the list — that's correct +/// (VS Code does the same), and the selection must survive it: scrolling +/// back brings the very same entry back, still highlighted. +#[test] +fn test_wheel_may_scroll_selection_out_of_view_without_losing_it() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, None); + + let selected_before = selected_row_text(&harness, &rows).expect("an entry must be highlighted"); + + // Four notches of 3 rows: the first entry is far off the top now. + for _ in 0..4 { + harness.mouse_scroll_down(50, rows[5]).unwrap(); + } + assert_eq!( + selected_row_text(&harness, &rows), + None, + "the selection scrolled out of view, so no row is highlighted:\n{}", + harness.screen_to_string() + ); + + for _ in 0..4 { + harness.mouse_scroll_up(50, rows[5]).unwrap(); + } + assert_eq!( + selected_row_text(&harness, &rows).as_deref(), + Some(selected_before.as_str()), + "scrolling back must reveal the same selected entry:\n{}", + harness.screen_to_string() + ); +} + +/// Keyboard navigation still re-engages keep-the-selection-visible +/// scrolling: after wheeling the selection off screen, an arrow key brings +/// the view back to it. This is what keeps the manual-scroll latch honest +/// now that the wheel no longer touches the selection. +#[test] +fn test_arrow_key_after_wheel_brings_the_selection_back_into_view() { + let mut harness = EditorTestHarness::new(100, 24).unwrap(); + let rows = open_overflowing_dropdown(&mut harness, None); + + for _ in 0..4 { + harness.mouse_scroll_down(50, rows[5]).unwrap(); + } + assert_eq!(selected_row_text(&harness, &rows), None); + + harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + harness.render().unwrap(); + assert!( + selected_row_text(&harness, &rows).is_some(), + "an arrow key must scroll the selection back into view:\n{}", + harness.screen_to_string() + ); +} diff --git a/crates/fresh-editor/tests/e2e/mod.rs b/crates/fresh-editor/tests/e2e/mod.rs index dfd4c2e107..d3131a6bdb 100644 --- a/crates/fresh-editor/tests/e2e/mod.rs +++ b/crates/fresh-editor/tests/e2e/mod.rs @@ -100,8 +100,10 @@ pub mod issue_2796_key_release_duplicates; #[cfg(feature = "plugins")] pub mod issue_2810_session_escape_flush; pub mod issue_2843_single_line_viewport; +pub mod issue_2876_prompt_input_hscroll; pub mod issue_2878_split_cursor_independence; pub mod issue_2893_replace_all_many_matches; +pub mod issue_623_prompt_dropdown_scrollbar; pub mod issue_779_after_eof_shade; pub mod issue_close_file_in_split_hides_buffer_group; pub mod language_dialog_esc_cancels_edit;