fix(segment-view): prevent NaN/Infinity scrollRatio when a single content item is present#31113
fix(segment-view): prevent NaN/Infinity scrollRatio when a single content item is present#31113OS-susmitabhowmik wants to merge 5 commits intonextfrom
Conversation
…tent item is present
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Nice! Just a couple things to make this bullet-proof
| const { scrollLeft, scrollWidth, clientWidth } = ev.target as HTMLElement; | ||
| const max = scrollWidth - clientWidth; | ||
| // When only one content item is present max is 0 — skip to avoid NaN/Infinity scrollRatio. | ||
| if (max <= 0) return; |
There was a problem hiding this comment.
Mind adding a quick Playwright test for this in test/basic/segment-view.e2e.ts? A single ion-segment-content that scrolls and asserts ionSegmentViewScroll doesn't fire (or fires with a finite scrollRatio) would lock the regression in. Without one, the next refactor of handleScroll can put the NaN back silently.
| const { scrollLeft, scrollWidth, clientWidth } = ev.target as HTMLElement; | ||
| const max = scrollWidth - clientWidth; | ||
| // When only one content item is present max is 0 — skip to avoid NaN/Infinity scrollRatio. | ||
| if (max <= 0) return; |
There was a problem hiding this comment.
The early return also skips resetScrollEndTimeout(). If setContent set isManualScroll = false and started the timer, then a scroll event lands in the max <= 0 branch, the timer doesn't get extended on this event and isManualScroll could clear early. I think it's fine in practice since a non-overflowing element shouldn't really fire scroll events, but worth a sanity check to just go ahead and resetScrollEndTimeout in the if body.
There was a problem hiding this comment.
Updated to include resetScrollEndTimeout within the if block as a part of this commit dcbf391
…ngle content item
2 participants
Issue number: resolves internal
What is the current behavior?
Currently if there is a single content item present the
scrollRatiois computed asNaNorInfinityresulting in a console error.What is the new behavior?
Updates
handleScrollinion-segment-viewto skiphandleScrollifmax <= 0Does this introduce a breaking change?
Other information