Skip to content

fix(ui): backport bottom-edge scroll snap-back fix to 0.9.x#196

Merged
benvinegar merged 1 commit into
0.9.xfrom
backport/0.9.2-bottom-edge-scroll-trap
Apr 12, 2026
Merged

fix(ui): backport bottom-edge scroll snap-back fix to 0.9.x#196
benvinegar merged 1 commit into
0.9.xfrom
backport/0.9.2-bottom-edge-scroll-trap

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • backport the bottom-edge scroll snap-back fix from main onto 0.9.x
  • keep this as a code-only backport and leave release notes/changelog handling to the release work
  • preserve the new component and PTY regressions on the maintenance branch

Notes

  • This intentionally omits the CHANGELOG.md update because 0.9.x predates that file.
  • Backport of f6de1cb from main without the changelog change.

Testing

  • bun run typecheck
  • bun run lint
  • bun run format:check
  • bun test
  • bun run test:integration

This PR description was generated by Pi using OpenAI o3

Backport of f6de1cb from main without the changelog update.
@greptile-apps

greptile-apps Bot commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR backports the bottom-edge scroll snap-back fix (f6de1cb) from main onto 0.9.x. The core fix changes the settle check in the file-top-alignment useLayoutEffect to compare currentTop against clampReviewScrollTop(targetSection.bodyTop, viewportHeight) — the actually reachable target — rather than the raw bodyTop, which may be beyond contentHeight - viewportHeight for short trailing files. The hunk-reveal and note-reveal scroll paths also receive the same clamping. New unit test and PTY regression coverage is preserved from main.

Confidence Score: 5/5

Safe to merge — targeted fix with full unit and PTY regression coverage, no regressions identified.

All findings are P2 or lower. The logic change is minimal and self-consistent: both the scroll dispatch (scrollFileHeaderToTop) and the settle comparison (pendingFileTopAlignFileIdRef effect) use the same clampReviewScrollTop call against the same totalContentHeight, so they agree on what settled means. New tests directly exercise the scenario that was broken.

No files require special attention.

Important Files Changed

Filename Overview
src/ui/components/panes/DiffPane.tsx Adds clampVerticalScrollTop/clampReviewScrollTop helpers and applies them to the file-top-align settle check, scrollFileHeaderToTop, and both hunk/note reveal scroll paths to prevent bottom-edge snap-back.
src/ui/components/ui-components.test.tsx Adds viewport-sized bottom-hunk helpers (plain and wrapped variants) and three new component-level tests covering bottom-clamped alignment and viewport-sized hunk visibility.
test/pty/harness.ts Adds createBottomClampedRepoFixture (30-line first file + 3-line short second file) and exports it from the harness for PTY regression coverage.
test/pty/ui-integration.test.ts Adds end-to-end PTY test that navigates to a short trailing file, then verifies four up-key presses successfully move the viewport back into the first file's content.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[selectedFileTopAlignRequestId changes] --> B[scrollFileHeaderToTop called]
    B --> C["scrollTo(clampReviewScrollTop(bodyTop, vh))"]
    C --> D[Scroll box settles at reachable bottom edge]
    D --> E[Settle-check useLayoutEffect runs]
    E --> F{"currentTop ≈ clampReviewScrollTop(bodyTop, vh)?"}
    F -- "Yes (new: clamped target matches)" --> G[clearPendingFileTopAlign]
    G --> H[Manual upward scrolling works freely]
    F -- "No (old: raw bodyTop was unreachable)" --> I[scrollFileHeaderToTop re-called]
    I --> J[Snap-back loop — manual scroll fought]
Loading

Reviews (1): Last reviewed commit: "fix(ui): stop bottom-edge scroll snap-ba..." | Re-trigger Greptile

@benvinegar
benvinegar merged commit bbc58f1 into 0.9.x Apr 12, 2026
3 checks passed
@benvinegar
benvinegar deleted the backport/0.9.2-bottom-edge-scroll-trap branch April 12, 2026 02:00
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