fix: Fix container-query driven layout oscillation caused by scrollbars#4615
fix: Fix container-query driven layout oscillation caused by scrollbars#4615avinashbot wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4615 +/- ##
==========================================
+ Coverage 97.50% 97.52% +0.01%
==========================================
Files 948 948
Lines 30275 30364 +89
Branches 11039 11079 +40
==========================================
+ Hits 29520 29611 +91
+ Misses 748 706 -42
- Partials 7 47 +40 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| * can grow/shrink the page enough to toggle the viewport scrollbar, which in turn changes the | ||
| * measured width and flips the breakpoint back — an infinite layout loop (see AWSUI-62065). | ||
| */ | ||
| const BREAKPOINT_SWITCH_OFFSET = 20; |
There was a problem hiding this comment.
While this is probably fine, I am wondering if we could/should calculate the width of the scrollbar. This way, the approach would also work for cases where the scrollbar is bigger than 20px. Note: As of now, all default scrollbar sizes are <= 17px (https://codepen.io/sambible/post/browser-scrollbar-widths) - but could theoretically be changed by browser extensions (or even ourselves through CSS).
There was a problem hiding this comment.
I could go either way on that, I was conflicted about this.
We actually already have a utility for it (browser-scrollbar-size.ts) but the only reliable way of doing is to modify the DOM and measure the elements, which is a bit of a worrying performance trap (not something I noticed on my Macbook on a simple dev page, but breakpoint measurements are pretty foundational and used in a lot of places). Plus, having a fixed number makes it more predictable between test and real environments (for example, we don't have any testing for overlay scrollbars, even browser/screenshot tests).
Description
Cleanest solution I could find for the issue in the ticket — make breakpoint changes "stick" or "lag" into a boundary between two breakpoints. This prevents any added scrollbars from making the breakpoint calculation unstable. Of course, this now means that adjacent breakpoints values are implicitly dependent on the current breakpoint, but only by 20px, so we're not messing up people's layouts too badly.
I use term "stickiness" in the code; I imagine it like a kind of magnet that doesn't let go until you pass a certain distance in either direction of the numerical boundary values.
Related links, issue #, if available: AWSUI-62065
How has this been tested?
Kiro wrote me up some unit tests, but there's a dev page recreating the original ticket scenario.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.