From 7bb08a8651f9bcf3164ce2a8200eb89b721e14e9 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Fri, 26 Jul 2024 13:03:45 +0200 Subject: [PATCH 1/2] fix(various): re-enable scrolling when disconnecting the component before animationend --- src/elements/menu/menu/menu.ts | 1 + src/elements/navigation/navigation/navigation.ts | 1 + src/elements/overlay/overlay-base-element.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/src/elements/menu/menu/menu.ts b/src/elements/menu/menu/menu.ts index 28261989b8..2a4db75642 100644 --- a/src/elements/menu/menu/menu.ts +++ b/src/elements/menu/menu/menu.ts @@ -210,6 +210,7 @@ export class SbbMenuElement extends SbbNamedSlotListMixin< this._windowEventsController?.abort(); this._focusHandler.disconnect(); removeInertMechanism(); + this._scrollHandler.enableScroll(); } private _checkListCase(event: Event): void { diff --git a/src/elements/navigation/navigation/navigation.ts b/src/elements/navigation/navigation/navigation.ts index c0301b980f..b9cd095201 100644 --- a/src/elements/navigation/navigation/navigation.ts +++ b/src/elements/navigation/navigation/navigation.ts @@ -340,6 +340,7 @@ export class SbbNavigationElement extends SbbUpdateSchedulerMixin(SbbOpenCloseBa this._navigationObserver.disconnect(); this._navigationResizeObserver.disconnect(); removeInertMechanism(); + this._scrollHandler.enableScroll(); } protected override render(): TemplateResult { diff --git a/src/elements/overlay/overlay-base-element.ts b/src/elements/overlay/overlay-base-element.ts index f135ada3c2..100b4c5a24 100644 --- a/src/elements/overlay/overlay-base-element.ts +++ b/src/elements/overlay/overlay-base-element.ts @@ -85,6 +85,7 @@ export abstract class SbbOverlayBaseElement extends SbbNegativeMixin(SbbOpenClos this.focusHandler.disconnect(); this.removeInstanceFromGlobalCollection(); removeInertMechanism(); + this.scrollHandler.enableScroll(); } protected attachOpenOverlayEvents(): void { From e75e0b3358e1777a8eeee9d35afcba16197e3757 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Fri, 26 Jul 2024 14:41:17 +0200 Subject: [PATCH 2/2] test: add test for new logic --- src/elements/core/testing.ts | 1 + src/elements/core/testing/wait-for-event.ts | 19 +++++++++++++++++++ .../navigation/navigation/navigation.spec.ts | 13 ++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/elements/core/testing/wait-for-event.ts diff --git a/src/elements/core/testing.ts b/src/elements/core/testing.ts index 701f53e83d..11960275c9 100644 --- a/src/elements/core/testing.ts +++ b/src/elements/core/testing.ts @@ -2,5 +2,6 @@ export * from './testing/event-spy.js'; export * from './testing/mocha-extensions.js'; export * from './testing/scroll.js'; export * from './testing/wait-for-condition.js'; +export * from './testing/wait-for-event.js'; export * from './testing/wait-for-image-ready.js'; export * from './testing/wait-for-render.js'; diff --git a/src/elements/core/testing/wait-for-event.ts b/src/elements/core/testing/wait-for-event.ts new file mode 100644 index 0000000000..c40dc872f4 --- /dev/null +++ b/src/elements/core/testing/wait-for-event.ts @@ -0,0 +1,19 @@ +export function waitForEvent( + element: HTMLElement, + eventName: string, + timeout = 1000, +): Promise { + return new Promise((resolve, reject) => { + const signal = AbortSignal.timeout(timeout); + const timeoutReached = (): void => reject(`Timeout of ${timeout} reached`); + signal.addEventListener('abort', timeoutReached); + element.addEventListener( + eventName, + () => { + signal.removeEventListener('abort', timeoutReached); + resolve(); + }, + { passive: true, signal }, + ); + }); +} diff --git a/src/elements/navigation/navigation/navigation.spec.ts b/src/elements/navigation/navigation/navigation.spec.ts index 0e722b49c7..4c64d9268d 100644 --- a/src/elements/navigation/navigation/navigation.spec.ts +++ b/src/elements/navigation/navigation/navigation.spec.ts @@ -3,9 +3,10 @@ import { sendKeys } from '@web/test-runner-commands'; import { html } from 'lit/static-html.js'; import type { SbbButtonElement } from '../../button.js'; +import { pageScrollDisabled } from '../../core/dom.js'; import { tabKey } from '../../core/testing/private/keys.js'; import { fixture } from '../../core/testing/private.js'; -import { waitForCondition, waitForLitRender, EventSpy } from '../../core/testing.js'; +import { waitForCondition, waitForLitRender, EventSpy, waitForEvent } from '../../core/testing.js'; import type { SbbNavigationButtonElement } from '../navigation-button.js'; import type { SbbNavigationSectionElement } from '../navigation-section.js'; @@ -522,4 +523,14 @@ describe(`sbb-navigation`, () => { expect(element).to.have.attribute('data-state', 'opened'); }); + + it('should re-enable scrolling when removed from the DOM', async () => { + element.open(); + await waitForEvent(element, SbbNavigationElement.events.didOpen); + + expect(pageScrollDisabled()).to.be.true; + + element.remove(); + expect(pageScrollDisabled()).to.be.false; + }); });