From 94541523e7e11543515828fbb238d98e996e2a25 Mon Sep 17 00:00:00 2001 From: altair823 Date: Mon, 4 May 2026 16:41:24 +0000 Subject: [PATCH] =?UTF-8?q?refactor(kebab-tui):=20p9-fb-24=20task=202=20?= =?UTF-8?q?=E2=80=94=20Inspect=20PgUp/PgDn=20via=20pager::PAGE=5FSTEP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor the Inspect pane's PageDown/PageUp handlers to consume the PAGE_STEP constant from pager.rs instead of hard-coding 10. Adds regression tests to pin the scroll delta (=10), ensuring future viewport-aware refactors surface here rather than silently in user-visible behaviour. Test coverage: added page_down_scrolls_by_ten_in_inspect and page_up_rewinds_by_ten_saturating_in_inspect (+ existing page_keys_scroll_by_ten still passes). Remove #[allow(dead_code)] from pager.rs now that PAGE_STEP is consumed. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/kebab-tui/src/inspect.rs | 4 ++-- crates/kebab-tui/src/pager.rs | 6 ------ crates/kebab-tui/tests/inspect.rs | 33 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/crates/kebab-tui/src/inspect.rs b/crates/kebab-tui/src/inspect.rs index 6fe3b36..39aa90f 100644 --- a/crates/kebab-tui/src/inspect.rs +++ b/crates/kebab-tui/src/inspect.rs @@ -426,11 +426,11 @@ pub fn handle_key_inspect(state: &mut App, key: KeyEvent) -> KeyOutcome { KeyOutcome::Continue } (KeyCode::PageDown, _) => { - s.scroll = s.scroll.saturating_add(10); + s.scroll = s.scroll.saturating_add(crate::pager::PAGE_STEP); KeyOutcome::Continue } (KeyCode::PageUp, _) => { - s.scroll = s.scroll.saturating_sub(10); + s.scroll = s.scroll.saturating_sub(crate::pager::PAGE_STEP); KeyOutcome::Continue } (KeyCode::Char('c'), _) => { diff --git a/crates/kebab-tui/src/pager.rs b/crates/kebab-tui/src/pager.rs index 7781f58..bdeabb1 100644 --- a/crates/kebab-tui/src/pager.rs +++ b/crates/kebab-tui/src/pager.rs @@ -8,10 +8,4 @@ //! lives behind this single edit point. /// Rows scrolled per `PgUp` / `PgDn` keystroke. -/// -/// `#[allow(dead_code)]` is intentional for the Task 1 commit only — -/// Task 2 (Inspect refactor) immediately consumes the constant and -/// removes this attribute. Without it, `cargo clippy -- -D warnings` -/// rejects this commit alone, breaking the per-task review gate. -#[allow(dead_code)] pub(crate) const PAGE_STEP: u16 = 10; diff --git a/crates/kebab-tui/tests/inspect.rs b/crates/kebab-tui/tests/inspect.rs index ef34a85..3667731 100644 --- a/crates/kebab-tui/tests/inspect.rs +++ b/crates/kebab-tui/tests/inspect.rs @@ -188,6 +188,39 @@ fn page_keys_scroll_by_ten() { assert_eq!(app.inspect.as_ref().unwrap().scroll, 0); } +/// p9-fb-24 task 2: PageDown advances scroll by `PAGE_STEP` (= 10). +/// Pins the constant so a future viewport-aware refactor surfaces +/// here, not silently in user-visible behaviour. +#[test] +fn page_down_scrolls_by_ten_in_inspect() { + let mut app = fresh_app(); + let outcome = handle_key_inspect( + &mut app, + KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE), + ); + assert_eq!(outcome, KeyOutcome::Continue); + assert_eq!(app.inspect.as_ref().unwrap().scroll, 10); +} + +/// p9-fb-24 task 2: PageUp rewinds scroll by `PAGE_STEP`, saturating +/// at 0 (no underflow). +#[test] +fn page_up_rewinds_by_ten_saturating_in_inspect() { + let mut app = fresh_app(); + app.inspect.as_mut().unwrap().scroll = 25; + handle_key_inspect( + &mut app, + KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE), + ); + assert_eq!(app.inspect.as_ref().unwrap().scroll, 15); + app.inspect.as_mut().unwrap().scroll = 3; + handle_key_inspect( + &mut app, + KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE), + ); + assert_eq!(app.inspect.as_ref().unwrap().scroll, 0); +} + #[test] fn c_toggles_collapse_state() { let mut app = fresh_app();