Fix card list UI layout shifts (jumps) on load

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:

- Enable artifact upload in CI/CD even if E2E tests fail.
- Distinguish uploaded artifacts by operating system for clarity.

.github/workflows/tests.e2e.yaml

@@ -27,6 +27,7 @@ jobs:
       -
         name: Output artifact directories
         id: artifacts
+        if: always() # Run even if previous steps fail because test run video is always captured
         shell: bash
         run: |-
           declare -r dirs_json_file='cypress-dirs.json'
@@ -49,15 +50,15 @@ jobs:
           echo "VIDEOS_DIR=${VIDEOS_DIR}" >> "${GITHUB_OUTPUT}"
       -
         name: Upload screenshots
-        if: failure() # Run only if previous step fails because screenshots will be generated only if E2E test failed
+        if: failure() # Run only if previous steps fail because screenshots will be generated only if E2E test failed
         uses: actions/upload-artifact@v3
         with:
-          name: e2e-screenshots
+          name: e2e-screenshots-${{ matrix.os }}
           path: ${{ steps.artifacts.outputs.SCREENSHOTS_DIR }}
       -
         name: Upload videos
-        if: always() # Run even if previous step fails because test run video is always captured
+        if: always() # Run even if previous steps fail because test run video is always captured
         uses: actions/upload-artifact@v3
         with:
-          name: e2e-videos
+          name: e2e-videos-${{ matrix.os }}
           path: ${{ steps.artifacts.outputs.VIDEOS_DIR }}

cypress.config.ts

@@ -14,6 +14,12 @@ export default defineConfig({
     specPattern: `${cypressDirs.base}/**/*.cy.{js,jsx,ts,tsx}`, // Default: cypress/e2e/**/*.cy.{js,jsx,ts,tsx}
     supportFile: `${cypressDirs.base}/support/e2e.ts`,
   },
+
+  /*
+    Disabling Chrome's web security to allow for faster DOM queries to access DOM earlier than
+    `cy.get()`. It bypasses the usual same-origin policy constraints
+  */
+  chromeWebSecurity: false,
 });
 
 function getApplicationPort(): number {

src/presentation/assets/styles/_mixins.scss

@@ -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;
-}
+}

src/presentation/components/Scripts/View/Cards/CardList.vue

@@ -1,34 +1,36 @@
 <template>
   <SizeObserver v-on:widthChanged="width = $event">
-    <!--
-      <div id="responsivity-debug">
-        Width: {{ width || 'undefined' }}
-        Size:
-          <span v-if="width <= 500">small</span>
-          <span v-if="width > 500 && width < 750">medium</span>
-          <span v-if="width >= 750">big</span>
+    <transition name="fade-transition">
+      <div v-if="width">
+        <!-- <div id="responsivity-debug">
+          Width: {{ width || 'undefined' }}
+          Size:
+            <span v-if="width <= 500">small</span>
+            <span v-if="width > 500 && width < 750">medium</span>
+            <span v-if="width >= 750">big</span>
+        </div> -->
+        <div
+          v-if="categoryIds.length > 0"
+          class="cards"
+        >
+          <CardListItem
+            class="card"
+            v-bind:class="{
+              'small-screen': width <= 500,
+              'medium-screen': width > 500 && width < 750,
+              'big-screen': width >= 750,
+            }"
+            v-for="categoryId of categoryIds"
+            :data-category="categoryId"
+            v-bind:key="categoryId"
+            :categoryId="categoryId"
+            :activeCategoryId="activeCategoryId"
+            v-on:cardExpansionChanged="onSelected(categoryId, $event)"
+          />
+        </div>
+        <div v-else class="error">Something went bad 😢</div>
       </div>
-    -->
-    <div
-      v-if="categoryIds.length > 0"
-      class="cards"
-    >
-      <CardListItem
-        class="card"
-        v-bind:class="{
-          'small-screen': width <= 500,
-          'medium-screen': width > 500 && width < 750,
-          'big-screen': width >= 750,
-        }"
-        v-for="categoryId of categoryIds"
-        :data-category="categoryId"
-        v-bind:key="categoryId"
-        :categoryId="categoryId"
-        :activeCategoryId="activeCategoryId"
-        v-on:cardExpansionChanged="onSelected(categoryId, $event)"
-      />
-    </div>
-    <div v-else class="error">Something went bad 😢</div>
+    </transition>
   </SizeObserver>
 </template>
 
@@ -49,7 +51,8 @@ export default defineComponent({
   setup() {
     const { currentState, onStateChange } = injectKey((keys) => keys.useCollectionState);
 
-    const width = ref<number>(0);
+    const width = ref<number | undefined>();
+
     const categoryIds = computed<readonly number[]>(
       () => currentState.value.collection.actions.map((category) => category.id),
     );
@@ -138,4 +141,6 @@ function isClickable(element: Element) {
   font-size: 3.5em;
   font-family: $font-normal;
 }
+
+@include fade-transition('fade-transition');
 </style>

src/presentation/components/Shared/SizeObserver.vue

@@ -9,6 +9,7 @@ import {
   defineComponent, shallowRef, onMounted, onBeforeUnmount, watch,
 } from 'vue';
 import { useResizeObserverPolyfill } from '@/presentation/components/Shared/Hooks/UseResizeObserverPolyfill';
+import { throttle } from '@/presentation/components/Shared/Throttle';
 
 export default defineComponent({
   emits: {
@@ -34,10 +35,11 @@ export default defineComponent({
           return;
         }
         resizeObserverReady.then(() => {
-          observer = new ResizeObserver(updateSize);
+          disposeObserver();
+          observer = new ResizeObserver(throttle(updateSize, 200));
           observer.observe(element);
         });
-        updateSize();
+        updateSize(); // Do not throttle, immediately inform new width
       }, { immediate: true });
     });
 

tests/e2e/card-list-layout-stability-on-load.cy.ts

@@ -0,0 +1,231 @@
+// eslint-disable-next-line max-classes-per-file
+import { waitForHeaderBrandTitle } from './shared/ApplicationLoad';
+
+interface Stoppable {
+  stop(): void;
+}
+
+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 },
+    ];
+    const testCleanup = new Array<Stoppable>();
+    afterEach(() => {
+      testCleanup.forEach((c) => c.stop());
+      testCleanup.length = 0;
+    });
+    testScenarios.forEach(({ name, width, height }) => {
+      it(`ensures layout stability on ${name}`, () => {
+        // arrange
+        const dimensions = new DimensionsStorage();
+        cy.viewport(width, height);
+        // act
+        cy.window().then((win) => {
+          findElementFast(win, '.cards', (cardList) => {
+            testCleanup.push(
+              new SizeMonitor().start(cardList, () => dimensions.add(captureDimensions(cardList))),
+            );
+          });
+          testCleanup.push(
+            new ContinuousRunner()
+              .start(() => {
+                /*
+                  As Cypress does not inherently support CPU throttling, this workaround is used to
+                  intentionally slow down Cypress's execution. It allows capturing sudden layout
+                  issues, such as brief flashes or shifts.
+                */
+                cy.window().then(() => {
+                  cy.log('Throttling');
+                  // eslint-disable-next-line cypress/no-unnecessary-waiting
+                  cy.wait(50, { log: false });
+                });
+              }, 100),
+          );
+        });
+        cy.visit('/');
+        for (const assertNextCheckpoint of Object.values(checkpoints)) {
+          assertNextCheckpoint();
+        }
+
+        // assert
+        const widthToleranceInPx = 0;
+        const widthsInPx = dimensions.getUniqueWidths();
+        expect(isWithinTolerance(widthsInPx, widthToleranceInPx)).to.equal(true, [
+          `Unique width values over time: ${[...widthsInPx].join(', ')}`,
+          `Height changes are more than ${widthToleranceInPx}px tolerance`,
+          `Captured metrics: ${dimensions.toString()}`,
+        ].join('\n\n'));
+
+        const heightToleranceInPx = 100; // Set in relation to card sizes.
+        // Tolerance allows for minor layout shifts without (e.g. for icon or font loading)
+        // false test failures. The number `100` accounts for shifts when the number of
+        // cards per row changes, avoiding failures for shifts less than the smallest card
+        // size (~175px).
+        const heightsInPx = dimensions.getUniqueHeights();
+        expect(isWithinTolerance(heightsInPx, heightToleranceInPx)).to.equal(true, [
+          `Unique height values over time: ${[...heightsInPx].join(', ')}`,
+          `Height changes are more than ${heightToleranceInPx}px tolerance`,
+          `Captured metrics: ${dimensions.toString()}`,
+        ].join('\n\n'));
+      });
+    });
+  });
+});
+
+/*
+  It finds a DOM element as quickly as possible.
+  It's crucial for detecting early layout shifts during page load,
+  which may be missed by standard Cypress commands such as `cy.get`, `cy.document`.
+*/
+function findElementFast(
+  win: Cypress.AUTWindow,
+  query: string,
+  handler: (element: Element) => void,
+  timeoutInMs = 5000,
+): void {
+  const endTime = Date.now() + timeoutInMs;
+  const finder = new ContinuousRunner();
+  finder.start(() => {
+    const element = win.document.querySelector(query);
+    if (element) {
+      handler(element);
+      finder.stop();
+      return;
+    }
+    if (Date.now() >= endTime) {
+      finder.stop();
+      throw new Error(`Timed out. Failed to find element. Query: ${query}. Timeout: ${timeoutInMs}ms`);
+    }
+  }, 1 /* As aggressive as possible */);
+}
+
+class DimensionsStorage {
+  private readonly dimensions = new Array<SizeDimensions>();
+
+  public add(newDimension: SizeDimensions): void {
+    if (this.dimensions.length > 0) {
+      const lastDimension = this.dimensions[this.dimensions.length - 1];
+      if (lastDimension.width === newDimension.width
+        && lastDimension.height === newDimension.height) {
+        return;
+      }
+    }
+    cy.window().then(() => {
+      cy.log(`Captured: ${JSON.stringify(newDimension)}`);
+    });
+    this.dimensions.push(newDimension);
+  }
+
+  public getUniqueWidths(): readonly number[] {
+    return [...new Set(this.dimensions.map((d) => d.width))];
+  }
+
+  public getUniqueHeights(): readonly number[] {
+    return [...new Set(this.dimensions.map((d) => d.height))];
+  }
+
+  public toString(): string {
+    return JSON.stringify(this.dimensions);
+  }
+}
+
+function isWithinTolerance(
+  numbers: readonly number[],
+  tolerance: number,
+) {
+  let changeWithinTolerance = true;
+  const [firstValue, ...otherValues] = numbers;
+  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,
+}
+
+const checkpoints: Record<ApplicationLoadStep, () => void> = {
+  [ApplicationLoadStep.IndexHtmlLoaded]: () => cy.get('#app').should('be.visible'),
+  [ApplicationLoadStep.AppVueLoaded]: () => cy.get('.app__wrapper').should('be.visible'),
+  [ApplicationLoadStep.HeaderBrandTitleLoaded]: () => waitForHeaderBrandTitle(),
+};
+
+class ContinuousRunner implements Stoppable {
+  private timer: ReturnType<typeof setTimeout> | undefined;
+
+  public start(callback: () => void, intervalInMs: number): this {
+    this.stop();
+    this.timer = setInterval(() => {
+      if (this.isStopped) {
+        return;
+      }
+      callback();
+    }, intervalInMs);
+    return this;
+  }
+
+  public stop() {
+    if (this.timer === undefined) {
+      return;
+    }
+    clearInterval(this.timer);
+    this.timer = undefined;
+  }
+
+  private get isStopped() {
+    return this.timer === undefined;
+  }
+}
+
+class SizeMonitor implements Stoppable {
+  private observer: ResizeObserver | undefined;
+
+  public start(element: Element, sizeChangedCallback: () => void): this {
+    this.stop();
+    this.observer = new ResizeObserver(() => {
+      if (this.isStopped) {
+        return;
+      }
+      sizeChangedCallback();
+    });
+    this.observer.observe(element);
+    return this;
+  }
+
+  public stop() {
+    this.observer?.disconnect();
+    this.observer = undefined;
+  }
+
+  private get isStopped() {
+    return this.observer === undefined;
+  }
+}

tests/e2e/initialization.cy.ts

@@ -1,9 +1,11 @@
+import { waitForHeaderBrandTitle } from './shared/ApplicationLoad';
+
 describe('application is initialized as expected', () => {
   it('loads title as expected', () => {
     // act
     cy.visit('/');
     // assert
-    cy.contains('h1', 'privacy.sexy');
+    waitForHeaderBrandTitle();
   });
   it('there are no console.error output', () => {
     // act

tests/e2e/shared/ApplicationLoad.ts

@@ -0,0 +1,3 @@
+export function waitForHeaderBrandTitle() {
+  cy.contains('h1', 'privacy.sexy');
+}

tests/unit/presentation/components/Scripts/View/Cards/CardList.spec.ts

@@ -0,0 +1,63 @@
+import { shallowMount } from '@vue/test-utils';
+import { describe, it, expect } from 'vitest';
+import { Ref, nextTick, ref } from 'vue';
+import CardList from '@/presentation/components/Scripts/View/Cards/CardList.vue';
+import { useCollectionState } from '@/presentation/components/Shared/Hooks/UseCollectionState';
+import { UseCollectionStateStub } from '@tests/unit/shared/Stubs/UseCollectionStateStub';
+import { InjectionKeys } from '@/presentation/injectionSymbols';
+import { createSizeObserverStub } from '@tests/unit/shared/Stubs/SizeObserverStub';
+
+const DOM_SELECTOR_CARDS = '.cards';
+
+describe('CardList.vue', () => {
+  describe('rendering cards based on width', () => {
+    it('renders cards when a valid width is provided', async () => {
+      // arrange
+      const expectedCardsExistence = true;
+      const width = ref(0);
+      // act
+      const wrapper = mountComponent({
+        widthRef: width,
+      });
+      width.value = 800;
+      await nextTick();
+      // assert
+      const actual = wrapper.find(DOM_SELECTOR_CARDS).exists();
+      expect(actual).to.equal(expectedCardsExistence, wrapper.html());
+    });
+    it('does not render cards when width is not set', async () => {
+      // arrange
+      const expectedCardsExistence = false;
+      const width = ref(0);
+      const wrapper = mountComponent({
+        widthRef: width,
+      });
+      // act
+      await nextTick();
+      // assert
+      const actual = wrapper.find(DOM_SELECTOR_CARDS).exists();
+      expect(actual).to.equal(expectedCardsExistence, wrapper.html());
+    });
+  });
+});
+
+function mountComponent(options?: {
+  readonly useCollectionState?: ReturnType<typeof useCollectionState>,
+  readonly widthRef?: Readonly<Ref<number>>,
+}) {
+  const {
+    name: sizeObserverName,
+    component: sizeObserverStub,
+  } = createSizeObserverStub(options?.widthRef);
+  return shallowMount(CardList, {
+    global: {
+      provide: {
+        [InjectionKeys.useCollectionState.key]:
+          () => options?.useCollectionState ?? new UseCollectionStateStub().get(),
+      },
+      stubs: {
+        [sizeObserverName]: sizeObserverStub,
+      },
+    },
+  });
+}

tests/unit/shared/Stubs/CategoryCollectionStateStub.ts

@@ -22,7 +22,7 @@ export class CategoryCollectionStateStub implements ICategoryCollectionState {
     return this.collection.os;
   }
 
-  public collection: ICategoryCollection = new CategoryCollectionStub();
+  public collection: ICategoryCollection = new CategoryCollectionStub().withSomeActions();
 
   public selection: IUserSelection = new UserSelectionStub([]);
 

tests/unit/shared/Stubs/CategoryCollectionStub.ts

@@ -21,6 +21,13 @@ export class CategoryCollectionStub implements ICategoryCollection {
 
   public readonly actions = new Array<ICategory>();
 
+  public withSomeActions(): this {
+    this.withAction(new CategoryStub(1));
+    this.withAction(new CategoryStub(2));
+    this.withAction(new CategoryStub(3));
+    return this;
+  }
+
   public withAction(category: ICategory): this {
     this.actions.push(category);
     return this;

tests/unit/shared/Stubs/SizeObserverStub.ts

@@ -0,0 +1,27 @@
+import { defineComponent, ref, watch } from 'vue';
+import type { Ref } from 'vue';
+
+const COMPONENT_SIZE_OBSERVER_NAME = 'SizeObserver';
+
+export function createSizeObserverStub(
+  widthRef: Readonly<Ref<number>> = ref(500),
+) {
+  const component = defineComponent({
+    name: COMPONENT_SIZE_OBSERVER_NAME,
+    template: `<div id="${COMPONENT_SIZE_OBSERVER_NAME}-stub"><slot /></div>`,
+    emits: {
+      /* eslint-disable @typescript-eslint/no-unused-vars */
+      widthChanged: (newWidth: number) => true,
+      /* eslint-enable @typescript-eslint/no-unused-vars */
+    },
+    setup: (_, { emit }) => {
+      watch(widthRef, (newValue) => {
+        emit('widthChanged', newValue);
+      });
+    },
+  });
+  return {
+    name: COMPONENT_SIZE_OBSERVER_NAME,
+    component,
+  };
+}