fix(SSE): smooth tab bar scrolling with mouse wheel and buttons#138
fix(SSE): smooth tab bar scrolling with mouse wheel and buttons#138maxm11 wants to merge 2 commits into
Conversation
Tab navigation routed through setTabVisible('forward'/'backward'), which
snaps just far enough to reveal the next partially hidden tab rather than
scrolling by pixels. This made the scroll buttons jump an inconsistent,
often tiny amount, and the mouse wheel (throttled on Mac) felt clunky.
Rewrite the wheel handler to scroll the bar by the real pixel delta of
the wheel event (handling both deltaX trackpad swipes and deltaY wheels,
normalizing line/page deltas), and make the scroll buttons move a
consistent ~one-page amount. Both go through a shared scrollByDelta
helper that keeps the trailing spacer, RTL handling and button enable
state intact.
Fixes Euro-Office#121
Agentic Model: Claude Opus 4.8
Signed-off-by: Max Murphy <maxmrphy@gmail.com>
maxm11
requested review from
MonaAghili and
j-base64
and removed request for
a team
| var delta = e.deltaX || e.deltaY; | ||
| if (delta === undefined || delta === null) { // legacy mousewheel/DOMMouseScroll | ||
| delta = e.wheelDelta ? -e.wheelDelta : (e.detail || 0); | ||
| } |
There was a problem hiding this comment.
e.deltaX || e.deltaY on line 352 both of them are always numbers on a wheel event they never will be undefined or null!!
so the condition on 353 is never true
e.wheelDelta and e.detail are both undefined on wheel events anyway, so delta would become 0 and the !delta check on line 356 would return
I think
if (delta === undefined || delta === null) { // legacy mousewheel/DOMMouseScroll
delta = e.wheelDelta ? -e.wheelDelta : (e.detail || 0);
}
is dead code and should be dropped(lines 353–354 entirely)
There was a problem hiding this comment.
Good catch, thanks — you're right that this handler only ever receives WheelEvent instances (bound via addEventListener('wheel', ...)), so deltaX/deltaY are always numbers and the legacy wheelDelta/detail fallback is unreachable. Dropped it in 5772a6a.
|
Other than the dead code comment I left inline every other thing LGTM. |
The handler is only ever bound via addEventListener('wheel', ...), so
e is always a WheelEvent where deltaX/deltaY are always numbers
(default 0), never undefined/null. The legacy wheelDelta/detail
fallback was unreachable dead code.
Signed-off-by: Max Murphy <maxmrphy@gmail.com>
|
@MonaAghili addressed your comment on the mousewheel fallback — replied inline and pushed 5772a6a. Note: I don't have permission to formally request a re-review on this fork-based PR, so flagging here instead. |
2 participants
Summary
Fixes #121 — scrolling the spreadsheet sheet-tab bar was clunky: the buttons jumped an inconsistent (often tiny) amount and the mouse wheel did not scroll smoothly.
The tab bar (
<ul id="statusbar_bottom">) is the scroll container, but all navigation routed throughsetTabVisible('forward'/'backward'), which snaps just far enough to reveal the next partially-hidden tab instead of scrolling by pixels. That caused the erratic button jumps, and the mouse wheel (throttled on Mac) fed the same discrete snap.Changes
_onMouseWheelnow reads the realwheelevent delta (bothdeltaXtrackpad swipes anddeltaYwheels), normalizes line/page deltas to pixels, and applies it directly toscrollLeft. Removed the Mac-only throttled handler and the legacymousewheel/DOMMouseScrollbranch — a singlewheellistener covers all browsers.scrollByPage()moves ~one visible width per click instead of the unpredictable next-tab snap.scrollByDeltahelper that keeps the trailing spacer (so the last tab clears the overlapping status-bar buttons), handles RTL's negative-scroll model, lets the browser clampscrollLeft, and firescheckInvisible()to refresh the arrow-button enabled state.TabBarcomponent). The numericsetTabVisible(index)path used to scroll the active sheet into view is untouched.Testing
All three files pass
node --check. Worth a manual smoke test in a built editor across LTR/RTL and trackpad/notched-wheel before merge.Agentic Model: Claude Opus 4.8