diff --git a/CHANGELOG.md b/CHANGELOG.md index 74b9d71..15daa54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added ZIP archive creation for the focused item or current selection, with a configurable `create_archive` (`C`) shortcut, editable archive name, background progress, and cancellation. ([#215]) +### Changed + +- Made create and bulk rename overlays show more rows before scrolling, adapt better on short terminals, and use consistent scrollbar styling. + ## [1.10.0] - 2026-06-30 ### Added diff --git a/src/ui/browser/tests.rs b/src/ui/browser/tests.rs index efd43a8..85f7465 100644 --- a/src/ui/browser/tests.rs +++ b/src/ui/browser/tests.rs @@ -806,12 +806,12 @@ fn create_overlay_scrolls_to_keep_the_active_line_visible() { app.handle_event(Event::Key(KeyEvent::from(KeyCode::Char('a')))) .expect("create overlay should open"); - for index in 0..10 { + for index in 0..14 { for ch in format!("file-{index:02}.txt").chars() { app.handle_event(Event::Key(KeyEvent::from(KeyCode::Char(ch)))) .expect("typing create line should succeed"); } - if index < 9 { + if index < 13 { app.handle_event(Event::Key(KeyEvent::new( KeyCode::Char('j'), KeyModifiers::CONTROL, @@ -827,7 +827,7 @@ fn create_overlay_scrolls_to_keep_the_active_line_visible() { assert_eq!( state.create_scroll_top, 2, - "create overlay should scroll once the cursor moves past the eighth visible line" + "create overlay should scroll once the cursor moves past the twelfth visible line" ); assert!( rect_row_text(terminal.backend().buffer(), list_area, list_area.y).contains("file-02.txt"), @@ -841,7 +841,7 @@ fn create_overlay_scrolls_to_keep_the_active_line_visible() { .y .saturating_add(list_area.height.saturating_sub(1)), ) - .contains("file-09.txt"), + .contains("file-13.txt"), "expected the active create line to remain visible at the bottom of the list" ); @@ -852,7 +852,7 @@ fn create_overlay_scrolls_to_keep_the_active_line_visible() { fn bulk_rename_overlay_scrolls_to_keep_the_active_row_visible() { let root = temp_path("bulk-rename-overlay-scroll"); fs::create_dir_all(&root).expect("failed to create temp root"); - for index in 0..10 { + for index in 0..14 { fs::write(root.join(format!("file-{index:02}.txt")), "content") .expect("failed to write test file"); } @@ -861,13 +861,13 @@ fn bulk_rename_overlay_scrolls_to_keep_the_active_row_visible() { app.navigation.view_mode = crate::app::ViewMode::List; let mut terminal = Terminal::new(TestBackend::new(90, 24)).expect("terminal should init"); - for _ in 0..10 { + for _ in 0..14 { app.handle_event(Event::Key(KeyEvent::from(KeyCode::Char(' ')))) .expect("selection toggle should succeed"); } app.handle_event(Event::Key(KeyEvent::from(KeyCode::Char('r')))) .expect("bulk rename overlay should open"); - for _ in 0..9 { + for _ in 0..13 { app.handle_event(Event::Key(KeyEvent::from(KeyCode::Down))) .expect("bulk rename cursor movement should succeed"); } @@ -883,7 +883,7 @@ fn bulk_rename_overlay_scrolls_to_keep_the_active_row_visible() { ); assert_eq!( state.bulk_rename_scroll_top, 2, - "bulk rename overlay should scroll once the active row moves past the eighth visible line" + "bulk rename overlay should scroll once the active row moves past the twelfth visible line" ); assert!( rect_row_text(terminal.backend().buffer(), list_area, list_area.y).contains("file-02.txt"), @@ -897,7 +897,7 @@ fn bulk_rename_overlay_scrolls_to_keep_the_active_row_visible() { .y .saturating_add(list_area.height.saturating_sub(1)), ) - .contains("file-09.txt"), + .contains("file-13.txt"), "expected the active bulk rename row to remain visible at the bottom of the list" ); diff --git a/src/ui/overlay_manager/archive_create.rs b/src/ui/overlay_manager/archive_create.rs index 8e2e765..fad00dd 100644 --- a/src/ui/overlay_manager/archive_create.rs +++ b/src/ui/overlay_manager/archive_create.rs @@ -1,3 +1,4 @@ +use super::{edit_overlay_visible_rows, scrollbar::render_overlay_scrollbar_on_bg}; use crate::app::{App, FrameState}; use crate::ui::{ helpers, @@ -11,8 +12,6 @@ use ratatui::{ widgets::{Clear, Paragraph}, }; -const MAX_VISIBLE_CONTENTS: usize = 8; - pub(super) fn render_archive_create_overlay( frame: &mut Frame<'_>, area: Rect, @@ -21,8 +20,8 @@ pub(super) fn render_archive_create_overlay( palette: Palette, ) { let item_count = app.archive_create_source_names().len(); - let visible_lines = item_count.min(MAX_VISIBLE_CONTENTS) as u16; let has_error = app.archive_create_error().is_some(); + let visible_lines = edit_overlay_visible_rows(area, item_count, 7 + u16::from(has_error)); let popup_width = area.width.saturating_sub(8).clamp(40, 68); let popup_height = visible_lines + 7 + u16::from(has_error); let popup = helpers::centered_rect(area, popup_width, popup_height); @@ -122,10 +121,15 @@ fn render_contents_list( state.archive_create_list_area = Some(list_area); let source_names = app.archive_create_source_names(); let show_scrollbar = source_names.len() > visible_lines; + let scrollbar_area = show_scrollbar.then_some(Rect { + x: list_area.x + list_area.width.saturating_sub(1), + width: 1, + ..list_area + }); let scroll_top = app.archive_create_source_scroll(visible_lines); let row_width = list_area .width - .saturating_sub(if show_scrollbar { 2 } else { 0 }); + .saturating_sub(if show_scrollbar { 1 } else { 0 }); for (row_offset, name) in source_names .iter() @@ -162,26 +166,15 @@ fn render_contents_list( ); } - if show_scrollbar { - let bar_x = list_area.x + list_area.width.saturating_sub(1); - let thumb_size = (visible_lines * visible_lines / source_names.len()).max(1); - let max_scroll = source_names.len().saturating_sub(visible_lines); - let thumb_top = if max_scroll == 0 || visible_lines <= thumb_size { - 0 - } else { - scroll_top * (visible_lines - thumb_size) / max_scroll - }; - for row_offset in 0..visible_lines { - let in_thumb = row_offset >= thumb_top && row_offset < thumb_top + thumb_size; - let bar_char = if in_thumb { "▐" } else { " " }; - let bar_color = if in_thumb { - palette.muted - } else { - palette.path_bg - }; - frame.buffer_mut()[(bar_x, list_area.y + row_offset as u16)].set_symbol(bar_char); - frame.buffer_mut()[(bar_x, list_area.y + row_offset as u16)] - .set_style(Style::default().bg(palette.path_bg).fg(bar_color)); - } + if let Some(scrollbar_area) = scrollbar_area { + render_overlay_scrollbar_on_bg( + frame, + scrollbar_area, + source_names.len(), + visible_lines, + scroll_top, + palette, + palette.path_bg, + ); } } diff --git a/src/ui/overlay_manager/bulk_rename.rs b/src/ui/overlay_manager/bulk_rename.rs index 22251d2..1c82ca6 100644 --- a/src/ui/overlay_manager/bulk_rename.rs +++ b/src/ui/overlay_manager/bulk_rename.rs @@ -1,4 +1,6 @@ -use super::compute_scroll_top; +use super::{ + compute_scroll_top, edit_overlay_visible_rows, scrollbar::render_overlay_scrollbar_on_bg, +}; use crate::app::{App, FrameState}; use crate::ui::{ helpers, @@ -20,7 +22,7 @@ pub(super) fn render_bulk_rename_overlay( palette: Palette, ) { let item_count = app.bulk_rename_item_count(); - let visible_lines = item_count.min(8) as u16; + let visible_lines = edit_overlay_visible_rows(area, item_count, 5); let popup_width = area.width.saturating_sub(8).clamp(40, 68); let popup_height = visible_lines + 5; let popup = helpers::centered_rect(area, popup_width, popup_height); @@ -59,17 +61,11 @@ pub(super) fn render_bulk_rename_overlay( state.bulk_rename_scroll_top = scroll_top; let show_scrollbar = item_count > visible_lines as usize; - let thumb_size = if show_scrollbar { - (visible_lines as usize * visible_lines as usize / item_count).max(1) - } else { - 0 - }; - let max_scroll = item_count.saturating_sub(visible_lines as usize); - let thumb_pos = scroll_top - .checked_mul(visible_lines as usize - thumb_size) - .and_then(|offset| offset.checked_div(max_scroll)) - .unwrap_or(0); - let bar_x = list_area.x + list_area.width.saturating_sub(1); + let scrollbar_area = show_scrollbar.then_some(Rect { + x: list_area.x + list_area.width.saturating_sub(1), + width: 1, + ..list_area + }); let mut cursor_screen_pos: Option<(u16, u16)> = None; @@ -93,7 +89,7 @@ pub(super) fn render_bulk_rename_overlay( let text_width = list_area .width .saturating_sub(prefix_width) - .saturating_sub(if show_scrollbar { 2 } else { 0 }); + .saturating_sub(if show_scrollbar { 1 } else { 0 }); let col = if is_cursor_line { cursor_col } else { 0 }; let (visible_text, visible_col) = helpers::input_window(new_name, col, text_width); @@ -107,7 +103,7 @@ pub(super) fn render_bulk_rename_overlay( let row_width = list_area .width - .saturating_sub(if show_scrollbar { 2 } else { 0 }); + .saturating_sub(if show_scrollbar { 1 } else { 0 }); let row_rect = Rect { x: list_area.x, y: list_area.y + row_offset as u16, @@ -127,20 +123,6 @@ pub(super) fn render_bulk_rename_overlay( row_rect, ); - if show_scrollbar { - let y = list_area.y + row_offset as u16; - let in_thumb = row_offset >= thumb_pos && row_offset < thumb_pos + thumb_size; - let bar_char = if in_thumb { "▐" } else { " " }; - let bar_color = if in_thumb { - palette.muted - } else { - palette.path_bg - }; - frame.buffer_mut()[(bar_x, y)].set_symbol(bar_char); - frame.buffer_mut()[(bar_x, y)] - .set_style(Style::default().bg(palette.path_bg).fg(bar_color)); - } - if is_cursor_line { let cursor_x = (row_rect.x + prefix_width + visible_col) .min(row_rect.x + row_rect.width.saturating_sub(1)); @@ -152,6 +134,18 @@ pub(super) fn render_bulk_rename_overlay( frame.set_cursor_position((cx, cy)); } + if let Some(scrollbar_area) = scrollbar_area { + render_overlay_scrollbar_on_bg( + frame, + scrollbar_area, + item_count, + visible_lines as usize, + scroll_top, + palette, + palette.path_bg, + ); + } + if let Some(error) = app.bulk_rename_line_error(cursor_line) { frame.render_widget( Paragraph::new(Line::from(vec![Span::styled( diff --git a/src/ui/overlay_manager/create.rs b/src/ui/overlay_manager/create.rs index c669d5e..565f3eb 100644 --- a/src/ui/overlay_manager/create.rs +++ b/src/ui/overlay_manager/create.rs @@ -1,4 +1,6 @@ -use super::compute_scroll_top; +use super::{ + compute_scroll_top, edit_overlay_visible_rows, scrollbar::render_overlay_scrollbar_on_bg, +}; use crate::app::{App, FrameState}; use crate::ui::{ helpers, @@ -20,7 +22,7 @@ pub(super) fn render_create_overlay( palette: Palette, ) { let line_count = app.create_line_count().max(1); - let visible_lines = line_count.min(8) as u16; + let visible_lines = edit_overlay_visible_rows(area, line_count, 5); let popup_width = area.width.saturating_sub(8).clamp(36, 64); let popup_height = visible_lines + 5; let popup = helpers::centered_rect(area, popup_width, popup_height); @@ -58,17 +60,11 @@ pub(super) fn render_create_overlay( state.create_scroll_top = scroll_top; let show_scrollbar = line_count > visible_lines as usize; - let thumb_size = if show_scrollbar { - (visible_lines as usize * visible_lines as usize / line_count).max(1) - } else { - 0 - }; - let max_scroll = line_count.saturating_sub(visible_lines as usize); - let thumb_pos = scroll_top - .checked_mul(visible_lines as usize - thumb_size) - .and_then(|offset| offset.checked_div(max_scroll)) - .unwrap_or(0); - let bar_x = list_area.x + list_area.width.saturating_sub(1); + let scrollbar_area = show_scrollbar.then_some(Rect { + x: list_area.x + list_area.width.saturating_sub(1), + width: 1, + ..list_area + }); let mut cursor_screen_pos: Option<(u16, u16)> = None; @@ -100,7 +96,7 @@ pub(super) fn render_create_overlay( let text_width = list_area .width .saturating_sub(prefix_width) - .saturating_sub(if show_scrollbar { 2 } else { 0 }); + .saturating_sub(if show_scrollbar { 1 } else { 0 }); let col = if is_cursor_line { cursor_col } else { 0 }; let (visible_text, visible_col) = helpers::input_window(line_text, col, text_width); @@ -137,7 +133,7 @@ pub(super) fn render_create_overlay( y: list_area.y + row_offset as u16, width: list_area .width - .saturating_sub(if show_scrollbar { 2 } else { 0 }), + .saturating_sub(if show_scrollbar { 1 } else { 0 }), height: 1, }; frame.render_widget( @@ -146,20 +142,6 @@ pub(super) fn render_create_overlay( row_rect, ); - if show_scrollbar { - let y = list_area.y + row_offset as u16; - let in_thumb = row_offset >= thumb_pos && row_offset < thumb_pos + thumb_size; - let bar_char = if in_thumb { "▐" } else { " " }; - let bar_color = if in_thumb { - palette.muted - } else { - palette.path_bg - }; - frame.buffer_mut()[(bar_x, y)].set_symbol(bar_char); - frame.buffer_mut()[(bar_x, y)] - .set_style(Style::default().bg(palette.path_bg).fg(bar_color)); - } - if is_cursor_line { let cursor_x = row_rect.x + prefix_width + visible_col; let cursor_x = cursor_x.min(row_rect.x + row_rect.width.saturating_sub(1)); @@ -171,6 +153,18 @@ pub(super) fn render_create_overlay( frame.set_cursor_position((cx, cy)); } + if let Some(scrollbar_area) = scrollbar_area { + render_overlay_scrollbar_on_bg( + frame, + scrollbar_area, + line_count, + visible_lines as usize, + scroll_top, + palette, + palette.path_bg, + ); + } + if let Some(error) = app.create_line_error(cursor_line) { frame.render_widget( Paragraph::new(Line::from(vec![Span::styled( diff --git a/src/ui/overlay_manager/mod.rs b/src/ui/overlay_manager/mod.rs index dc85975..4151dac 100644 --- a/src/ui/overlay_manager/mod.rs +++ b/src/ui/overlay_manager/mod.rs @@ -16,6 +16,8 @@ mod scrollbar; mod search; mod trash; +const MAX_EDIT_OVERLAY_VISIBLE_ROWS: usize = 12; + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(super) enum HelpMode { Normal, @@ -154,3 +156,29 @@ fn compute_scroll_top(cursor_line: usize, visible: usize) -> usize { cursor_line - visible + 1 } } + +fn edit_overlay_visible_rows(area: Rect, row_count: usize, popup_chrome_height: u16) -> u16 { + let available_rows = area + .height + .saturating_sub(popup_chrome_height.saturating_add(2)) + .max(1) as usize; + row_count + .clamp(1, MAX_EDIT_OVERLAY_VISIBLE_ROWS) + .min(available_rows) as u16 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn edit_overlay_visible_rows_caps_and_shrinks_to_terminal_height() { + let area = Rect::new(0, 0, 90, 24); + assert_eq!(edit_overlay_visible_rows(area, 40, 5), 12); + + let short_area = Rect::new(0, 0, 90, 10); + assert_eq!(edit_overlay_visible_rows(short_area, 40, 5), 3); + assert_eq!(edit_overlay_visible_rows(short_area, 40, 7), 1); + assert_eq!(edit_overlay_visible_rows(short_area, 0, 5), 1); + } +} diff --git a/src/ui/overlay_manager/scrollbar.rs b/src/ui/overlay_manager/scrollbar.rs index 8162777..ce774a1 100644 --- a/src/ui/overlay_manager/scrollbar.rs +++ b/src/ui/overlay_manager/scrollbar.rs @@ -2,7 +2,7 @@ use crate::ui::theme::Palette; use ratatui::{ Frame, layout::Rect, - style::{Modifier, Style}, + style::{Color, Modifier, Style}, text::{Line, Span}, widgets::Paragraph, }; @@ -14,10 +14,30 @@ pub(super) fn render_overlay_scrollbar( visible_rows: usize, scroll_row: usize, palette: Palette, +) { + render_overlay_scrollbar_on_bg( + frame, + area, + total_rows, + visible_rows, + scroll_row, + palette, + palette.chrome_alt, + ); +} + +pub(super) fn render_overlay_scrollbar_on_bg( + frame: &mut Frame<'_>, + area: Rect, + total_rows: usize, + visible_rows: usize, + scroll_row: usize, + palette: Palette, + bg: Color, ) { if area.height == 0 || total_rows <= visible_rows.max(1) { frame.render_widget( - Paragraph::new(" ").style(Style::default().bg(palette.chrome_alt).fg(palette.border)), + Paragraph::new(" ").style(Style::default().bg(bg).fg(palette.border)), area, ); return; @@ -31,7 +51,7 @@ pub(super) fn render_overlay_scrollbar( )); area.height as usize ]) - .style(Style::default().bg(palette.chrome_alt)), + .style(Style::default().bg(bg)), area, ); @@ -55,7 +75,7 @@ pub(super) fn render_overlay_scrollbar( )); thumb_height ]) - .style(Style::default().bg(palette.chrome_alt)), + .style(Style::default().bg(bg)), Rect { x: area.x, y: area.y + thumb_top as u16,