Fix layout jumps/shifts and overflow on modals
This commit improves the user interface in modal display. - Prevent layout shifts caused by background scrollbars when modals are active. - Fix unintended overflow of modals on small screens, preventing parts of the modal from being cut off on the right side. - Refactor DOM manipulation, enhancing modularity, reusability, extensibility, and separation of concerns. - Centralize viewport test scenarios for different sizes in a single definition for E2E tests.
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
// eslint-disable-next-line max-classes-per-file
|
||||
import { getHeaderBrandTitle } from './support/interactions/header';
|
||||
import { ViewportTestScenarios } from './support/scenarios/viewport-test-scenarios';
|
||||
|
||||
interface Stoppable {
|
||||
stop(): void;
|
||||
@@ -7,21 +8,12 @@ interface Stoppable {
|
||||
|
||||
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 }) => {
|
||||
ViewportTestScenarios.forEach(({ name, width, height }) => {
|
||||
it(`ensures layout stability on ${name}`, () => {
|
||||
// arrange
|
||||
const dimensions = new DimensionsStorage();
|
||||
|
||||
86
tests/e2e/modal-layout-shifts.cy.ts
Normal file
86
tests/e2e/modal-layout-shifts.cy.ts
Normal file
@@ -0,0 +1,86 @@
|
||||
import { ViewportTestScenarios } from './support/scenarios/viewport-test-scenarios';
|
||||
|
||||
describe('Modal interaction and layout stability', () => {
|
||||
ViewportTestScenarios.forEach(({ // some shifts are observed only on extra small or large screens
|
||||
name, width, height,
|
||||
}) => {
|
||||
it(name, () => {
|
||||
cy.viewport(width, height);
|
||||
cy.visit('/');
|
||||
|
||||
let metricsBeforeModal: ViewportMetrics | undefined;
|
||||
|
||||
captureViewportMetrics((metrics) => {
|
||||
metricsBeforeModal = metrics;
|
||||
});
|
||||
|
||||
cy
|
||||
.contains('a', 'Privacy')
|
||||
.click();
|
||||
|
||||
cy
|
||||
.get('.modal-content')
|
||||
.should('be.visible');
|
||||
|
||||
captureViewportMetrics((metrics) => {
|
||||
const metricsAfterModal = metrics;
|
||||
expect(metricsBeforeModal).to.deep.equal(metricsAfterModal, [
|
||||
`Expected (initial metrics before modal): ${JSON.stringify(metricsBeforeModal)}`,
|
||||
`Actual (metrics after modal is opened): ${JSON.stringify(metricsAfterModal)}`,
|
||||
].join('\n'));
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
interface ViewportMetrics {
|
||||
readonly x: number;
|
||||
readonly y: number;
|
||||
/*
|
||||
Excluding height and width from the metrics to ensure test accuracy.
|
||||
Height and width measurements can lead to false negatives due to layout shifts caused by
|
||||
delayed loading of fonts and icons.
|
||||
*/
|
||||
}
|
||||
|
||||
function captureViewportMetrics(callback: (metrics: ViewportMetrics) => void): void {
|
||||
cy.window().then((win) => {
|
||||
cy.get('body')
|
||||
.then((body) => {
|
||||
const position = getElementViewportMetrics(body[0], win);
|
||||
cy.log(`Captured metrics: ${JSON.stringify(position)}`);
|
||||
callback(position);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function getElementViewportMetrics(element: HTMLElement, win: Window): ViewportMetrics {
|
||||
const elementXRelativeToViewport = getElementXRelativeToViewport(element, win);
|
||||
const elementYRelativeToViewport = getElementYRelativeToViewport(element, win);
|
||||
return {
|
||||
x: elementXRelativeToViewport,
|
||||
y: elementYRelativeToViewport,
|
||||
};
|
||||
}
|
||||
|
||||
function getElementYRelativeToViewport(element: HTMLElement, win: Window): number {
|
||||
const relativeTop = element.getBoundingClientRect().top;
|
||||
const { position, top } = win.getComputedStyle(element);
|
||||
const topValue = position === 'static' ? 0 : parseInt(top, 10);
|
||||
if (Number.isNaN(topValue)) {
|
||||
throw new Error(`Could not calculate Y position value from 'top': ${top}`);
|
||||
}
|
||||
const viewportRelativeY = relativeTop - topValue + win.scrollY;
|
||||
return viewportRelativeY;
|
||||
}
|
||||
|
||||
function getElementXRelativeToViewport(element: HTMLElement, win: Window): number {
|
||||
const relativeLeft = element.getBoundingClientRect().left;
|
||||
const { position, left } = win.getComputedStyle(element);
|
||||
const leftValue = position === 'static' ? 0 : parseInt(left, 10);
|
||||
if (Number.isNaN(leftValue)) {
|
||||
throw new Error(`Could not calculate X position value from 'left': ${left}`);
|
||||
}
|
||||
const viewportRelativeX = relativeLeft - leftValue + win.scrollX;
|
||||
return viewportRelativeX;
|
||||
}
|
||||
11
tests/e2e/support/scenarios/viewport-test-scenarios.ts
Normal file
11
tests/e2e/support/scenarios/viewport-test-scenarios.ts
Normal file
@@ -0,0 +1,11 @@
|
||||
export const ViewportTestScenarios: readonly ViewportScenario[] = [
|
||||
{ name: 'iPhone SE', width: 375, height: 667 },
|
||||
{ name: '13-inch Laptop', width: 1280, height: 800 },
|
||||
{ name: '4K Ultra HD Desktop', width: 3840, height: 2160 },
|
||||
] as const;
|
||||
|
||||
interface ViewportScenario {
|
||||
readonly name: string;
|
||||
readonly width: number;
|
||||
readonly height: number;
|
||||
}
|
||||
@@ -0,0 +1,288 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { shallowMount } from '@vue/test-utils';
|
||||
import { ref, nextTick, defineComponent } from 'vue';
|
||||
import { useLockBodyBackgroundScroll } from '@/presentation/components/Shared/Modal/Hooks/ScrollLock/UseLockBodyBackgroundScroll';
|
||||
import { ScrollDomStateAccessor } from '@/presentation/components/Shared/Modal/Hooks/ScrollLock/ScrollDomStateAccessor';
|
||||
import { PropertyKeys } from '@/TypeHelpers';
|
||||
|
||||
describe('useLockBodyBackgroundScroll', () => {
|
||||
describe('initialization', () => {
|
||||
describe('activates scroll lock when initially active', () => {
|
||||
itEachScrollBlockEffect(async (dom) => {
|
||||
// arrange
|
||||
const isInitiallyActive = true;
|
||||
|
||||
// act
|
||||
createComponent(isInitiallyActive, dom);
|
||||
await nextTick();
|
||||
});
|
||||
});
|
||||
it('maintains initial styles when initially inactive', async () => {
|
||||
// arrange
|
||||
const isInitiallyActive = false;
|
||||
|
||||
// act
|
||||
const { initialDomState, actualDomState } = createComponent(isInitiallyActive);
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(actualDomState).to.deep.equal(initialDomState);
|
||||
});
|
||||
});
|
||||
describe('toggling scroll lock', () => {
|
||||
describe('enforces scroll lock when activated', async () => {
|
||||
itEachScrollBlockEffect(async (dom) => {
|
||||
// arrange
|
||||
const isInitiallyActive = false;
|
||||
const { isActive } = createComponent(isInitiallyActive, dom);
|
||||
|
||||
// act
|
||||
isActive.value = true;
|
||||
await nextTick();
|
||||
});
|
||||
});
|
||||
it('reverts to initial styles when deactivated', async () => {
|
||||
// arrange
|
||||
const isInitiallyActive = true;
|
||||
const { isActive, initialDomState, actualDomState } = createComponent(isInitiallyActive);
|
||||
|
||||
// act
|
||||
isActive.value = false;
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(actualDomState).to.deep.equal(initialDomState);
|
||||
});
|
||||
});
|
||||
it('restores original styles on unmount', async () => {
|
||||
// arrange
|
||||
const isInitiallyActive = true;
|
||||
const { component, initialDomState, actualDomState } = createComponent(isInitiallyActive);
|
||||
|
||||
// act
|
||||
component.unmount();
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(actualDomState).to.deep.equal(initialDomState);
|
||||
});
|
||||
});
|
||||
|
||||
function createComponent(
|
||||
initialIsActiveValue: boolean,
|
||||
dom?: ScrollDomStateAccessor,
|
||||
) {
|
||||
const actualDomState = dom ?? createMockDomStateAccessor();
|
||||
const initialDomState = { ...actualDomState };
|
||||
const isActive = ref(initialIsActiveValue);
|
||||
const component = shallowMount(defineComponent({
|
||||
setup() {
|
||||
useLockBodyBackgroundScroll(isActive, actualDomState);
|
||||
},
|
||||
template: '<div></div>',
|
||||
}));
|
||||
return {
|
||||
component, isActive, initialDomState, actualDomState,
|
||||
};
|
||||
}
|
||||
|
||||
function itEachScrollBlockEffect(act: (dom: ScrollDomStateAccessor) => Promise<void>) {
|
||||
testScenarios.forEach((m) => {
|
||||
const description = m.description ? ` (${m.description})` : '';
|
||||
it(`handles '${m.propertyName}'${description}`, async () => {
|
||||
// arrange
|
||||
const dom = createMockDomStateAccessor();
|
||||
const initialDom = { ...dom };
|
||||
if (m.prepare) {
|
||||
m.prepare(dom);
|
||||
}
|
||||
// act
|
||||
await act(dom);
|
||||
// assert
|
||||
const expectedValue = m.getExpectedValueOnBlock(initialDom, dom);
|
||||
const actualValue = dom[m.propertyName];
|
||||
expect(actualValue).to.equal(expectedValue);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
type DomPropertyType = string | number;
|
||||
|
||||
interface DomStateChange {
|
||||
readonly propertyName: PropertyKeys<ScrollDomStateAccessor>;
|
||||
readonly description?: string;
|
||||
readonly prepare?: (dom: Writable<ScrollDomStateAccessor>) => void;
|
||||
getExpectedValueOnBlock(
|
||||
initialDom: Readonly<ScrollDomStateAccessor>,
|
||||
actualDom: Readonly<ScrollDomStateAccessor>,
|
||||
): DomPropertyType;
|
||||
}
|
||||
|
||||
const testScenarios: ReadonlyArray<DomStateChange> = [
|
||||
{
|
||||
propertyName: 'bodyStyleOverflowX',
|
||||
description: 'visible horizontal scrollbar',
|
||||
prepare: (dom) => {
|
||||
dom.htmlClientWidth = 5;
|
||||
dom.htmlScrollWidth = 10;
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'scroll',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleOverflowX',
|
||||
description: 'invisible horizontal scrollbar',
|
||||
prepare: (dom) => {
|
||||
dom.htmlClientWidth = 10;
|
||||
dom.htmlScrollWidth = 5;
|
||||
},
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.bodyStyleOverflowX,
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleOverflowY',
|
||||
description: 'visible vertical scrollbar',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollHeight = 10;
|
||||
dom.htmlClientHeight = 5;
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'scroll',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleOverflowY',
|
||||
description: 'invisible vertical scrollbar',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollHeight = 5;
|
||||
dom.htmlClientHeight = 10;
|
||||
},
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.bodyStyleOverflowY,
|
||||
},
|
||||
{
|
||||
propertyName: 'htmlScrollLeft',
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.htmlScrollLeft,
|
||||
},
|
||||
{
|
||||
propertyName: 'htmlScrollTop',
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.htmlScrollTop,
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleLeft',
|
||||
description: 'adjusts for scrolled position',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollLeft = 22;
|
||||
},
|
||||
getExpectedValueOnBlock: () => '-22px',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleLeft',
|
||||
description: 'unaffected by no horizontal scroll',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollLeft = 0;
|
||||
},
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.bodyStyleLeft,
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleTop',
|
||||
description: 'adjusts for scrolled position',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollTop = 12;
|
||||
},
|
||||
getExpectedValueOnBlock: () => '-12px',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleTop',
|
||||
description: 'unaffected by no vertical scroll',
|
||||
prepare: (dom) => {
|
||||
dom.htmlScrollTop = 0;
|
||||
},
|
||||
getExpectedValueOnBlock: (initialDom) => initialDom.bodyStyleTop,
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStylePosition',
|
||||
getExpectedValueOnBlock: () => 'fixed',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleWidth',
|
||||
description: 'no margin',
|
||||
getExpectedValueOnBlock: () => '100%',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleWidth',
|
||||
description: 'margin on left',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginLeft = '3px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (3px))',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleWidth',
|
||||
description: 'margin on right',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginRight = '4px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (4px))',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleWidth',
|
||||
description: 'margin on left and right',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginLeft = '5px';
|
||||
dom.bodyComputedMarginRight = '5px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (5px + 5px))',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleHeight',
|
||||
description: 'no margin',
|
||||
getExpectedValueOnBlock: () => '100%',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleHeight',
|
||||
description: 'margin on top',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginTop = '3px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (3px))',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleHeight',
|
||||
description: 'margin on bottom',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginBottom = '4px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (4px))',
|
||||
},
|
||||
{
|
||||
propertyName: 'bodyStyleHeight',
|
||||
description: 'margin on top and bottom',
|
||||
prepare: (dom) => {
|
||||
dom.bodyComputedMarginTop = '5px';
|
||||
dom.bodyComputedMarginBottom = '5px';
|
||||
},
|
||||
getExpectedValueOnBlock: () => 'calc(100% - (5px + 5px))',
|
||||
},
|
||||
];
|
||||
|
||||
function createMockDomStateAccessor(): ScrollDomStateAccessor {
|
||||
return {
|
||||
bodyStyleOverflowX: '',
|
||||
bodyStyleOverflowY: '',
|
||||
htmlScrollLeft: 0,
|
||||
htmlScrollTop: 0,
|
||||
bodyStyleLeft: '',
|
||||
bodyStyleTop: '',
|
||||
bodyStylePosition: '',
|
||||
bodyStyleWidth: '',
|
||||
bodyStyleHeight: '',
|
||||
bodyComputedMarginLeft: '',
|
||||
bodyComputedMarginRight: '',
|
||||
bodyComputedMarginTop: '',
|
||||
bodyComputedMarginBottom: '',
|
||||
htmlScrollWidth: 0,
|
||||
htmlScrollHeight: 0,
|
||||
htmlClientWidth: 0,
|
||||
htmlClientHeight: 0,
|
||||
};
|
||||
}
|
||||
|
||||
type Writable<T> = {
|
||||
-readonly [P in keyof T]: T[P];
|
||||
};
|
||||
@@ -1,114 +0,0 @@
|
||||
import {
|
||||
describe, it, expect, afterEach,
|
||||
} from 'vitest';
|
||||
import { shallowMount } from '@vue/test-utils';
|
||||
import { ref, nextTick, defineComponent } from 'vue';
|
||||
import { useLockBodyBackgroundScroll } from '@/presentation/components/Shared/Modal/Hooks/UseLockBodyBackgroundScroll';
|
||||
|
||||
describe('useLockBodyBackgroundScroll', () => {
|
||||
afterEach(() => {
|
||||
document.body.style.overflow = '';
|
||||
document.body.style.width = '';
|
||||
});
|
||||
|
||||
describe('initialization', () => {
|
||||
it('blocks scroll if initially active', async () => {
|
||||
// arrange
|
||||
createComponent(true);
|
||||
|
||||
// act
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).to.equal('hidden');
|
||||
expect(document.body.style.width).to.equal('100vw');
|
||||
});
|
||||
|
||||
it('preserves initial styles if inactive', async () => {
|
||||
// arrange
|
||||
const originalOverflow = 'scroll';
|
||||
const originalWidth = '90vw';
|
||||
document.body.style.overflow = originalOverflow;
|
||||
document.body.style.width = originalWidth;
|
||||
|
||||
// act
|
||||
createComponent(false);
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).to.equal(originalOverflow);
|
||||
expect(document.body.style.width).to.equal(originalWidth);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toggling', () => {
|
||||
it('blocks scroll when activated', async () => {
|
||||
// arrange
|
||||
const { isActive } = createComponent(false);
|
||||
|
||||
// act
|
||||
isActive.value = true;
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).to.equal('hidden');
|
||||
expect(document.body.style.width).to.equal('100vw');
|
||||
});
|
||||
|
||||
it('unblocks scroll when deactivated', async () => {
|
||||
// arrange
|
||||
const { isActive } = createComponent(true);
|
||||
|
||||
// act
|
||||
isActive.value = false;
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).not.to.equal('hidden');
|
||||
expect(document.body.style.width).not.to.equal('100vw');
|
||||
});
|
||||
});
|
||||
|
||||
describe('unmounting', () => {
|
||||
it('restores original styles on unmount', async () => {
|
||||
// arrange
|
||||
const originalOverflow = 'scroll';
|
||||
const originalWidth = '90vw';
|
||||
document.body.style.overflow = originalOverflow;
|
||||
document.body.style.width = originalWidth;
|
||||
|
||||
// act
|
||||
const { component } = createComponent(true);
|
||||
component.unmount();
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).to.equal(originalOverflow);
|
||||
expect(document.body.style.width).to.equal(originalWidth);
|
||||
});
|
||||
|
||||
it('resets styles on unmount', async () => {
|
||||
// arrange
|
||||
const { component } = createComponent(true);
|
||||
|
||||
// act
|
||||
component.unmount();
|
||||
await nextTick();
|
||||
|
||||
// assert
|
||||
expect(document.body.style.overflow).to.equal('');
|
||||
expect(document.body.style.width).to.equal('');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function createComponent(initialIsActiveValue: boolean) {
|
||||
const isActive = ref(initialIsActiveValue);
|
||||
const component = shallowMount(defineComponent({
|
||||
setup() {
|
||||
useLockBodyBackgroundScroll(isActive);
|
||||
},
|
||||
template: '<div></div>',
|
||||
}));
|
||||
return { component, isActive };
|
||||
}
|
||||
Reference in New Issue
Block a user