Skip to content

feat(tui): add PageUp/PageDown scrolling to all panes#1656

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
major:feat/tui-pageup-pagedown
Jun 1, 2026
Merged

feat(tui): add PageUp/PageDown scrolling to all panes#1656
johntmyers merged 1 commit into
NVIDIA:mainfrom
major:feat/tui-pageup-pagedown

Conversation

@major
Copy link
Copy Markdown
Contributor

@major major commented Jun 1, 2026

Summary

Add PageUp/PageDown key support to the policy, logs, and draft/rules views in openshell term. All three scrollable panes now scroll by one viewport height per keypress, matching standard terminal conventions.

Also fixes a pre-existing scroll clamping bug where pressing G (go to bottom) or scrolling past the end would overshoot into a blank screen.

Related Issue

N/A (user-reported usability gap, no tracking issue)

Changes

  • Add policy_viewport_height field to App, set during draw by sandbox_policy::draw() (matches how logs/draft already store viewport height)
  • Handle KeyCode::PageDown / KeyCode::PageUp in handle_policy_key(), handle_logs_key(), and handle_draft_key()
  • Fix scroll_policy() max clamp to use viewport-aware bound (total - viewport) instead of total - 1
  • Fix G keybinding in policy view to use the same viewport-aware clamping
  • sandbox_policy::draw() takes &mut App (consistent with sandbox_logs::draw() and sandbox_draft::draw())
  • Logs PageDown/PageUp disables autoscroll (consistent with j/k behavior)
  • Draft PageUp guards on total > 0 (consistent with PageDown)
  • Extract clamped_scroll() helper with 10 unit tests covering empty content, short/equal/long content, pre-first-draw viewport, and boundary clamping

Testing

  • Built with cargo build --release -p openshell-cli
  • Manual testing: verified PageUp/PageDown in policy view with 90+ rules
  • Verified no blank-screen overshoot on PageDown at end of content
  • LSP diagnostics clean on all changed files
  • mise run pre-commit passes (rust:format:check, rust:lint, helm:lint, markdown:lint, license:check all green)
  • cargo test -p openshell-tui passes (22 tests, including 10 new scroll tests)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@major major requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners June 1, 2026 16:09
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jun 1, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@major
Copy link
Copy Markdown
Contributor Author

major commented Jun 1, 2026

I have read the DCO document and I hereby sign the DCO.

@major
Copy link
Copy Markdown
Contributor Author

major commented Jun 1, 2026

recheck

@major major force-pushed the feat/tui-pageup-pagedown branch 2 times, most recently from 9e944ab to 9e41eb2 Compare June 1, 2026 16:15
Add PageUp/PageDown key support to the policy, logs, and draft/rules
views. All three panes now scroll by one viewport height per keypress.

Also fix scroll_policy() clamping to stop at the last viewport of
content instead of the last line, preventing a blank-screen overshoot
on G and PageDown.

Signed-off-by: Major Hayden <major@redhat.com>
@major major force-pushed the feat/tui-pageup-pagedown branch from 9e41eb2 to ef00759 Compare June 1, 2026 16:32
@johntmyers
Copy link
Copy Markdown
Collaborator

/ok to test ef00759

@johntmyers johntmyers enabled auto-merge (squash) June 1, 2026 19:55
@johntmyers johntmyers merged commit eb97fb3 into NVIDIA:main Jun 1, 2026
25 checks passed
@major major deleted the feat/tui-pageup-pagedown branch June 1, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants