-
-
Notifications
You must be signed in to change notification settings - Fork 187
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix jumpy card list UI on medium and large screens
This commit fixes layout shifts that occur on card list part of the page when the page is initially loaded. - Resolve issue where card list starts with minimal width, leading to jumps in UI until correct width is calculated on medium and big screens. - Dispose of existing `ResizeObserver` properly before creating a new one. This prevents leaks and incorrect width calculations if `containerElement` changes. - Throttle resize events to minimize width/height calculation changes, enhancing performance and reducing the chances for layout shifts. Supporting CI/CD improvements: - Fix CI/CD cannot upload artifacts when E2E tests fail. - Fix uploded artifacts being merged for different operating systems by adding the name of the used operating system in uploaded artifact file.
- Loading branch information
1 parent
3864f04
commit 22af57e
Showing
7 changed files
with
234 additions
and
63 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,52 +1,67 @@ | ||
| @mixin hover-or-touch($selector-suffix: '', $selector-prefix: '&') { | ||
| @media (hover: hover) { | ||
| /* We only do this if hover is truly supported; otherwise the emulator in mobile | ||
| @media (hover: hover) { | ||
|
|
||
| /* We only do this if hover is truly supported; otherwise the emulator in mobile | ||
| keeps hovered style in-place even after touching, making it sticky. */ | ||
| #{$selector-prefix}:hover #{$selector-suffix} { | ||
| @content; | ||
| } | ||
| #{$selector-prefix}:hover #{$selector-suffix} { | ||
| @content; | ||
| } | ||
| @media (hover: none) { | ||
| /* We only do this if hover is not supported,otherwise the desktop behavior is not | ||
| } | ||
|
|
||
| @media (hover: none) { | ||
|
|
||
| /* We only do this if hover is not supported,otherwise the desktop behavior is not | ||
| as desired; it does not get activated on hover but only during click/touch. */ | ||
| #{$selector-prefix}:active #{$selector-suffix} { | ||
| @content; | ||
| } | ||
| #{$selector-prefix}:active #{$selector-suffix} { | ||
| @content; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @mixin clickable($cursor: 'pointer') { | ||
| cursor: #{$cursor}; | ||
| user-select: none; | ||
| /* | ||
| cursor: #{$cursor}; | ||
| user-select: none; | ||
| /* | ||
| It removes (blue) background during touch as seen in mobile webkit browsers (Chrome, Safari, Edge). | ||
| The default behavior is that any element (or containing element) that has cursor:pointer | ||
| explicitly set and is clicked will flash blue momentarily. | ||
| Removing it could have accessibility issue since that hides an interactive cue. But as we still provide | ||
| response to user actions through :active by `hover-or-touch` mixin. | ||
| */ | ||
| -webkit-tap-highlight-color: transparent; | ||
| -webkit-tap-highlight-color: transparent; | ||
| } | ||
|
|
||
| @mixin fade-transition($name) { | ||
| .#{$name}-enter-active, | ||
| .#{$name}-leave-active { | ||
| transition: opacity 0.3s ease; | ||
| } | ||
|
|
||
| .#{$name}-enter-from, | ||
| .#{$name}-leave-to { | ||
| opacity: 0; | ||
| } | ||
| } | ||
|
|
||
| @mixin fade-slide-transition($name, $duration, $offset-upward: null) { | ||
| .#{$name}-enter-active, | ||
| .#{$name}-leave-active { | ||
| transition: all $duration; | ||
| } | ||
|
|
||
| .#{$name}-leave-active, | ||
| .#{$name}-enter-from | ||
| { | ||
| opacity: 0; | ||
| .#{$name}-enter-active, | ||
| .#{$name}-leave-active { | ||
| transition: all $duration; | ||
| } | ||
|
|
||
| @if $offset-upward { | ||
| transform: translateY($offset-upward); | ||
| } | ||
| .#{$name}-leave-active, | ||
| .#{$name}-enter-from { | ||
| opacity: 0; | ||
|
|
||
| @if $offset-upward { | ||
| transform: translateY($offset-upward); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @mixin reset-ul { | ||
| margin: 0; | ||
| padding: 0; | ||
| list-style: none; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| import { waitForHeaderBrandTitle } from './shared/ApplicationLoad'; | ||
|
|
||
| describe('card list layout stability', () => { | ||
| describe('during initial page load', () => { | ||
| const testScenarios: ReadonlyArray<{ | ||
| readonly name: string; | ||
| readonly width: number; | ||
| readonly height: number; | ||
| }> = [ | ||
| { name: 'iPhone SE', width: 375, height: 667 }, | ||
| { name: '13-inch Laptop', width: 1280, height: 800 }, | ||
| { name: '4K Ultra HD Desktop', width: 3840, height: 2160 }, // regression bug | ||
| ]; | ||
| testScenarios.forEach(({ name, width, height }) => { | ||
| it(`ensures layout stability on ${name}`, () => { | ||
| // arrange | ||
| cy.viewport(width, height); | ||
| const dimensions = new Array<SizeDimensions>(); | ||
| const addDimension = (newDimension: SizeDimensions) => { | ||
| if (dimensions.length > 0) { | ||
| const lastDimension = dimensions[dimensions.length - 1]; | ||
| if (lastDimension.width === newDimension.width | ||
| && lastDimension.height === newDimension.height) { | ||
| return; | ||
| } | ||
| } | ||
| dimensions.push(newDimension); | ||
| }; | ||
| const capturer = new ContinuousSizeMonitor(); | ||
|
|
||
| // act | ||
| cy.window().then((win) => { | ||
| // We need to start capturing size changes before visiting the page (loading) as this | ||
| // layout shift/jump happens rapidly and will not be visible once the page is loaded. | ||
| capturer.startCapturing(win, (metric) => { | ||
| addDimension(metric); | ||
| }); | ||
| }); | ||
| cy.visit('/'); | ||
| capturer.stopCapturing(); | ||
| const captureMetrics = () => cy.window().then( | ||
| (win) => { | ||
| const cardList = findCardList(win); | ||
| if (cardList) { | ||
| addDimension(captureDimensions(cardList)); | ||
| } | ||
| }, | ||
| ); | ||
| captureMetrics(); | ||
| for (const waitUntilNextCheckpoint of Object.values(checkpoints)) { | ||
| waitUntilNextCheckpoint(); | ||
| captureMetrics(); | ||
| } | ||
|
|
||
| // assert | ||
| cy.document().then(() => { | ||
| const widthTolerance = 0; | ||
| const widths = new Set(dimensions.map((d) => d.width)); | ||
| expect(isWithinTolerance(widths, widthTolerance)).to.equal(true, [ | ||
| `Unique width values over time: ${[...widths].join(', ')}`, | ||
| `Height changes are more than ${widthTolerance}px tolerance`, | ||
| `Captured metrics: ${JSON.stringify(dimensions)}`, | ||
| ].join('\n\n')); | ||
|
|
||
| const heightTolerance = 100; | ||
| const heights = new Set(dimensions.map((d) => d.height)); | ||
| expect(isWithinTolerance(heights, heightTolerance)).to.equal(true, [ | ||
| `Unique height values over time: ${[...heights].join(', ')}`, | ||
| `Height changes are more than ${heightTolerance}px tolerance`, | ||
| `Captured metrics: ${JSON.stringify(dimensions)}`, | ||
| ].join('\n\n')); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| function findCardList(win: Cypress.AUTWindow): Element | undefined { | ||
| return win.document.querySelector('.cards') || undefined; | ||
| } | ||
|
|
||
| class ContinuousSizeMonitor { | ||
| private capturer: ReturnType<typeof setTimeout> | undefined; | ||
|
|
||
| public startCapturing(win: Cypress.AUTWindow, callback: (metric: SizeDimensions) => void) { | ||
| this.capturer = setInterval(() => { | ||
| const cardList = findCardList(win); | ||
| if (cardList) { | ||
| const metrics = captureDimensions(cardList); | ||
| callback(metrics); | ||
| } | ||
| }, 5); | ||
| } | ||
|
|
||
| public stopCapturing() { | ||
| clearInterval(this.capturer); | ||
| } | ||
| } | ||
|
|
||
| function isWithinTolerance( | ||
| numbers: Iterable<number>, | ||
| tolerance: number, | ||
| ) { | ||
| let changeWithinTolerance = true; | ||
| const values = [...numbers]; | ||
| const [firstValue, ...otherValues] = values; | ||
| let previousValue = firstValue; | ||
| otherValues.forEach((value) => { | ||
| const difference = Math.abs(value - previousValue); | ||
| if (difference > tolerance) { | ||
| changeWithinTolerance = false; | ||
| } | ||
| previousValue = value; | ||
| }); | ||
| return changeWithinTolerance; | ||
| } | ||
|
|
||
| interface SizeDimensions { | ||
| readonly width: number; | ||
| readonly height: number; | ||
| } | ||
|
|
||
| function captureDimensions(element: Element): SizeDimensions { | ||
| const dimensions = element.getBoundingClientRect(); // more reliable than body.scroll... | ||
| return { | ||
| width: Math.round(dimensions.width), | ||
| height: Math.round(dimensions.height), | ||
| }; | ||
| } | ||
|
|
||
| enum ApplicationLoadStep { | ||
| IndexHtmlLoaded = 0, | ||
| AppVueLoaded = 1, | ||
| HeaderBrandTitleLoaded = 2, | ||
| CardListLoaded = 3, | ||
| } | ||
|
|
||
| const checkpoints: Record<ApplicationLoadStep, () => void> = { | ||
| [ApplicationLoadStep.IndexHtmlLoaded]: () => cy.get('#app'), | ||
| [ApplicationLoadStep.AppVueLoaded]: () => cy.get('.app__wrapper').should('be.visible'), | ||
| [ApplicationLoadStep.HeaderBrandTitleLoaded]: () => waitForHeaderBrandTitle(), | ||
| [ApplicationLoadStep.CardListLoaded]: () => cy.get('.cards').should('be.visible'), | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export function waitForHeaderBrandTitle() { | ||
| cy.contains('h1', 'privacy.sexy'); | ||
| } |