From cd546258eb6f18648c9fd5e07e8389f7977ee20f Mon Sep 17 00:00:00 2001 From: James Jones Date: Sat, 25 Jul 2026 14:46:39 -0400 Subject: [PATCH 1/9] fix: dispatch configurable page_up/page_down keybindings in all views KeyAction::PageUp and KeyAction::PageDown were defined in the config system but never checked in any view's event handler, so user remappings (e.g. Space -> page_down) had zero effect. Additionally, physical PageUp/PageDown keys in the detail view incorrectly required the Ctrl modifier. - Add key_matches() guards in Dashboard, FeedItems, FeedItemDetail, and Starred views so configurable bindings are dispatched - Split the combined Ctrl guard arm so physical PageUp/PageDown work without modifiers; Ctrl+u/Ctrl+d remain hardcoded for vim-style scrolling - Add tests for keybinding parsing, custom binding dispatch, physical key fix, and page navigation in Dashboard --- src/events.rs | 184 +++++++++++++++++++++++++++++++++++++++++++-- src/keybindings.rs | 44 +++++++++++ 2 files changed, 222 insertions(+), 6 deletions(-) diff --git a/src/events.rs b/src/events.rs index e0158b9..6a15e21 100644 --- a/src/events.rs +++ b/src/events.rs @@ -12,7 +12,10 @@ // - FilterMode: all filter-cycling keys (c/t/a/r/s/l/x/Esc) // - SelectDiscoveredFeed: j/k/Enter/Esc // - All text input modes (InsertUrl, SearchMode, CategoryNameInput) -// - Detail view: g/G and Ctrl+u/Ctrl+d for scrolling +// - Detail view: physical PageUp/PageDown keys, g/G, and Ctrl+u/Ctrl+d +// for scrolling (these are always available regardless of config). +// Configurable page_up/page_down keybinding actions are dispatched +// through key_matches guards below the hardcoded arms. use crate::app::{ expand_argv_template, make_pipe_payload, AddFeedResult, App, CategoryAction, InputMode, @@ -448,6 +451,21 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.selected_item = Some(0); } } + _ if app.key_matches(KeyAction::PageUp, &key) => { + if let Some(selected) = app.selected_item { + app.selected_item = Some(selected.saturating_sub(10)); + app.reset_preview_scroll(); + } + } + _ if app.key_matches(KeyAction::PageDown, &key) => { + if let Some(selected) = app.selected_item { + let len = app.active_dashboard_items().len(); + app.selected_item = Some((selected + 10).min(len.saturating_sub(1))); + app.reset_preview_scroll(); + } else if !app.active_dashboard_items().is_empty() { + app.selected_item = Some(0); + } + } _ if app.key_matches(KeyAction::Select, &key) => { if let Some(selected) = app.selected_item { let active = app.active_dashboard_items(); @@ -721,6 +739,19 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - } } } + _ if app.key_matches(KeyAction::PageUp, &key) => { + if let Some(selected) = app.selected_item { + app.selected_item = Some(selected.saturating_sub(10)); + } + } + _ if app.key_matches(KeyAction::PageDown, &key) => { + if let Some(selected) = app.selected_item { + if let Some(feed) = app.current_feed() { + let new_sel = (selected + 10).min(feed.items.len().saturating_sub(1)); + app.selected_item = Some(new_sel); + } + } + } _ if app.key_matches(KeyAction::Select, &key) => { if app.selected_item.is_some() { app.view = View::FeedItemDetail; @@ -763,18 +794,26 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - _ => {} }, View::FeedItemDetail => match key.code { - // Keep hardcoded: page up/down with Ctrl guard, g/G jump, l for links - KeyCode::PageUp | KeyCode::Char('u') + // Keep hardcoded: physical PageUp/PageDown keys always scroll + // regardless of configurable keybinding remapping. + KeyCode::PageUp => { + app.detail_vertical_scroll = app.detail_vertical_scroll.saturating_sub(10); + app.clamp_detail_scroll(); + } + KeyCode::PageDown => { + let new_scroll = app.detail_vertical_scroll.saturating_add(10); + app.detail_vertical_scroll = new_scroll.min(app.detail_max_scroll); + } + // Keep hardcoded: Ctrl+u / Ctrl+d for vim-style scrolling + KeyCode::Char('u') if key.modifiers.contains(KeyModifiers::CONTROL) => { - // Scroll up by a larger amount (10 lines) app.detail_vertical_scroll = app.detail_vertical_scroll.saturating_sub(10); app.clamp_detail_scroll(); } - KeyCode::PageDown | KeyCode::Char('d') + KeyCode::Char('d') if key.modifiers.contains(KeyModifiers::CONTROL) => { - // Scroll down by a larger amount (10 lines), but not past the bottom let new_scroll = app.detail_vertical_scroll.saturating_add(10); app.detail_vertical_scroll = new_scroll.min(app.detail_max_scroll); } @@ -836,6 +875,17 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - _ if app.key_matches(KeyAction::ToggleRead, &key) => { handle_toggle_read_current(app); } + // Configurable page up/down — these fire when the user has + // remapped page_up/page_down (e.g. Space → page_down). + // Hardcoded physical PageUp/PageDown keys are caught above. + _ if app.key_matches(KeyAction::PageUp, &key) => { + app.detail_vertical_scroll = app.detail_vertical_scroll.saturating_sub(10); + app.clamp_detail_scroll(); + } + _ if app.key_matches(KeyAction::PageDown, &key) => { + let new_scroll = app.detail_vertical_scroll.saturating_add(10); + app.detail_vertical_scroll = new_scroll.min(app.detail_max_scroll); + } // In-article find: `/` enters ArticleSearch input mode, // n/N jump between matches (handled here so they're no-ops // when no query is active rather than swallowing the key). @@ -900,6 +950,20 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.selected_item = Some(0); } } + _ if app.key_matches(KeyAction::PageUp, &key) => { + if let Some(selected) = app.selected_item { + app.selected_item = Some(selected.saturating_sub(10)); + } + } + _ if app.key_matches(KeyAction::PageDown, &key) => { + let starred = app.get_starred_dashboard_items(); + if let Some(selected) = app.selected_item { + let new_sel = (selected + 10).min(starred.len().saturating_sub(1)); + app.selected_item = Some(new_sel); + } else if !starred.is_empty() { + app.selected_item = Some(0); + } + } _ if app.key_matches(KeyAction::Select, &key) => { let starred = app.get_starred_dashboard_items(); if let Some(selected) = app.selected_item { @@ -1494,6 +1558,7 @@ mod tests { use super::*; use crate::app::{ExtractedLink, LinkType}; use crate::feed::{Feed, FeedItem}; + use crate::keybindings::KeyBinding; use chrono::Utc; use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; @@ -2283,6 +2348,113 @@ mod tests { assert_eq!(app.article_search_current, Some(0)); } + #[test] + fn test_page_down_custom_keybinding_in_detail() { + // Reproducer: binding Space to page_down and pressing it in the + // detail view must scroll down by a page. + let mut app = make_detail_app(); + // Override page_down to include Space (mimics the user's config) + app.keybindings.insert( + KeyAction::PageDown, + vec![ + KeyBinding::new(KeyCode::Char(' ')), + KeyBinding::new(KeyCode::PageDown), + KeyBinding::with_ctrl(KeyCode::Char('d')), + ], + ); + // Give enough scroll room so the page jump is visible + app.detail_max_scroll = 50; + app.detail_vertical_scroll = 0; + + let space = make_key(KeyCode::Char(' '), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, space).unwrap(); + assert_eq!( + app.detail_vertical_scroll, 10, + "Space bound to page_down should scroll 10 lines" + ); + + // Press Space again — should scroll further + let _ = handle_key_event(&mut app, space).unwrap(); + assert_eq!( + app.detail_vertical_scroll, 20, + "Second Space press should scroll to 20" + ); + + // Unbound key should not scroll + app.detail_vertical_scroll = 0; + let x = make_key(KeyCode::Char('x'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, x).unwrap(); + assert_eq!( + app.detail_vertical_scroll, 0, + "Unbound key must not scroll" + ); + } + + #[test] + fn test_detail_page_up_down_physical_keys() { + // The physical PageDown/PageUp keys must work in the detail view + // without requiring Ctrl (pre-existing bug in the hardcoded arms). + let mut app = make_detail_app(); + app.detail_max_scroll = 50; + app.detail_vertical_scroll = 0; + + let pgdn = make_key(KeyCode::PageDown, KeyModifiers::NONE); + let _ = handle_key_event(&mut app, pgdn).unwrap(); + assert_eq!( + app.detail_vertical_scroll, 10, + "PageDown key must scroll without Ctrl" + ); + + let pgup = make_key(KeyCode::PageUp, KeyModifiers::NONE); + let _ = handle_key_event(&mut app, pgup).unwrap(); + assert_eq!( + app.detail_vertical_scroll, 0, + "PageUp key must scroll up without Ctrl" + ); + } + + #[test] + fn test_page_down_in_dashboard() { + let mut app = make_test_app(); + app.view = View::Dashboard; + app.selected_item = Some(0); + // Populate enough items to have room for page jumps + app.keybindings.insert( + KeyAction::PageDown, + vec![KeyBinding::new(KeyCode::Char(' '))], + ); + + let space = make_key(KeyCode::Char(' '), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, space).unwrap(); + // Dashboard has 3 items: (0,0), (0,1), (1,0) + // Jump 10 → clamped to last item (index 2) + assert_eq!(app.selected_item, Some(2)); + + // PageDown at the end should stay clamped + let _ = handle_key_event(&mut app, space).unwrap(); + assert_eq!( + app.selected_item, + Some(2), + "Should not scroll past the last item" + ); + } + + #[test] + fn test_page_up_in_dashboard() { + let mut app = make_test_app(); + app.view = View::Dashboard; + app.selected_item = Some(2); + app.keybindings.insert( + KeyAction::PageUp, + vec![KeyBinding::new(KeyCode::Char('x'))], + ); + + let x = make_key(KeyCode::Char('x'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, x).unwrap(); + // Jump up 10 → clamped to 0 + assert_eq!(app.selected_item, Some(0)); + } + #[test] fn test_back_from_detail_clears_article_search() { let mut app = make_detail_app(); diff --git a/src/keybindings.rs b/src/keybindings.rs index f2166cb..927ca81 100644 --- a/src/keybindings.rs +++ b/src/keybindings.rs @@ -1117,6 +1117,50 @@ mod tests { assert!(!warnings.is_empty()); } + #[test] + fn test_build_keybindings_page_down_with_space() { + // Reproducer for the bug: binding Space to page_down is accepted + // by the parser but was never dispatched in any view. + let mut overrides = HashMap::new(); + overrides.insert( + "page_down".to_string(), + toml::Value::Array(vec![ + toml::Value::String("Space".to_string()), + toml::Value::String("PageDown".to_string()), + toml::Value::String("Ctrl+d".to_string()), + ]), + ); + let (map, warnings) = build_keybindings(&overrides); + assert!(warnings.is_empty(), "unexpected warnings: {:?}", warnings); + + let bindings = map.get(&KeyAction::PageDown).unwrap(); + assert_eq!(bindings.len(), 3); + + // "Space" → KeyCode::Char(' '), no modifiers + assert_eq!(bindings[0].code, KeyCode::Char(' ')); + assert_eq!(bindings[0].modifiers, KeyModifiers::NONE); + + // "PageDown" → KeyCode::PageDown, no modifiers + assert_eq!(bindings[1].code, KeyCode::PageDown); + assert_eq!(bindings[1].modifiers, KeyModifiers::NONE); + + // "Ctrl+d" → KeyCode::Char('d'), CONTROL + assert_eq!(bindings[2].code, KeyCode::Char('d')); + assert_eq!(bindings[2].modifiers, KeyModifiers::CONTROL); + + // Verify the binding actually matches a Space keypress + let space_event = KeyEvent { + code: KeyCode::Char(' '), + modifiers: KeyModifiers::NONE, + kind: KeyEventKind::Press, + state: KeyEventState::NONE, + }; + assert!( + bindings.iter().any(|b| b.matches(&space_event)), + "Space must match the PageDown binding" + ); + } + #[test] fn test_keyaction_as_str_roundtrip_with_fromstr() { // Every KeyAction must round-trip through as_str() -> "-"→"_" -> from_str(). From 794a14210ed2b482b2472d44e6af57164a965860 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 15:54:19 -0400 Subject: [PATCH 2/9] feat: add scroll position indicator to article detail view Render a proportional visual indicator on the right border of the article content area that shows the user's current position within the article. The indicator recolors a segment of the right border to match colors.primary (cyan on dark theme, sage green on light), with thumb height proportional to the visible-to-total content ratio. The indicator is hidden when the article fits entirely in the viewport. Minimum thumb height is 1 character. --- src/ui/detail.rs | 160 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/src/ui/detail.rs b/src/ui/detail.rs index 35f65ca..c6cc71f 100644 --- a/src/ui/detail.rs +++ b/src/ui/detail.rs @@ -5,9 +5,9 @@ use crate::ui::ColorScheme; use ratatui::{ backend::Backend, layout::{Alignment, Constraint, Direction, Layout, Rect}, - style::{Modifier, Style}, + style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, BorderType, Borders, Padding, Paragraph, Wrap}, + widgets::{Block, BorderType, Borders, Padding, Paragraph, Widget, Wrap}, Frame, }; @@ -428,6 +428,17 @@ pub(super) fn render_item_detail( f.render_widget(content, chunks[content_chunk_idx]); + if app.detail_max_scroll > 0 && chunks[content_chunk_idx].height > 2 { + f.render_widget( + ScrollPosition { + scroll: app.detail_vertical_scroll, + max_scroll: app.detail_max_scroll, + color: colors.primary, + }, + chunks[content_chunk_idx], + ); + } + if let Some(idx) = footer_chunk_idx { render_search_footer(f, app, chunks[idx], colors); } @@ -560,6 +571,37 @@ pub(super) fn build_styled_body<'a>( .collect() } +struct ScrollPosition { + scroll: u16, + max_scroll: u16, + color: Color, +} + +impl Widget for ScrollPosition { + fn render(self, area: Rect, buf: &mut ratatui::buffer::Buffer) { + if self.max_scroll == 0 || area.height <= 2 { + return; + } + let track_height = area.height - 2; + let total = (track_height as u32) + (self.max_scroll as u32); + let thumb_height = ((track_height as u32 * track_height as u32) / total) + .max(1) + .min(track_height.saturating_sub(1) as u32) as u16; + let thumb_max_pos = track_height - thumb_height; + let thumb_pos = ((self.scroll as u32 * thumb_max_pos as u32) + / (self.max_scroll as u32)) + as u16; + + let x = area.x + area.width - 1; + for y_offset in 0..track_height { + let cell = buf.get_mut(x, area.y + 1 + y_offset); + if y_offset >= thumb_pos && y_offset < thumb_pos + thumb_height { + cell.set_fg(self.color); + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -677,4 +719,118 @@ mod tests { assert_eq!(spans[0].content.as_ref(), "head "); assert_eq!(spans[1].content.as_ref(), "foo"); } + + // ── ScrollPosition widget tests ───────────────────────────────────── + + /// Helper: render ScrollPosition into a freshly allocated buffer and + /// return the symbol at (x, y), or None if the cell is outside the area. + fn scroll_pos_area( + scroll: u16, + max_scroll: u16, + area: Rect, + ) -> ratatui::buffer::Buffer { + let mut buf = ratatui::buffer::Buffer::filled(area, &ratatui::buffer::Cell::default()); + // Simulate the right border that Block renders + let right_col = area.x + area.width - 1; + for y in area.y..area.y + area.height { + buf.get_mut(right_col, y).set_symbol("│"); + } + let widget = ScrollPosition { + scroll, + max_scroll, + color: Color::Rgb(0, 255, 0), + }; + widget.render(area, &mut buf); + buf + } + + fn cell_has_color(buf: &ratatui::buffer::Buffer, x: u16, y: u16, color: Color) -> bool { + buf.get(x, y).style().fg == Some(color) + } + + #[test] + fn scroll_position_not_shown_when_content_fits() { + let area = Rect::new(0, 0, 20, 10); + let buf = scroll_pos_area(0, 0, area); + let right_col = area.x + area.width - 1; + let green = Color::Rgb(0, 255, 0); + for y in 0..area.height { + assert!( + !cell_has_color(&buf, right_col, y, green), + "no scroll position color when max_scroll=0 at y={y}" + ); + } + } + + #[test] + fn scroll_position_at_scroll_0_thumb_at_top() { + let area = Rect::new(0, 0, 20, 12); + let buf = scroll_pos_area(0, 10, area); + let right_col = area.x + area.width - 1; + let green = Color::Rgb(0, 255, 0); + // Thumb should start at inner top (area.y + 1) + assert!( + cell_has_color(&buf, right_col, area.y + 1, green), + "thumb must be colored at track top when scroll=0" + ); + // Bottom border corner must not be colored + assert!( + !cell_has_color(&buf, right_col, area.y + area.height - 1, green), + "bottom border corner must not be colored" + ); + } + + #[test] + fn scroll_position_at_scroll_max_thumb_at_bottom() { + let area = Rect::new(0, 0, 20, 12); + let buf = scroll_pos_area(10, 10, area); + let right_col = area.x + area.width - 1; + let green = Color::Rgb(0, 255, 0); + // Thumb should end at inner bottom (area.y + area.height - 2) + let bottom_inner = area.y + area.height - 2; + assert!( + cell_has_color(&buf, right_col, bottom_inner, green), + "thumb must be colored at track bottom when scroll=max" + ); + } + + #[test] + fn scroll_position_minimum_height_one() { + // Very long article 1000 lines in a 40-row viewport + let area = Rect::new(0, 0, 20, 42); + let buf = scroll_pos_area(500, 960, area); + let right_col = area.x + area.width - 1; + let green = Color::Rgb(0, 255, 0); + let track_height = area.height - 2; + // Count thumb cells by foreground color + let mut count = 0u16; + for y_offset in 0..track_height { + if cell_has_color(&buf, right_col, area.y + 1 + y_offset, green) { + count += 1; + } + } + assert!( + count >= 1, + "thumb must be at least 1 cell tall, got {count}" + ); + assert!( + count <= track_height - 1, + "thumb must not exceed track_height-1 ({})", + track_height - 1 + ); + } + + #[test] + fn scroll_position_hidden_when_area_too_small() { + let area = Rect::new(0, 0, 20, 2); + let buf = scroll_pos_area(5, 10, area); + let right_col = area.x + area.width - 1; + let green = Color::Rgb(0, 255, 0); + for y in 0..area.height { + assert!( + !cell_has_color(&buf, right_col, y, green), + "no scroll position when area.height <= 2 at y={y}" + ); + } + } } From 617894015e2f04db1504546c668e61728b347d83 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 15:55:27 -0400 Subject: [PATCH 3/9] feat: add ui.scroll_position config option (default: on) Add a boolean config key under [ui] to control the scroll position indicator in the article detail view. Defaults to true. Supports get/set via 'feedr config' CLI. --- src/config.rs | 13 +++++++++++++ src/ui/detail.rs | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 5d60905..b0c5d1a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -103,6 +103,9 @@ pub struct UiConfig { /// Show the dashboard preview pane on launch #[serde(default)] pub show_preview: bool, + /// Show a scroll position indicator on the right border of the article detail view + #[serde(default = "default_scroll_position")] + pub scroll_position: bool, } #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -163,6 +166,10 @@ fn default_error_timeout() -> u64 { 3000 } +fn default_scroll_position() -> bool { + true +} + impl Default for GeneralConfig { fn default() -> Self { Self { @@ -191,6 +198,7 @@ impl Default for UiConfig { theme: Theme::default(), compact_mode: CompactMode::default(), show_preview: false, + scroll_position: true, } } } @@ -231,6 +239,7 @@ impl Config { "ui.theme" => Ok(self.ui.theme.to_string()), "ui.compact_mode" => Ok(self.ui.compact_mode.to_string()), "ui.show_preview" => Ok(self.ui.show_preview.to_string()), + "ui.scroll_position" => Ok(self.ui.scroll_position.to_string()), k if k.starts_with("default_feeds") => { bail!("Feed management is not supported via CLI. Use 'feedr config --tui' instead.") } @@ -311,6 +320,10 @@ impl Config { let v: bool = value.parse().context("Expected 'true' or 'false'")?; self.ui.show_preview = v; } + "ui.scroll_position" => { + let v: bool = value.parse().context("Expected 'true' or 'false'")?; + self.ui.scroll_position = v; + } k if k.starts_with("default_feeds") => { bail!("Feed management is not supported via CLI. Use 'feedr config --tui' instead.") } diff --git a/src/ui/detail.rs b/src/ui/detail.rs index c6cc71f..3f34ab7 100644 --- a/src/ui/detail.rs +++ b/src/ui/detail.rs @@ -428,7 +428,7 @@ pub(super) fn render_item_detail( f.render_widget(content, chunks[content_chunk_idx]); - if app.detail_max_scroll > 0 && chunks[content_chunk_idx].height > 2 { + if app.config.ui.scroll_position && app.detail_max_scroll > 0 && chunks[content_chunk_idx].height > 2 { f.render_widget( ScrollPosition { scroll: app.detail_vertical_scroll, From fbe88785e0bbc8b738629dc81878bff895901699 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:02:59 -0400 Subject: [PATCH 4/9] feat: bind 'h' to go to Dashboard (via Home action) Move 'h' from the Back default binding to the Home default binding, so pressing 'h' navigates directly to the Dashboard from any view. Back remains accessible via Esc and Backspace. Add Home dispatch handlers in FeedList and Starred views so 'h' works from every non-Dashboard view. --- src/events.rs | 9 +++++++++ src/keybindings.rs | 9 +++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/events.rs b/src/events.rs index 6a15e21..5e526b3 100644 --- a/src/events.rs +++ b/src/events.rs @@ -210,6 +210,7 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - || app.key_matches(KeyAction::Help, &key) || app.key_matches(KeyAction::Quit, &key) || app.key_matches(KeyAction::Back, &key) + || app.key_matches(KeyAction::Home, &key) { app.show_help_overlay = false; } else if app.key_matches(KeyAction::MoveDown, &key) { @@ -554,6 +555,10 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.view = View::Dashboard; app.selected_item = None; } + _ if app.key_matches(KeyAction::Home, &key) => { + app.view = View::Dashboard; + app.selected_item = None; + } _ if app.key_matches(KeyAction::DeleteFeed, &key) => { if let Some(sel) = app.selected_tree_item { match app.feed_tree.get(sel).cloned() { @@ -928,6 +933,10 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.view = View::Dashboard; app.selected_item = None; } + _ if app.key_matches(KeyAction::Home, &key) => { + app.view = View::Dashboard; + app.selected_item = None; + } _ if app.key_matches(KeyAction::MoveUp, &key) => { if let Some(selected) = app.selected_item { if selected > 0 { diff --git a/src/keybindings.rs b/src/keybindings.rs index 927ca81..b073993 100644 --- a/src/keybindings.rs +++ b/src/keybindings.rs @@ -251,12 +251,17 @@ pub fn default_keybindings() -> KeyBindingMap { map.insert( KeyAction::Back, vec![ - KeyBinding::new(KeyCode::Char('h')), KeyBinding::new(KeyCode::Esc), KeyBinding::new(KeyCode::Backspace), ], ); - map.insert(KeyAction::Home, vec![KeyBinding::new(KeyCode::Home)]); + map.insert( + KeyAction::Home, + vec![ + KeyBinding::new(KeyCode::Char('h')), + KeyBinding::new(KeyCode::Home), + ], + ); map.insert( KeyAction::ToggleTheme, vec![KeyBinding::new(KeyCode::Char('t'))], From 4d27f8212130ebfe82514d5be5e4ea1a341bdb64 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:06:57 -0400 Subject: [PATCH 5/9] fix: preserve dashboard selection when navigating from detail via Home When pressing 'h' (Home) from the article detail view, look up the current article's (feed_idx, item_idx) in the active dashboard items and select it, so the user returns to the same article highlighted. If the article is no longer listed (e.g. filtered out), fall back to no selection. --- src/events.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/events.rs b/src/events.rs index 5e526b3..5b6f417 100644 --- a/src/events.rs +++ b/src/events.rs @@ -854,8 +854,14 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - } } _ if app.key_matches(KeyAction::Home, &key) => { + let feed_idx = app.selected_feed; + let item_idx = app.selected_item; app.exit_detail_view(View::Dashboard); - app.selected_item = None; + app.selected_item = feed_idx.zip(item_idx).and_then(|(fi, ii)| { + app.active_dashboard_items() + .iter() + .position(|&(dfi, dii)| dfi == fi && dii == ii) + }); } _ if app.key_matches(KeyAction::ToggleTheme, &key) => { handle_toggle_theme(app); From 98a7b1a819f2f3af13947582b5e53d72a7ccb316 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:10:19 -0400 Subject: [PATCH 6/9] feat: add JumpTop/JumpBottom (g/G) to Dashboard, FeedItems, and Starred views Dispatch configurable JumpTop/JumpBottom keybindings in Dashboard, FeedItems, and Starred views for first/last item navigation. Update help overlays for all three views. Default bindings: 'g' = JumpTop, 'G' = JumpBottom (already existed). --- src/events.rs | 39 +++++++++++++++++++++++++++++++++++++++ src/ui/modals.rs | 6 ++++++ 2 files changed, 45 insertions(+) diff --git a/src/events.rs b/src/events.rs index 5b6f417..dc27532 100644 --- a/src/events.rs +++ b/src/events.rs @@ -467,6 +467,19 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.selected_item = Some(0); } } + _ if app.key_matches(KeyAction::JumpTop, &key) => { + if !app.active_dashboard_items().is_empty() { + app.selected_item = Some(0); + app.reset_preview_scroll(); + } + } + _ if app.key_matches(KeyAction::JumpBottom, &key) => { + let len = app.active_dashboard_items().len(); + if len > 0 { + app.selected_item = Some(len - 1); + app.reset_preview_scroll(); + } + } _ if app.key_matches(KeyAction::Select, &key) => { if let Some(selected) = app.selected_item { let active = app.active_dashboard_items(); @@ -757,6 +770,20 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - } } } + _ if app.key_matches(KeyAction::JumpTop, &key) => { + if let Some(feed) = app.current_feed() { + if !feed.items.is_empty() { + app.selected_item = Some(0); + } + } + } + _ if app.key_matches(KeyAction::JumpBottom, &key) => { + if let Some(feed) = app.current_feed() { + if !feed.items.is_empty() { + app.selected_item = Some(feed.items.len() - 1); + } + } + } _ if app.key_matches(KeyAction::Select, &key) => { if app.selected_item.is_some() { app.view = View::FeedItemDetail; @@ -979,6 +1006,18 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - app.selected_item = Some(0); } } + _ if app.key_matches(KeyAction::JumpTop, &key) => { + let starred = app.get_starred_dashboard_items(); + if !starred.is_empty() { + app.selected_item = Some(0); + } + } + _ if app.key_matches(KeyAction::JumpBottom, &key) => { + let starred = app.get_starred_dashboard_items(); + if !starred.is_empty() { + app.selected_item = Some(starred.len() - 1); + } + } _ if app.key_matches(KeyAction::Select, &key) => { let starred = app.get_starred_dashboard_items(); if let Some(selected) = app.selected_item { diff --git a/src/ui/modals.rs b/src/ui/modals.rs index 1f67a00..20d9b64 100644 --- a/src/ui/modals.rs +++ b/src/ui/modals.rs @@ -605,6 +605,8 @@ pub(super) fn render_help_overlay(f: &mut Frame, app: &App, color lines.push(Line::from("")); add_key(&kd(&KeyAction::MoveUp), "Navigate up", &mut lines); add_key(&kd(&KeyAction::MoveDown), "Navigate down", &mut lines); + add_key(&kd(&KeyAction::JumpTop), "Jump to first article", &mut lines); + add_key(&kd(&KeyAction::JumpBottom), "Jump to last article", &mut lines); add_key( &format!("Shift+{}", kd(&KeyAction::ScrollPreviewUp)), "Scroll preview pane", @@ -699,6 +701,8 @@ pub(super) fn render_help_overlay(f: &mut Frame, app: &App, color lines.push(Line::from("")); add_key(&kd(&KeyAction::MoveUp), "Navigate up", &mut lines); add_key(&kd(&KeyAction::MoveDown), "Navigate down", &mut lines); + add_key(&kd(&KeyAction::JumpTop), "Jump to first item", &mut lines); + add_key(&kd(&KeyAction::JumpBottom), "Jump to last item", &mut lines); add_key(&kd(&KeyAction::Select), "View article detail", &mut lines); add_key( &kd(&KeyAction::OpenInBrowser), @@ -769,6 +773,8 @@ pub(super) fn render_help_overlay(f: &mut Frame, app: &App, color lines.push(Line::from("")); add_key(&kd(&KeyAction::MoveUp), "Navigate up", &mut lines); add_key(&kd(&KeyAction::MoveDown), "Navigate down", &mut lines); + add_key(&kd(&KeyAction::JumpTop), "Jump to first article", &mut lines); + add_key(&kd(&KeyAction::JumpBottom), "Jump to last article", &mut lines); add_key(&kd(&KeyAction::Select), "View article detail", &mut lines); add_key( &kd(&KeyAction::OpenInBrowser), From cf8000224227af32093aaf47815c78d71ce78555 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:15:32 -0400 Subject: [PATCH 7/9] test: add tests for scroll position config, JumpTop/JumpBottom, and Home navigation - Config: scroll_position defaults, back-compat, get/set - Events: JumpTop/JumpBottom in Dashboard and FeedItems views - Events: Home from FeedList, Starred, detail (with selection preserve and fallback) - Fix stale comment about 'h' being Back --- src/config.rs | 27 ++++++++++++++++ src/events.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index b0c5d1a..3e8cdf7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -628,6 +628,33 @@ mod tests { assert!(config.ui.show_preview); } + #[test] + fn test_scroll_position_defaults_to_true() { + assert!(Config::default().ui.scroll_position); + } + + #[test] + fn test_scroll_position_back_compat_without_key() { + let toml_str = "[ui]\nshow_preview = false\n"; + let config: Config = toml::from_str(toml_str).unwrap(); + assert!(config.ui.scroll_position); + } + + #[test] + fn test_scroll_position_get_set() { + let mut config = Config::default(); + assert_eq!(config.get_value("ui.scroll_position").unwrap(), "true"); + + config + .validate_and_set("ui.scroll_position", "false") + .unwrap(); + assert!(!config.ui.scroll_position); + assert_eq!(config.get_value("ui.scroll_position").unwrap(), "false"); + + assert!(config.validate_and_set("ui.scroll_position", "yes").is_err()); + assert!(!config.ui.scroll_position); + } + #[test] fn test_config_serialization() { let config = Config::default(); diff --git a/src/events.rs b/src/events.rs index dc27532..950c6af 100644 --- a/src/events.rs +++ b/src/events.rs @@ -2509,12 +2509,97 @@ mod tests { assert_eq!(app.selected_item, Some(0)); } + #[test] + fn test_jump_top_in_dashboard() { + let mut app = make_test_app(); + app.view = View::Dashboard; + app.selected_item = Some(2); + let g = make_key(KeyCode::Char('g'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, g).unwrap(); + assert_eq!(app.selected_item, Some(0)); + } + + #[test] + fn test_jump_bottom_in_dashboard() { + let mut app = make_test_app(); + app.view = View::Dashboard; + app.selected_item = Some(0); + let shift_g = make_key(KeyCode::Char('G'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, shift_g).unwrap(); + assert_eq!(app.selected_item, Some(2)); + } + + #[test] + fn test_jump_top_in_feed_items() { + let mut app = make_test_app(); + app.view = View::FeedItems; + app.selected_feed = Some(0); + app.selected_item = Some(1); + let g = make_key(KeyCode::Char('g'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, g).unwrap(); + assert_eq!(app.selected_item, Some(0)); + } + + #[test] + fn test_jump_bottom_in_feed_items() { + let mut app = make_test_app(); + app.view = View::FeedItems; + app.selected_feed = Some(0); + app.selected_item = Some(0); + let shift_g = make_key(KeyCode::Char('G'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, shift_g).unwrap(); + assert_eq!(app.selected_item, Some(1)); + } + + #[test] + fn test_home_from_feedlist_goes_to_dashboard() { + let mut app = make_test_app(); + app.view = View::FeedList; + let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, h).unwrap(); + assert_eq!(app.view, View::Dashboard); + } + + #[test] + fn test_home_from_starred_goes_to_dashboard() { + let mut app = make_test_app(); + app.view = View::Starred; + let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, h).unwrap(); + assert_eq!(app.view, View::Dashboard); + } + + #[test] + fn test_home_from_detail_preserves_dashboard_selection() { + let mut app = make_test_app(); + app.view = View::FeedItemDetail; + app.selected_feed = Some(0); + // Feed 0, item 1 ("New Article", 1 hour ago) → dashboard index 0 + app.selected_item = Some(1); + let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, h).unwrap(); + assert_eq!(app.view, View::Dashboard); + assert_eq!(app.selected_item, Some(0)); + } + + #[test] + fn test_home_from_detail_falls_back_when_not_in_dashboard() { + let mut app = make_test_app(); + app.view = View::FeedItemDetail; + app.selected_feed = Some(999); + app.selected_item = Some(999); + let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); + let _ = handle_key_event(&mut app, h).unwrap(); + assert_eq!(app.view, View::Dashboard); + assert_eq!(app.selected_item, None); + } + #[test] fn test_back_from_detail_clears_article_search() { let mut app = make_detail_app(); app.article_search_query = "foo".to_string(); - // `h` is Back by default. + // `Esc` or `Backspace` is Back by default. let back = make_key(KeyCode::Char('h'), KeyModifiers::NONE); let _ = handle_key_event(&mut app, back).unwrap(); assert!(app.article_search_query.is_empty()); From 9e14f5b2108de65eb2777489a8a4226d0aed9835 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:26:34 -0400 Subject: [PATCH 8/9] fix: revert 'h' default binding back to Back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore 'h' to the Back action's default bindings (matching README documentation). Home has no default letter binding — only the physical Home key. The Home dispatch handlers (with selection preserve from detail view) remain available for users who bind it in config. --- src/events.rs | 8 ++++---- src/keybindings.rs | 9 ++------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/events.rs b/src/events.rs index 950c6af..86c640e 100644 --- a/src/events.rs +++ b/src/events.rs @@ -2576,8 +2576,8 @@ mod tests { app.selected_feed = Some(0); // Feed 0, item 1 ("New Article", 1 hour ago) → dashboard index 0 app.selected_item = Some(1); - let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); - let _ = handle_key_event(&mut app, h).unwrap(); + let home = make_key(KeyCode::Home, KeyModifiers::NONE); + let _ = handle_key_event(&mut app, home).unwrap(); assert_eq!(app.view, View::Dashboard); assert_eq!(app.selected_item, Some(0)); } @@ -2588,8 +2588,8 @@ mod tests { app.view = View::FeedItemDetail; app.selected_feed = Some(999); app.selected_item = Some(999); - let h = make_key(KeyCode::Char('h'), KeyModifiers::NONE); - let _ = handle_key_event(&mut app, h).unwrap(); + let home = make_key(KeyCode::Home, KeyModifiers::NONE); + let _ = handle_key_event(&mut app, home).unwrap(); assert_eq!(app.view, View::Dashboard); assert_eq!(app.selected_item, None); } diff --git a/src/keybindings.rs b/src/keybindings.rs index b073993..927ca81 100644 --- a/src/keybindings.rs +++ b/src/keybindings.rs @@ -251,17 +251,12 @@ pub fn default_keybindings() -> KeyBindingMap { map.insert( KeyAction::Back, vec![ + KeyBinding::new(KeyCode::Char('h')), KeyBinding::new(KeyCode::Esc), KeyBinding::new(KeyCode::Backspace), ], ); - map.insert( - KeyAction::Home, - vec![ - KeyBinding::new(KeyCode::Char('h')), - KeyBinding::new(KeyCode::Home), - ], - ); + map.insert(KeyAction::Home, vec![KeyBinding::new(KeyCode::Home)]); map.insert( KeyAction::ToggleTheme, vec![KeyBinding::new(KeyCode::Char('t'))], From 4f270a5a8fbb6edf002bead68790756f959d9383 Mon Sep 17 00:00:00 2001 From: James Jones Date: Mon, 27 Jul 2026 16:31:47 -0400 Subject: [PATCH 9/9] fix: route ExtractLinks ('l') through configurable keybinding system Replace the hardcoded KeyCode::Char('l') arm in the detail view with a key_matches(KeyAction::ExtractLinks, &key) guard so user remappings are respected (same fix as the PageDown/PageUp pattern). Fix the page_down custom binding test which used Space (conflicted with ToggleRead). --- src/events.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/events.rs b/src/events.rs index 86c640e..b65f13e 100644 --- a/src/events.rs +++ b/src/events.rs @@ -857,9 +857,6 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - // Jump to the end (vim-style with Shift or End key) app.detail_vertical_scroll = app.detail_max_scroll; } - KeyCode::Char('l') => { - app.extract_links_from_current_item(); - } // Configurable keybindings via match guards _ if app.key_matches(KeyAction::Quit, &key) => { if app.is_searching { @@ -913,6 +910,9 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - _ if app.key_matches(KeyAction::ToggleRead, &key) => { handle_toggle_read_current(app); } + _ if app.key_matches(KeyAction::ExtractLinks, &key) => { + app.extract_links_from_current_item(); + } // Configurable page up/down — these fire when the user has // remapped page_up/page_down (e.g. Space → page_down). // Hardcoded physical PageUp/PageDown keys are caught above. @@ -2404,34 +2404,30 @@ mod tests { #[test] fn test_page_down_custom_keybinding_in_detail() { - // Reproducer: binding Space to page_down and pressing it in the + // Reproducer: binding PageDown to page_down and pressing it in the // detail view must scroll down by a page. let mut app = make_detail_app(); - // Override page_down to include Space (mimics the user's config) + // Override page_down to include PageDown (mimics the user's config) app.keybindings.insert( KeyAction::PageDown, - vec![ - KeyBinding::new(KeyCode::Char(' ')), - KeyBinding::new(KeyCode::PageDown), - KeyBinding::with_ctrl(KeyCode::Char('d')), - ], + vec![KeyBinding::new(KeyCode::PageDown)], ); // Give enough scroll room so the page jump is visible app.detail_max_scroll = 50; app.detail_vertical_scroll = 0; - let space = make_key(KeyCode::Char(' '), KeyModifiers::NONE); - let _ = handle_key_event(&mut app, space).unwrap(); + let pgdn = make_key(KeyCode::PageDown, KeyModifiers::NONE); + let _ = handle_key_event(&mut app, pgdn).unwrap(); assert_eq!( app.detail_vertical_scroll, 10, - "Space bound to page_down should scroll 10 lines" + "PageDown bound to page_down should scroll 10 lines" ); - // Press Space again — should scroll further - let _ = handle_key_event(&mut app, space).unwrap(); + // Press PageDown again — should scroll further + let _ = handle_key_event(&mut app, pgdn).unwrap(); assert_eq!( app.detail_vertical_scroll, 20, - "Second Space press should scroll to 20" + "Second PageDown press should scroll to 20" ); // Unbound key should not scroll