diff --git a/.changeset/itchy-peas-develop.md b/.changeset/itchy-peas-develop.md new file mode 100644 index 0000000000..07fad892ee --- /dev/null +++ b/.changeset/itchy-peas-develop.md @@ -0,0 +1,6 @@ +--- +"@justeattakeaway/pie-modal": patch +"pie-storybook": patch +--- + +[Fixed] - Ensure page doesn't scroll when the modal is opened diff --git a/apps/pie-storybook/stories/pie-modal.stories.ts b/apps/pie-storybook/stories/pie-modal.stories.ts index 3a53bdc53d..9b12cc457a 100644 --- a/apps/pie-storybook/stories/pie-modal.stories.ts +++ b/apps/pie-storybook/stories/pie-modal.stories.ts @@ -274,7 +274,8 @@ const FormStoryTemplate = (props: ModalProps) => { const ScrollablePageStoryTemplate = (props: ModalProps) => html` ${BaseStoryTemplate(props)} - ${createScrollablePageHTML()}`; + ${createScrollablePageHTML()} + Toggle Modal`; const FocusableElementsPageStoryTemplate = (props: ModalProps) => html` ${BaseStoryTemplate(props)} diff --git a/apps/pie-storybook/stories/testing/pie-modal.test.stories.ts b/apps/pie-storybook/stories/testing/pie-modal.test.stories.ts index 8fd03ada71..0a610500cd 100644 --- a/apps/pie-storybook/stories/testing/pie-modal.test.stories.ts +++ b/apps/pie-storybook/stories/testing/pie-modal.test.stories.ts @@ -285,7 +285,8 @@ const FormStoryTemplate = (props: ModalProps) => { const ScrollablePageStoryTemplate = (props: ModalProps) => html` ${BaseStoryTemplate(props)} - ${createScrollablePageHTML()}`; + ${createScrollablePageHTML()} + Toggle Modal`; const FocusToSpecifiedElementStoryTemplate = (props: ModalProps) => html` ${BaseStoryTemplate(props)} diff --git a/packages/components/pie-cookie-banner/test/visual/pie-cookie-banner.spec.ts b/packages/components/pie-cookie-banner/test/visual/pie-cookie-banner.spec.ts index 02ea6332bf..53c846f308 100644 --- a/packages/components/pie-cookie-banner/test/visual/pie-cookie-banner.spec.ts +++ b/packages/components/pie-cookie-banner/test/visual/pie-cookie-banner.spec.ts @@ -20,11 +20,6 @@ test.describe('PieCookieBanner - Visual tests`', () => { await pieCookieBannerComponent.clickManagePreferencesAction(); - // This fixes modal being pushed down in screenshots - await page.$eval('dialog', (el) => { - el.style.top = '20px'; - }); - await percySnapshot(page, 'PieCookieBanner Manage preferences - Visual Test'); }); diff --git a/packages/components/pie-modal/src/index.ts b/packages/components/pie-modal/src/index.ts index 71002cd391..924b196ff5 100644 --- a/packages/components/pie-modal/src/index.ts +++ b/packages/components/pie-modal/src/index.ts @@ -310,10 +310,6 @@ export class PieModal extends RtlMixin(LitElement) implements ModalProps { */ private _disableBodyScroll (): void { if (this._modalScrollContainer) { - // Hack to prevent Safari rendering the modal outside the viewport - // when the body scroll lock is active - if ('scrollTo' in window) window.scrollTo(0, 0); - disableBodyScroll(this._modalScrollContainer); } } diff --git a/packages/components/pie-modal/src/modal.scss b/packages/components/pie-modal/src/modal.scss index 787be7ee9a..440b39820a 100644 --- a/packages/components/pie-modal/src/modal.scss +++ b/packages/components/pie-modal/src/modal.scss @@ -36,6 +36,8 @@ flex-direction: column; } + position: fixed; + top: 0; border-radius: var(--modal-border-radius); border: none; box-shadow: var(--modal-elevation); diff --git a/packages/components/pie-modal/test/component/pie-modal.spec.ts b/packages/components/pie-modal/test/component/pie-modal.spec.ts index 19626d60ee..8c94dcfc08 100644 --- a/packages/components/pie-modal/test/component/pie-modal.spec.ts +++ b/packages/components/pie-modal/test/component/pie-modal.spec.ts @@ -436,6 +436,35 @@ test.describe('scrolling logic', () => { await expect.soft(page.getByText('Top of page copy')).not.toBeInViewport(); await expect(page.getByText('Bottom of page copy')).toBeInViewport(); }); + + test('Should preserve scroll position after opening and closing the modal', async ({ page }) => { + // Arrange + const modalScrollLockingPage = new ModalScrollLockingPage(page); + + const props: ModalProps = { + ...sharedProps, + isOpen: false, + }; + + await modalScrollLockingPage.load(props); + + // Act + // Scroll to the bottom of the page + await page.mouse.wheel(0, 5000); + + // The mouse.wheel function causes scrolling, but doesn't wait for the scroll to finish before returning. + await page.waitForTimeout(3000); + + // opens the modal + await modalScrollLockingPage.openModalFromPageBottom(); + + // closes the modal + await page.keyboard.press('Escape'); + + // Assert + await expect.soft(page.getByText('Top of page copy')).not.toBeInViewport(); + await expect(page.getByText('Bottom of page copy')).toBeInViewport(); + }); }); test.describe('`hasBackButton` prop', () => { diff --git a/packages/components/pie-modal/test/helpers/page-object/pie-modal-scroll-locking.page.ts b/packages/components/pie-modal/test/helpers/page-object/pie-modal-scroll-locking.page.ts index 3a5ec59342..c831143a47 100644 --- a/packages/components/pie-modal/test/helpers/page-object/pie-modal-scroll-locking.page.ts +++ b/packages/components/pie-modal/test/helpers/page-object/pie-modal-scroll-locking.page.ts @@ -5,10 +5,19 @@ import { ModalComponent } from './pie-modal.component.ts'; export class ModalScrollLockingPage extends BasePage { readonly modalComponent: ModalComponent; readonly openModalButtonLocator: Locator; + readonly openModalButtonAtPageBottomLocator: Locator; constructor (page: Page) { super(page, 'modal--scroll-locking'); this.modalComponent = new ModalComponent(page); this.openModalButtonLocator = page.locator('#open-modal'); + this.openModalButtonAtPageBottomLocator = page.locator('#open-modal-bottom'); + } + + /** + * Opens the modal by clicking the open modal button on the bottom of the page. + */ + async openModalFromPageBottom () { + await this.openModalButtonAtPageBottomLocator.click(); } }