From c42a46c7ca65fc2d66f16318ed61fd67dab0962d Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Thu, 17 Sep 2026 17:38:37 -0600 Subject: [PATCH] fix: page up and down stop at the table edges instead of wrapping A page is 20 rows and the index wrapped modulo one less than the row count, so the last row was unreachable by paging, 21-row and 5-row tables did nothing on page down from the first row, and 4-row tables cycled 0, 2, 1 and never reached row 3. Page down now clamps to the last row and page up clamps to the first. With no division left, a single-row table is safe, so the early return only guards the empty table again, matching scroll_to_top and scroll_to_bottom. --- src/handlers/handler_proptest.rs | 9 ++ src/handlers/table_handler_tests.rs | 5 +- src/models/stateful_table.rs | 11 +- src/models/stateful_table_tests.rs | 188 ++++++++++++++++++++++------ 4 files changed, 166 insertions(+), 47 deletions(-) diff --git a/src/handlers/handler_proptest.rs b/src/handlers/handler_proptest.rs index 5a216c8b..1bf29f49 100644 --- a/src/handlers/handler_proptest.rs +++ b/src/handlers/handler_proptest.rs @@ -171,10 +171,19 @@ mod property_tests { table.set_items(movies); for i in 0..page_ops { + let previous_index = table.state.selected().unwrap(); + if i % 2 == 0 { table.page_down(); + + prop_assert_eq!( + table.state.selected(), + Some((previous_index + 20).min(list_size - 1)) + ); } else { table.page_up(); + + prop_assert_eq!(table.state.selected(), Some(previous_index.saturating_sub(20))); } let current = table.current_selection(); diff --git a/src/handlers/table_handler_tests.rs b/src/handlers/table_handler_tests.rs index 45dc7b45..b234fdb1 100644 --- a/src/handlers/table_handler_tests.rs +++ b/src/handlers/table_handler_tests.rs @@ -606,17 +606,18 @@ mod tests { .take(100) .collect(); app.data.radarr_data.movies.set_items(movies_vec); + app.data.radarr_data.movies.state.select(Some(50)); TableHandlerUnit::new(key, &mut app, ActiveRadarrBlock::Movies, None).handle(); if key == Key::PgUp { assert_str_eq!( app.data.radarr_data.movies.current_selection().title.text, - "Test 79" + "Test 30" ); } else { assert_str_eq!( app.data.radarr_data.movies.current_selection().title.text, - "Test 20" + "Test 70" ); } } diff --git a/src/models/stateful_table.rs b/src/models/stateful_table.rs index 6ad579c5..6ac4a0d9 100644 --- a/src/models/stateful_table.rs +++ b/src/models/stateful_table.rs @@ -100,29 +100,26 @@ where { fn page_down(&mut self) { let items_len = self.active_items().len(); - if items_len <= 1 { + if items_len == 0 { return; } let state = self.active_state_mut(); match state.selected() { - Some(i) => state.select(Some(i.saturating_add(20) % (items_len - 1))), + Some(i) => state.select(Some(i.saturating_add(20).min(items_len - 1))), None => state.select_first(), } } fn page_up(&mut self) { let items_len = self.active_items().len(); - if items_len <= 1 { + if items_len == 0 { return; } let state = self.active_state_mut(); match state.selected() { - Some(i) => { - let len = items_len - 1; - state.select(Some((i + len - (20 % len)) % len)); - } + Some(i) => state.select(Some(i.saturating_sub(20))), None => state.select_last(), } } diff --git a/src/models/stateful_table_tests.rs b/src/models/stateful_table_tests.rs index c1851e69..b1ec752a 100644 --- a/src/models/stateful_table_tests.rs +++ b/src/models/stateful_table_tests.rs @@ -318,6 +318,69 @@ mod tests { 20 ); + stateful_table.page_down(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 40 + ); + + stateful_table.page_down(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 60 + ); + + stateful_table.page_down(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 80 + ); + + stateful_table.page_down(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 99 + ); + + stateful_table.page_down(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 99 + ); + + stateful_table.page_up(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 79 + ); + + stateful_table.page_up(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 59 + ); + + stateful_table.page_up(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 39 + ); + + stateful_table.page_up(); + + assert_some_eq_x!( + stateful_table.filtered_state.as_ref().unwrap().selected(), + 19 + ); + stateful_table.page_up(); assert_some_eq_x!( @@ -327,33 +390,10 @@ mod tests { stateful_table.page_up(); - assert_some_eq_x!( - stateful_table.filtered_state.as_ref().unwrap().selected(), - stateful_table.filtered_items.as_ref().unwrap().len() - 21 - ); - - stateful_table.page_down(); - assert_some_eq_x!( stateful_table.filtered_state.as_ref().unwrap().selected(), 0 ); - - stateful_table.scroll_down(); - stateful_table.page_up(); - - assert_some_eq_x!( - stateful_table.filtered_state.as_ref().unwrap().selected(), - stateful_table.filtered_items.as_ref().unwrap().len() - 20 - ); - - stateful_table.scroll_down(); - stateful_table.page_down(); - - assert_some_eq_x!( - stateful_table.filtered_state.as_ref().unwrap().selected(), - 2 - ); } #[test] @@ -376,33 +416,105 @@ mod tests { assert_some_eq_x!(stateful_table.state.selected(), 20); + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 40); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 60); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 80); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 99); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 99); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 79); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 59); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 39); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 19); + stateful_table.page_up(); assert_some_eq_x!(stateful_table.state.selected(), 0); stateful_table.page_up(); - assert_some_eq_x!( - stateful_table.state.selected(), - stateful_table.items.len() - 21 - ); + assert_some_eq_x!(stateful_table.state.selected(), 0); + } - stateful_table.page_down(); + #[test] + fn test_stateful_table_pagination_on_twenty_one_item_table_stops_at_last_item() { + let mut stateful_table = StatefulTable::default(); + stateful_table.set_items((0..21).collect()); assert_some_eq_x!(stateful_table.state.selected(), 0); - stateful_table.scroll_down(); - stateful_table.page_up(); - - assert_some_eq_x!( - stateful_table.state.selected(), - stateful_table.items.len() - 20 - ); - - stateful_table.scroll_down(); stateful_table.page_down(); - assert_some_eq_x!(stateful_table.state.selected(), 2); + assert_some_eq_x!(stateful_table.state.selected(), 20); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 20); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 0); + } + + #[test] + fn test_stateful_table_pagination_on_five_item_table_stops_at_last_item() { + let mut stateful_table = StatefulTable::default(); + stateful_table.set_items((0..5).collect()); + + assert_some_eq_x!(stateful_table.state.selected(), 0); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 4); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 4); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 0); + } + + #[test] + fn test_stateful_table_pagination_on_four_item_table_reaches_last_item() { + let mut stateful_table = StatefulTable::default(); + stateful_table.set_items((0..4).collect()); + + assert_some_eq_x!(stateful_table.state.selected(), 0); + + stateful_table.page_down(); + + assert_some_eq_x!(stateful_table.state.selected(), 3); + + stateful_table.page_up(); + + assert_some_eq_x!(stateful_table.state.selected(), 0); } #[test]