Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ All notable user-visible changes to Hunk are documented in this file.
### Fixed

- Smoothed mouse-wheel review scrolling so small diffs stay precise while sustained wheel gestures still speed up.
- Fixed Shift+mouse-wheel horizontal scrolling so it no longer leaks a one-line vertical scroll in some terminals.

## [0.9.4] - 2026-04-14

Expand Down
34 changes: 34 additions & 0 deletions src/ui/AppHost.interactions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,40 @@ describe("App interactions", () => {
}
});

test("shift plus native horizontal wheel events do not move the vertical review position", async () => {
const setup = await testRender(<AppHost bootstrap={createWrapScrollBootstrap()} />, {
width: 92,
height: 20,
});

try {
await flush(setup);

let frame = setup.captureCharFrame();
const initialTopLine = firstVisibleAddedLineNumber(frame);
expect(initialTopLine).toBeTruthy();
expect(frame).not.toContain("viewport anchoring");

for (let index = 0; index < 8; index += 1) {
await act(async () => {
await setup.mockMouse.scroll(60, 10, "right", { modifiers: { shift: true } });
});
await flush(setup);
frame = setup.captureCharFrame();
if (frame.includes("viewport anchoring")) {
break;
}
}

expect(frame).toContain("viewport anchoring");
expect(firstVisibleAddedLineNumber(frame)).toBe(initialTopLine);
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});

test("wrap toggles reset the horizontal code offset", async () => {
const setup = await testRender(<AppHost bootstrap={createWrapBootstrap()} />, {
width: 92,
Expand Down
15 changes: 13 additions & 2 deletions src/ui/components/panes/DiffPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export function DiffPane({

const preservedScrollTop = scrollBox.scrollTop;
const preservedScrollLeft = scrollBox.scrollLeft;
const scrollInfo = event.scroll;

if (direction === "left") {
onScrollCodeHorizontally(-1);
Expand All @@ -217,15 +218,25 @@ export function DiffPane({
return;
}

// OpenTUI runs ScrollBox's own wheel handler after this listener and it does not honor
// preventDefault(), so restore the pre-event viewport position on the next microtask.
// OpenTUI runs ScrollBox's own wheel handler after this listener and it ignores
// preventDefault(). Zero the wheel delta first so native Shift+Wheel left/right events
// cannot be remapped back into vertical scroll, then restore the viewport and clear any
// residual fractional state on the next microtask as a final guard.
if (scrollInfo) {
scrollInfo.delta = 0;
}

queueMicrotask(() => {
const currentScrollBox = scrollRef.current;
if (!currentScrollBox) {
return;
}

currentScrollBox.scrollTo({ x: preservedScrollLeft, y: preservedScrollTop });
currentScrollBox.scrollAcceleration.reset();
(
currentScrollBox as unknown as { resetScrollAccumulators?: () => void }
).resetScrollAccumulators?.();
Comment on lines +236 to +239
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent optional chaining on internal APIs

scrollAcceleration.reset() is called without optional chaining, while resetScrollAccumulators is accessed via as unknown cast with ?.() precisely because it may not exist. If scrollAcceleration is also absent on the current scroll box object (e.g., a different runtime version of OpenTUI, or a test mock), this will throw a TypeError inside the microtask — an uncatchable silent failure. For consistency with the pattern already used on the next line, consider guarding it the same way:

Suggested change
currentScrollBox.scrollAcceleration.reset();
(
currentScrollBox as unknown as { resetScrollAccumulators?: () => void }
).resetScrollAccumulators?.();
currentScrollBox.scrollAcceleration?.reset();
(
currentScrollBox as unknown as { resetScrollAccumulators?: () => void }
).resetScrollAccumulators?.();
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/components/panes/DiffPane.tsx
Line: 231-234

Comment:
**Inconsistent optional chaining on internal APIs**

`scrollAcceleration.reset()` is called without optional chaining, while `resetScrollAccumulators` is accessed via `as unknown` cast with `?.()` precisely because it may not exist. If `scrollAcceleration` is also absent on the current scroll box object (e.g., a different runtime version of OpenTUI, or a test mock), this will throw a `TypeError` inside the microtask — an uncatchable silent failure. For consistency with the pattern already used on the next line, consider guarding it the same way:

```suggestion
        currentScrollBox.scrollAcceleration?.reset();
        (
          currentScrollBox as unknown as { resetScrollAccumulators?: () => void }
        ).resetScrollAccumulators?.();
```

How can I resolve this? If you propose a fix, please make it concise.

});

event.preventDefault();
Expand Down
Loading