diff --git a/Cargo.toml b/Cargo.toml index cedb8ea..a9d7b5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ regex = "1.13" reqwest = { version = "0.13", default-features = false, features = ["blocking", "rustls"] } rqrr = { version = "0.10", default-features = false } rpassword = "7.5" -ratatui = { version = "0.30", default-features = false, features = ["crossterm_0_29", "layout-cache", "macros", "underline-color"] } +ratatui = { version = "0.30", default-features = false, features = ["crossterm_0_29", "layout-cache", "macros", "underline-color", "unstable-rendered-line-info"] } security-framework = "3.7" secret-service = { version = "5.1", default-features = false, features = ["rt-tokio-crypto-rust"] } serde = { version = "1", features = ["derive"] } diff --git a/apps/tui/COMMANDS.md b/apps/tui/COMMANDS.md index 82f929a..15df53f 100644 --- a/apps/tui/COMMANDS.md +++ b/apps/tui/COMMANDS.md @@ -19,7 +19,7 @@ Secret-bearing commands are masked and omitted from history. | --- | --- | --- | | Global | `?`, `:`, `C-l`/`C-z`, `q` | help, command prompt, secure lock, quit | | Tree | `j`/`k`, arrows, `h`/`l`, `Enter`, `/`, `n`/`N` | move, collapse/expand, open, filter, cycle matches | -| Entry | `Tab`/`Shift-Tab`, `v`/`V`, `y`, `e`, `Esc` | focus, reveal/hide, timed copy, edit, close | +| Entry | `j`/`k`, arrows, `Tab`/`Shift-Tab`, `v`/`V`, `y`, `e`, `Esc` | focus, reveal/hide, timed copy, edit, close | | Editor | `i`, `a`, `d`, `K`/`J`, `g`, `C-s` | edit/add/remove/reorder/generate/save fields | | Store | `I`, `i`, `p`, `\\`, `d d`, `m`, `c` | init, insert, generate, grep, remove, move, copy | | Git | `g p`, `g P` | pull, push; all other Git operations use `:git …` | @@ -87,6 +87,9 @@ renders five-row digit-only glyphs and a storage-period-driven countdown bar. Shorter panes fall back to a leading inline code and remaining-seconds label. The displayed TOTP survives field navigation, refreshes at its exact storage boundary, and is removed when the entry closes or the application relocks. +The details viewport remains fixed while the focused field is visible. It moves +by the minimum number of rows only when field navigation crosses an edge, and +recalculates that visibility after terminal and OTP layout changes. `:quit` is a TUI convenience. Shell-completion script generation is a CLI build-time/integration surface; interactive Tab completion replaces it here. diff --git a/apps/tui/src/action.rs b/apps/tui/src/action.rs index c6059b3..f035a9b 100644 --- a/apps/tui/src/action.rs +++ b/apps/tui/src/action.rs @@ -272,7 +272,7 @@ pub static ACTIONS: &[ActionSpec] = &[ (KeyCode::Char('j'), KeyModifiers::NONE, "j"), (KeyCode::Down, KeyModifiers::NONE, "↓"), ), - modes: &[Mode::Browser], + modes: BROWSER_LIKE, }, ActionSpec { action: Action::Previous, @@ -282,7 +282,7 @@ pub static ACTIONS: &[ActionSpec] = &[ (KeyCode::Char('k'), KeyModifiers::NONE, "k"), (KeyCode::Up, KeyModifiers::NONE, "↑"), ), - modes: &[Mode::Browser], + modes: BROWSER_LIKE, }, ActionSpec { action: Action::PageDown, @@ -399,22 +399,14 @@ pub static ACTIONS: &[ActionSpec] = &[ action: Action::ScrollDown, label: "scroll down", command: "scroll-down", - bindings: keys!( - (KeyCode::Char('j'), KeyModifiers::NONE, "j"), - (KeyCode::Down, KeyModifiers::NONE, "↓"), - (KeyCode::PageDown, KeyModifiers::NONE, "PgDn"), - ), + bindings: keys!((KeyCode::PageDown, KeyModifiers::NONE, "PgDn")), modes: &[Mode::Viewer], }, ActionSpec { action: Action::ScrollUp, label: "scroll up", command: "scroll-up", - bindings: keys!( - (KeyCode::Char('k'), KeyModifiers::NONE, "k"), - (KeyCode::Up, KeyModifiers::NONE, "↑"), - (KeyCode::PageUp, KeyModifiers::NONE, "PgUp"), - ), + bindings: keys!((KeyCode::PageUp, KeyModifiers::NONE, "PgUp")), modes: &[Mode::Viewer], }, ActionSpec { @@ -851,7 +843,11 @@ mod tests { ); assert_eq!( resolver.feed(Mode::Viewer, KeyCode::Down, KeyModifiers::NONE), - KeyResolution::Action(Action::ScrollDown) + KeyResolution::Action(Action::Next) + ); + assert_eq!( + resolver.feed(Mode::Viewer, KeyCode::Char('k'), KeyModifiers::NONE), + KeyResolution::Action(Action::Previous) ); } diff --git a/apps/tui/src/app.rs b/apps/tui/src/app.rs index e162fff..2aae0d7 100644 --- a/apps/tui/src/app.rs +++ b/apps/tui/src/app.rs @@ -963,6 +963,16 @@ impl App { .expect("checked grep view") .previous(); } + Action::Next if self.mode == Mode::Viewer => { + if let Some(viewer) = self.viewer.as_mut() { + viewer.focus_next(); + } + } + Action::Previous if self.mode == Mode::Viewer => { + if let Some(viewer) = self.viewer.as_mut() { + viewer.focus_previous(); + } + } Action::Next => self.sidebar.move_next(), Action::Previous => self.sidebar.move_previous(), Action::PageDown => self.sidebar.page_down(), diff --git a/apps/tui/src/ui.rs b/apps/tui/src/ui.rs index e9f519d..7a2bf0b 100644 --- a/apps/tui/src/ui.rs +++ b/apps/tui/src/ui.rs @@ -2,7 +2,7 @@ use ratatui::{ Frame, - layout::{Alignment, Constraint, Direction, Layout, Rect}, + layout::{Alignment, Constraint, Direction, Layout, Margin, Rect}, style::{Color, Modifier, Style}, text::{Line, Span}, widgets::{Block, Borders, Paragraph, Wrap}, @@ -217,6 +217,10 @@ fn render_content(frame: &mut Frame, app: &App, area: Rect) { let class = layout_class(frame.area()); let (direction, constraints) = match class { + LayoutClass::Narrow if app.mode() == Mode::Viewer && area.height < 10 => ( + Direction::Vertical, + [Constraint::Length(0), Constraint::Min(1)], + ), LayoutClass::Narrow => ( Direction::Vertical, [Constraint::Percentage(40), Constraint::Percentage(60)], @@ -250,9 +254,11 @@ fn render_content(frame: &mut Frame, app: &App, area: Rect) { app.otp_display().expect("large OTP was checked"), main_rows[0], ); + let lines = viewer_lines(viewer, None); + let scroll = viewer_scroll(viewer, &lines, main_rows[1]); frame.render_widget( - Paragraph::new(viewer_lines(viewer, None)) - .scroll((u16::try_from(viewer.scroll()).unwrap_or(u16::MAX), 0)) + Paragraph::new(lines) + .scroll((scroll, 0)) .block(pane_block( mode_title(app.mode()), app.focus() == PaneFocus::Main, @@ -273,11 +279,13 @@ fn render_content(frame: &mut Frame, app: &App, area: Rect) { } else { (panes[1], None) }; - render_compact_otp(frame, display, otp_area); if let Some(details_area) = details_area { + render_compact_otp(frame, display, otp_area); + let lines = viewer_lines(viewer, None); + let scroll = viewer_scroll(viewer, &lines, details_area); frame.render_widget( - Paragraph::new(viewer_lines(viewer, None)) - .scroll((u16::try_from(viewer.scroll()).unwrap_or(u16::MAX), 0)) + Paragraph::new(lines) + .scroll((scroll, 0)) .block(pane_block( mode_title(app.mode()), app.focus() == PaneFocus::Main, @@ -285,6 +293,8 @@ fn render_content(frame: &mut Frame, app: &App, area: Rect) { .wrap(Wrap { trim: false }), details_area, ); + } else { + render_compact_viewer(frame, app, viewer, display, otp_area); } return; } @@ -292,8 +302,9 @@ fn render_content(frame: &mut Frame, app: &App, area: Rect) { Mode::Viewer => app.viewer().map_or_else( || Paragraph::new(main_text(app)), |viewer| { - Paragraph::new(viewer_lines(viewer, app.otp_display())) - .scroll((u16::try_from(viewer.scroll()).unwrap_or(u16::MAX), 0)) + let lines = viewer_lines(viewer, app.otp_display()); + let scroll = viewer_scroll(viewer, &lines, panes[1]); + Paragraph::new(lines).scroll((scroll, 0)) }, ), Mode::Editor => app.editor().map_or_else( @@ -392,24 +403,95 @@ fn render_large_otp(frame: &mut Frame, display: &crate::app::OtpDisplay, area: R } fn render_compact_otp(frame: &mut Frame, display: &crate::app::OtpDisplay, area: Rect) { - let code = String::from_utf8_lossy(display.code().expose()); - let remaining = display.remaining_seconds().unwrap_or_default(); frame.render_widget( - Paragraph::new(Line::from(vec![ - Span::styled( - code.as_ref(), - Style::default() - .fg(Color::Green) - .add_modifier(Modifier::BOLD), - ), - Span::raw(format!(" — {remaining}s remaining")), - ])) - .alignment(Alignment::Center) - .block(Block::bordered().title("TOTP code")), + Paragraph::new(compact_otp_line(display)) + .alignment(Alignment::Center) + .block(Block::bordered().title("TOTP code")), area, ); } +fn compact_otp_line(display: &crate::app::OtpDisplay) -> Line<'_> { + let code = String::from_utf8_lossy(display.code().expose()); + let remaining = display.remaining_seconds().unwrap_or_default(); + Line::from(vec![ + Span::styled( + code, + Style::default() + .fg(Color::Green) + .add_modifier(Modifier::BOLD), + ), + Span::raw(format!(" — {remaining}s remaining")), + ]) +} + +fn render_compact_viewer( + frame: &mut Frame, + app: &App, + viewer: &EntryViewer, + display: &crate::app::OtpDisplay, + area: Rect, +) { + frame.render_widget( + pane_block(mode_title(app.mode()), app.focus() == PaneFocus::Main), + area, + ); + let inner = area.inner(Margin::new(1, 1)); + if inner.height == 0 { + return; + } + let rows = Layout::vertical([Constraint::Length(1), Constraint::Min(0)]).split(inner); + frame.render_widget(Paragraph::new(compact_otp_line(display)), rows[0]); + if rows[1].height == 0 { + return; + } + let lines = viewer_lines(viewer, None); + let scroll = viewer_scroll_in_content(viewer, &lines, rows[1].width, rows[1].height); + frame.render_widget( + Paragraph::new(lines) + .scroll((scroll, 0)) + .wrap(Wrap { trim: false }), + rows[1], + ); +} + +fn viewer_scroll(viewer: &EntryViewer, lines: &[Line<'_>], area: Rect) -> u16 { + viewer_scroll_in_content( + viewer, + lines, + area.width.saturating_sub(2), + area.height.saturating_sub(2), + ) +} + +fn viewer_scroll_in_content( + viewer: &EntryViewer, + lines: &[Line<'_>], + width: u16, + height: u16, +) -> u16 { + let row_heights = lines + .iter() + .map(|line| { + Paragraph::new(line.clone()) + .wrap(Wrap { trim: false }) + .line_count(width) + .max(1) + }) + .collect::>(); + let focused = viewer.focused_index().unwrap_or_default(); + let focused_start = row_heights.iter().take(focused).sum(); + let focused_height = row_heights.get(focused).copied().unwrap_or(1); + let total_rows = row_heights.iter().sum(); + let scroll = viewer.ensure_focus_visible( + focused_start, + focused_height, + total_rows, + usize::from(height), + ); + u16::try_from(scroll).unwrap_or(u16::MAX) +} + fn otp_ascii_row(code: &[u8], row: usize) -> Zeroizing { let mut rendered = Zeroizing::new(String::with_capacity( code.len().saturating_mul(4).saturating_sub(1), @@ -1157,6 +1239,116 @@ mod tests { assert!(wide.contains("Browser")); } + #[test] + fn viewer_scroll_changes_only_when_focus_crosses_the_rendered_viewport() { + let mut document = fixture_document("email/personal"); + for index in 0..5 { + let insertion = document.fields().len(); + document + .add( + insertion, + ironstorage::document::EntryFieldDraft::field( + format!("extra-{index}"), + format!("value-{index}").into_bytes(), + ) + .expect("field draft"), + ) + .expect("append test field"); + } + + let mut app = App::new(); + app.open_test_document("many/fields", document); + for action in [ + crate::action::Action::FocusNext, + crate::action::Action::Next, + crate::action::Action::FocusNext, + crate::action::Action::Next, + crate::action::Action::FocusNext, + crate::action::Action::Next, + crate::action::Action::FocusNext, + ] { + render(100, 20, &app); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); + app.dispatch(action); + } + + let mut small = App::new(); + let mut document = fixture_document("email/personal"); + for index in 0..5 { + let insertion = document.fields().len(); + document + .add( + insertion, + ironstorage::document::EntryFieldDraft::field( + format!("extra-{index}"), + format!("value-{index}").into_bytes(), + ) + .expect("field draft"), + ) + .expect("append test field"); + } + small.open_test_document("many/fields", document); + for (expected_scroll, action) in [ + (0, crate::action::Action::Next), + (0, crate::action::Action::FocusNext), + (0, crate::action::Action::Next), + (1, crate::action::Action::FocusNext), + (2, crate::action::Action::Next), + ] { + let output = render(40, 8, &small); + assert!(output.contains("Viewer")); + assert_eq!(small.viewer().expect("viewer").scroll(), expected_scroll); + small.dispatch(action); + } + small.dispatch(crate::action::Action::Previous); + render(40, 8, &small); + assert_eq!(small.viewer().expect("viewer").scroll(), 2); + small.dispatch(crate::action::Action::FocusPrevious); + render(40, 8, &small); + assert_eq!(small.viewer().expect("viewer").scroll(), 2); + small.dispatch(crate::action::Action::Previous); + render(40, 8, &small); + assert_eq!(small.viewer().expect("viewer").scroll(), 2); + small.dispatch(crate::action::Action::FocusPrevious); + render(40, 8, &small); + assert_eq!(small.viewer().expect("viewer").scroll(), 1); + + render(100, 20, &small); + assert_eq!(small.viewer().expect("viewer").scroll(), 0); + } + + #[test] + fn wrapped_viewer_fields_keep_the_selected_field_on_screen() { + let mut document = fixture_document("email/personal"); + document + .add( + 1, + ironstorage::document::EntryFieldDraft::field( + "url", + "xxxxxx ".repeat(15).into_bytes(), + ) + .expect("field draft"), + ) + .expect("prepend wrapped field"); + let mut app = App::new(); + app.open_test_document("many/wrapped-fields", document); + + render(40, 8, &app); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); + app.dispatch(crate::action::Action::Next); + let wrapped = render(40, 8, &app); + assert!(wrapped.contains("url: xxxxxx")); + let wrapped_scroll = app.viewer().expect("viewer").scroll(); + assert_eq!(wrapped_scroll, 0); + app.dispatch(crate::action::Action::FocusNext); + let login = render(40, 8, &app); + assert!(login.contains("login: alice@example.test")); + assert!(app.viewer().expect("viewer").scroll() > wrapped_scroll); + + render(140, 20, &app); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); + } + #[test] fn lease_warning_is_concise_and_never_contains_entry_state() { let mut app = App::new(); @@ -1276,11 +1468,13 @@ mod tests { assert!(large.contains("222 333")); assert!(!large.contains("code 123456")); assert!(!large.contains("JBSWY3DPEHPK3PXP")); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); let full_bar = large.matches('█').count(); let compact = render(60, 16, &app); assert!(compact.contains("123456 — 12s remaining")); assert!(!compact.contains("TOTP code —")); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); app.observe_time(66); let progressed = render(120, 20, &app); @@ -1321,6 +1515,8 @@ mod tests { assert!(refreshed.matches('█').count() > full_bar); let minimum = render(40, 8, &app); assert!(minimum.contains("654321 — 30s remaining")); + assert!(minimum.contains("password: ••••••••")); + assert_eq!(app.viewer().expect("viewer").scroll(), 0); let payload = ironstorage::repository::SecretBytes::new( b"otpauth://totp/test?secret=NEVER-RENDER".to_vec(), diff --git a/apps/tui/src/viewer.rs b/apps/tui/src/viewer.rs index d940f68..de702ec 100644 --- a/apps/tui/src/viewer.rs +++ b/apps/tui/src/viewer.rs @@ -1,6 +1,6 @@ //! Secret-aware presentation state for one storage-owned entry document. -use std::fmt; +use std::{cell::Cell, fmt}; use ironstorage::{ document::{DocumentError, EntryDocument, EntryField, EntryFieldId, EntrySensitivity}, @@ -15,7 +15,7 @@ pub struct EntryViewer { document: EntryDocument, focused: usize, revealed: Option, - scroll: usize, + scroll: Cell, } impl EntryViewer { @@ -24,7 +24,7 @@ impl EntryViewer { document, focused: 0, revealed: None, - scroll: 0, + scroll: Cell::new(0), } } @@ -52,7 +52,6 @@ impl EntryViewer { self.hide_revealed(); if !self.document.fields().is_empty() { self.focused = (self.focused + 1) % self.document.fields().len(); - self.scroll = self.focused; } } @@ -63,7 +62,6 @@ impl EntryViewer { .focused .checked_sub(1) .unwrap_or(self.document.fields().len() - 1); - self.scroll = self.focused; } } @@ -90,23 +88,56 @@ impl EntryViewer { } pub fn scroll(&self) -> usize { - self.scroll + self.scroll.get() + } + + /// Keep the focused rendered row range inside the supplied viewport while + /// retaining the current offset whenever it is already visible. + pub fn ensure_focus_visible( + &self, + focused_start: usize, + focused_height: usize, + total_rows: usize, + viewport_rows: usize, + ) -> usize { + if self.document.fields().is_empty() || total_rows == 0 { + self.scroll.set(0); + return 0; + } + if viewport_rows == 0 { + let scroll = self.scroll.get().min(total_rows.saturating_sub(1)); + self.scroll.set(scroll); + return scroll; + } + let maximum = total_rows.saturating_sub(viewport_rows); + let mut scroll = self.scroll.get().min(maximum); + let focused_height = focused_height.max(1); + let focused_end = focused_start.saturating_add(focused_height); + let viewport_end = scroll.saturating_add(viewport_rows); + if focused_height > viewport_rows { + if focused_end <= scroll { + scroll = focused_end.saturating_sub(1); + } else if focused_start >= viewport_end { + scroll = focused_start; + } + } else if focused_start < scroll { + scroll = focused_start; + } else { + if focused_end > viewport_end { + scroll = focused_end.saturating_sub(viewport_rows); + } + } + scroll = scroll.min(maximum); + self.scroll.set(scroll); + scroll } pub fn scroll_down(&mut self, rows: usize) { - self.scroll = self.scroll.saturating_add(rows).min(self.scroll_limit()); + self.scroll.set(self.scroll.get().saturating_add(rows)); } pub fn scroll_up(&mut self, rows: usize) { - self.scroll = self.scroll.saturating_sub(rows); - } - - fn scroll_limit(&self) -> usize { - self.document - .fields() - .iter() - .map(|field| field.value().len().saturating_add(1)) - .sum() + self.scroll.set(self.scroll.get().saturating_sub(rows)); } } @@ -117,7 +148,7 @@ impl fmt::Debug for EntryViewer { .field("document", &self.document) .field("focused", &self.focused) .field("revealed", &self.revealed.map(|_| "[REDACTED]")) - .field("scroll", &self.scroll) + .field("scroll", &self.scroll.get()) .finish() } } @@ -201,4 +232,69 @@ mod tests { viewer.scroll_up(usize::MAX); assert_eq!(viewer.scroll(), 0); } + + #[test] + fn focus_only_scrolls_when_it_crosses_a_viewport_edge() { + let mut viewer = EntryViewer::new(fixture_document("email/personal")); + assert_eq!(viewer.document().fields().len(), 3); + + assert_eq!(viewer.ensure_focus_visible(0, 1, 3, 3), 0); + viewer.focus_next(); + assert_eq!(viewer.ensure_focus_visible(1, 1, 3, 3), 0); + viewer.focus_next(); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 3), 0); + viewer.focus_next(); + assert_eq!(viewer.ensure_focus_visible(0, 1, 3, 3), 0); + + viewer.focus_previous(); + assert_eq!(viewer.focused_index(), Some(2)); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 2), 1); + viewer.focus_previous(); + assert_eq!(viewer.focused_index(), Some(1)); + assert_eq!(viewer.ensure_focus_visible(1, 1, 3, 2), 1); + viewer.focus_previous(); + assert_eq!(viewer.focused_index(), Some(0)); + assert_eq!(viewer.ensure_focus_visible(0, 1, 3, 2), 0); + } + + #[test] + fn viewport_resize_clamps_then_minimally_reveals_the_focus() { + let mut viewer = EntryViewer::new(fixture_document("email/personal")); + viewer.focus_next(); + viewer.focus_next(); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 1), 2); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 2), 1); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 3), 0); + assert_eq!(viewer.ensure_focus_visible(2, 1, 3, 1), 2); + + viewer.focus_previous(); + assert_eq!(viewer.ensure_focus_visible(1, 1, 3, 1), 1); + viewer.focus_previous(); + assert_eq!(viewer.ensure_focus_visible(0, 1, 3, 1), 0); + } + + #[test] + fn wrapped_fields_use_rendered_row_ranges_for_minimal_scrolling() { + let mut viewer = EntryViewer::new(fixture_document("email/personal")); + viewer.focus_next(); + viewer.focus_next(); + + assert_eq!(viewer.ensure_focus_visible(4, 2, 6, 4), 2); + viewer.focus_previous(); + assert_eq!(viewer.ensure_focus_visible(2, 2, 6, 4), 2); + viewer.focus_previous(); + assert_eq!(viewer.ensure_focus_visible(0, 2, 6, 4), 0); + } + + #[test] + fn a_field_taller_than_the_viewport_allows_needed_manual_scrolling() { + let mut viewer = EntryViewer::new(fixture_document("email/personal")); + viewer.focus_next(); + + assert_eq!(viewer.ensure_focus_visible(1, 5, 7, 3), 0); + viewer.scroll_down(2); + assert_eq!(viewer.ensure_focus_visible(1, 5, 7, 3), 2); + viewer.scroll_down(usize::MAX); + assert_eq!(viewer.ensure_focus_visible(1, 5, 7, 3), 4); + } }