fix: improve handling of animation events for zero duration (#3284)

This PR enables updating Playwright and therefore provides a solution for the chromium bug https://issues.chromium.org/issues/373506511 Only for autocomplete and select, some code improvements were needed to support zero animation duration.
sbb-design-systems · Dec 11, 2024 · 6da37fc · 6da37fc
1 parent 4cb2cdd
commit 6da37fc
Show file tree

Hide file tree

Showing 87 changed files with 1,132 additions and 418 deletions.
diff --git a/src/elements/alert/alert-group/__snapshots__/alert-group.snapshot.spec.snap.js b/src/elements/alert/alert-group/__snapshots__/alert-group.snapshot.spec.snap.js
@@ -9,7 +9,7 @@ snapshots["sbb-alert-group renders DOM"] =
 >
   <sbb-alert
     animation="all"
-    data-state="opening"
+    data-state="opened"
     size="m"
     title-content="Interruption between Genève and Lausanne"
   >
@@ -42,7 +42,7 @@ snapshots["sbb-alert-group renders with slotted DOM"] =
   </span>
   <sbb-alert
     animation="all"
-    data-state="opening"
+    data-state="opened"
     size="m"
     title-content="Interruption between Genève and Lausanne"
   >

diff --git a/src/elements/alert/alert-group/alert-group.spec.ts b/src/elements/alert/alert-group/alert-group.spec.ts
@@ -17,6 +17,10 @@ describe(`sbb-alert-group`, () => {
     const accessibilityTitle = 'Disruptions';
     const accessibilityTitleLevel = '3';
 
+    const alertOpenedEventSpy = new EventSpy(SbbAlertElement.events.didOpen, null, {
+      capture: true,
+    });
+
     // Given sbb-alert-group with two alerts
     element = await fixture(html`
       <sbb-alert-group
@@ -28,14 +32,15 @@ describe(`sbb-alert-group`, () => {
         <sbb-alert title-content="Interruption" id="alert2">Second</sbb-alert>
       </sbb-alert-group>
     `);
+
     const emptySpy = new EventSpy(SbbAlertGroupElement.events.empty);
     const alert1 = element.querySelector<SbbAlertElement>('sbb-alert#alert1')!;
     const alert2 = element.querySelector<SbbAlertElement>('sbb-alert#alert2')!;
     const alert1ClosedEventSpy = new EventSpy(SbbAlertElement.events.didClose, alert1);
     const alert2ClosedEventSpy = new EventSpy(SbbAlertElement.events.didClose, alert2);
 
-    await new EventSpy(SbbAlertElement.events.didOpen, alert1).calledOnce();
-    await new EventSpy(SbbAlertElement.events.didOpen, alert2).calledOnce();
+    // Wait until both alerts are opened
+    await alertOpenedEventSpy.calledTimes(2);
 
     // Then two alerts should be rendered and accessibility title should be displayed
     expect(element.querySelectorAll('sbb-alert').length).to.be.equal(2);

diff --git a/src/elements/alert/alert/__snapshots__/alert.snapshot.spec.snap.js b/src/elements/alert/alert/__snapshots__/alert.snapshot.spec.snap.js
@@ -4,7 +4,7 @@ export const snapshots = {};
 snapshots["sbb-alert should render default properties DOM"] = 
 `<sbb-alert
   animation="all"
-  data-state="opening"
+  data-state="opened"
   size="m"
   title-content="Interruption"
 >
@@ -77,7 +77,7 @@ snapshots["sbb-alert should render default properties Shadow DOM"] =
 snapshots["sbb-alert should render customized properties DOM"] = 
 `<sbb-alert
   animation="all"
-  data-state="opening"
+  data-state="opened"
   icon-name="disruption"
   size="l"
   title-content="Interruption"

diff --git a/src/elements/alert/alert/alert.scss b/src/elements/alert/alert/alert.scss
@@ -22,7 +22,7 @@ $open-anim-opacity-to: 1;
   --sbb-alert-close-icon-margin: var(--sbb-spacing-responsive-xxs);
   --sbb-alert-gap: var(--sbb-spacing-fixed-2x) var(--sbb-spacing-responsive-xs);
   --sbb-alert-animation-duration: var(
-    --sbb-disable-animation-zero-duration,
+    --sbb-disable-animation-duration,
     var(--sbb-animation-duration-6x)
   );
   --sbb-alert-timing-function: ease-in;
@@ -85,7 +85,7 @@ $open-anim-opacity-to: 1;
       fill-mode: forwards;
       duration: var(--sbb-alert-animation-duration);
       timing-function: var(--sbb-alert-timing-function);
-      delay: 0s, var(--sbb-disable-animation-zero-duration, var(--sbb-animation-duration-2x));
+      delay: 0s, var(--sbb-disable-animation-duration, var(--sbb-animation-duration-2x));
     }
   }
 }

diff --git a/src/elements/alert/alert/alert.spec.ts b/src/elements/alert/alert/alert.spec.ts
@@ -36,6 +36,24 @@ describe(`sbb-alert`, () => {
     expect(didCloseSpy.count).to.be.equal(1);
   });
 
+  it('should fire animation events with non-zero animation duration', async () => {
+    const didOpenSpy = new EventSpy(SbbAlertElement.events.didOpen, null, { capture: true });
+    const didCloseSpy = new EventSpy(SbbAlertElement.events.didClose, null, { capture: true });
+
+    const alert: SbbAlertElement = await fixture(
+      html`<sbb-alert title-content="disruption" style="--sbb-alert-animation-duration: 1ms">
+        Interruption
+      </sbb-alert>`,
+    );
+
+    await didOpenSpy.calledOnce();
+
+    alert.close();
+
+    await didCloseSpy.calledOnce();
+    expect(didCloseSpy.count).to.be.equal(1);
+  });
+
   it('should respect canceled willClose event', async () => {
     const didOpenSpy = new EventSpy(SbbAlertElement.events.didOpen, null, { capture: true });
     const willCloseSpy = new EventSpy(SbbAlertElement.events.willClose, null, { capture: true });

diff --git a/src/elements/alert/alert/alert.ts b/src/elements/alert/alert/alert.ts
@@ -4,7 +4,7 @@ import { customElement, property } from 'lit/decorators.js';
 import { SbbOpenCloseBaseElement } from '../../core/base-elements.js';
 import { SbbLanguageController } from '../../core/controllers.js';
 import { forceType } from '../../core/decorators.js';
-import { isLean } from '../../core/dom.js';
+import { isLean, isZeroAnimationDuration } from '../../core/dom.js';
 import { i18nCloseAlert } from '../../core/i18n.js';
 import { SbbIconNameMixin } from '../../icon.js';
 import type { SbbTitleLevel } from '../../title.js';
@@ -75,14 +75,26 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {
 
   /** Open the alert. */
   public open(): void {
-    this.willOpen.emit();
     this.state = 'opening';
+    this.willOpen.emit();
+
+    // If the animation duration is zero, the animationend event is not always fired reliably.
+    // In this case we directly set the `opened` state.
+    if (this._isZeroAnimationDuration()) {
+      this._handleOpening();
+    }
   }
 
   /** Close the alert. */
   public close(): void {
     if (this.state === 'opened' && this.willClose.emit()) {
       this.state = 'closing';
+
+      // If the animation duration is zero, the animationend event is not always fired reliably.
+      // In this case we directly set the `closed` state.
+      if (this._isZeroAnimationDuration()) {
+        this._handleClosing();
+      }
     }
   }
 
@@ -92,6 +104,10 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {
     this.open();
   }
 
+  private _isZeroAnimationDuration(): boolean {
+    return isZeroAnimationDuration(this, '--sbb-alert-animation-duration');
+  }
+
   private _onAnimationEnd(event: AnimationEvent): void {
     if (this.state === 'opening' && event.animationName === 'open-opacity') {
       this._handleOpening();

diff --git a/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.spec.ts b/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.spec.ts
@@ -1,4 +1,4 @@
-import { assert, expect } from '@open-wc/testing';
+import { assert, aTimeout, expect } from '@open-wc/testing';
 import { sendKeys, sendMouse } from '@web/test-runner-commands';
 import { html } from 'lit/static-html.js';
 
@@ -25,9 +25,9 @@ describe(`sbb-autocomplete-grid`, () => {
         <input />
         <sbb-autocomplete-grid id="myAutocomplete">
           <sbb-autocomplete-grid-row>
-            <sbb-autocomplete-grid-option value="1" id="option-1"
-              >Option 1</sbb-autocomplete-grid-option
-            >
+            <sbb-autocomplete-grid-option value="1" id="option-1">
+              Option 1
+            </sbb-autocomplete-grid-option>
             <sbb-autocomplete-grid-cell>
               <sbb-autocomplete-grid-button
                 id="button-1"
@@ -36,9 +36,9 @@ describe(`sbb-autocomplete-grid`, () => {
             </sbb-autocomplete-grid-cell>
           </sbb-autocomplete-grid-row>
           <sbb-autocomplete-grid-row>
-            <sbb-autocomplete-grid-option value="2" id="option-2"
-              >Option 2</sbb-autocomplete-grid-option
-            >
+            <sbb-autocomplete-grid-option value="2" id="option-2">
+              Option 2
+            </sbb-autocomplete-grid-option>
             <sbb-autocomplete-grid-cell>
               <sbb-autocomplete-grid-button
                 id="button-2"
@@ -70,8 +70,8 @@ describe(`sbb-autocomplete-grid`, () => {
       expect(input).to.have.attribute('role', 'combobox');
       expect(input).to.have.attribute('aria-autocomplete', 'list');
       expect(input).to.have.attribute('aria-haspopup', 'grid');
-      expect(input).to.have.attribute('aria-controls', 'myAutocomplete');
-      expect(input).to.have.attribute('aria-owns', 'myAutocomplete');
+      expect(input).to.have.attribute('aria-controls', element.id);
+      expect(input).to.have.attribute('aria-owns', element.id);
       expect(input).to.have.attribute('aria-expanded', 'false');
     });
   });
@@ -83,12 +83,14 @@ describe(`sbb-autocomplete-grid`, () => {
 
       expect(element).not.to.have.attribute('autocomplete-origin-borderless');
 
+      const id = element.shadowRoot!.querySelector('.sbb-autocomplete__options')!.id;
+
       expect(input).to.have.attribute('autocomplete', 'off');
       expect(input).to.have.attribute('role', 'combobox');
       expect(input).to.have.attribute('aria-autocomplete', 'list');
       expect(input).to.have.attribute('aria-haspopup', 'grid');
-      expect(input).to.have.attribute('aria-controls', 'sbb-autocomplete-grid-11');
-      expect(input).to.have.attribute('aria-owns', 'sbb-autocomplete-grid-11');
+      expect(input).to.have.attribute('aria-controls', id);
+      expect(input).to.have.attribute('aria-owns', id);
       expect(input).to.have.attribute('aria-expanded', 'false');
     });
   });
@@ -99,7 +101,8 @@ describe(`sbb-autocomplete-grid`, () => {
     const willCloseEventSpy = new EventSpy(SbbAutocompleteGridElement.events.willClose, element);
     const didCloseEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didClose, element);
 
-    input.click();
+    input.focus();
+
     await willOpenEventSpy.calledOnce();
     expect(willOpenEventSpy.count).to.be.equal(1);
 
@@ -145,15 +148,34 @@ describe(`sbb-autocomplete-grid`, () => {
     expect(input).to.have.attribute('aria-expanded', 'false');
   });
 
+  it('opens and closes with non-zero animation duration', async () => {
+    element.style.setProperty('--sbb-options-panel-animation-duration', '1ms');
+    const didOpenEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didOpen, element);
+    const didCloseEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didClose, element);
+
+    input.focus();
+
+    await didOpenEventSpy.calledOnce();
+    expect(input).to.have.attribute('aria-expanded', 'true');
+
+    await sendKeys({ press: 'Escape' });
+    await didCloseEventSpy.calledOnce();
+
+    expect(input).to.have.attribute('aria-expanded', 'false');
+  });
+
   it('select by mouse', async () => {
     const didOpenEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didOpen, element);
     const optionSelectedEventSpy = new EventSpy(
       SbbAutocompleteGridOptionElement.events.optionSelected,
     );
+    const inputEventSpy = new EventSpy('input', input);
+    const changeEventSpy = new EventSpy('change', input);
     const optTwo = element.querySelector<SbbAutocompleteGridOptionElement>('#option-2')!;
 
     input.focus();
     await didOpenEventSpy.calledOnce();
+
     const positionRect = optTwo.getBoundingClientRect();
 
     await sendMouse({
@@ -165,8 +187,11 @@ describe(`sbb-autocomplete-grid`, () => {
     });
     await waitForLitRender(element);
 
+    expect(inputEventSpy.count).to.be.equal(1);
+    expect(changeEventSpy.count).to.be.equal(1);
     expect(optionSelectedEventSpy.count).to.be.equal(1);
     expect(optionSelectedEventSpy.firstEvent!.target).to.have.property('id', 'option-2');
+    expect(document.activeElement).to.be.equal(input);
   });
 
   it('select button and get related option', async () => {
@@ -187,7 +212,7 @@ describe(`sbb-autocomplete-grid`, () => {
     await clickSpy.calledOnce();
     expect(clickSpy.count).to.be.equal(1);
     expect(
-      (clickSpy.firstEvent!.target as SbbAutocompleteGridButtonElement).option!.textContent,
+      (clickSpy.firstEvent!.target as SbbAutocompleteGridButtonElement).option!.textContent!.trim(),
     ).to.be.equal('Option 1');
     expect(
       (clickSpy.firstEvent!.target as SbbAutocompleteGridButtonElement).option!.value,
@@ -244,8 +269,10 @@ describe(`sbb-autocomplete-grid`, () => {
     const optionSelectedEventSpy = new EventSpy(
       SbbAutocompleteGridOptionElement.events.optionSelected,
     );
-    const optOne = element.querySelector('#option-1');
-    const optTwo = element.querySelector('#option-2');
+    const inputEventSpy = new EventSpy('input', input);
+    const changeEventSpy = new EventSpy('change', input);
+    const optOne = element.querySelector<SbbAutocompleteGridOptionElement>('#option-1');
+    const optTwo = element.querySelector<SbbAutocompleteGridOptionElement>('#option-2');
     const keydownSpy = new EventSpy('keydown', input);
 
     input.focus();
@@ -269,11 +296,27 @@ describe(`sbb-autocomplete-grid`, () => {
 
     expect(optTwo).not.to.have.attribute('data-active');
     expect(optTwo).to.have.attribute('selected');
+    expect(inputEventSpy.count).to.be.equal(1);
+    expect(changeEventSpy.count).to.be.equal(1);
     expect(optionSelectedEventSpy.count).to.be.equal(1);
     expect(input).to.have.attribute('aria-expanded', 'false');
     expect(input).not.to.have.attribute('aria-activedescendant');
   });
 
+  it('should not close on disabled option click', async () => {
+    const didOpenEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didOpen, element);
+    const optOne = element.querySelector<SbbAutocompleteGridOptionElement>('#option-1')!;
+    optOne.disabled = true;
+
+    input.focus();
+    await didOpenEventSpy.calledOnce();
+
+    optOne.click();
+
+    await aTimeout(0);
+    expect(element).to.have.attribute('data-state', 'opened');
+  });
+
   it('opens and select button with keyboard', async () => {
     const didOpenEventSpy = new EventSpy(SbbAutocompleteGridElement.events.didOpen, element);
     const clickSpy = new EventSpy('click');
@@ -307,10 +350,11 @@ describe(`sbb-autocomplete-grid`, () => {
     await sendKeys({ press: 'Enter' });
     await clickSpy.calledTimes(2);
     expect(clickSpy.count).to.be.equal(2);
+    expect(element).to.have.attribute('data-state', 'opened');
   });
 
   it('should stay closed when disabled', async () => {
-    input.setAttribute('disabled', '');
+    input.toggleAttribute('disabled', true);
 
     input.focus();
     await waitForLitRender(element);
@@ -326,7 +370,7 @@ describe(`sbb-autocomplete-grid`, () => {
   });
 
   it('should stay closed when readonly', async () => {
-    input.setAttribute('readonly', '');
+    input.toggleAttribute('readonly', true);
 
     input.focus();
     await waitForLitRender(element);

diff --git a/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.ts b/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.ts
@@ -6,7 +6,7 @@ import { hostAttributes } from '../../core/decorators.js';
 import { isSafari } from '../../core/dom.js';
 import { setAriaComboBoxAttributes } from '../../core/overlay.js';
 import type { SbbDividerElement } from '../../divider.js';
-import type { SbbOptGroupElement, SbbOptionElement } from '../../option.js';
+import type { SbbOptGroupElement } from '../../option.js';
 import type { SbbAutocompleteGridButtonElement } from '../autocomplete-grid-button.js';
 import { SbbAutocompleteGridOptionElement } from '../autocomplete-grid-option.js';
 import type { SbbAutocompleteGridRowElement } from '../autocomplete-grid-row.js';
@@ -54,16 +54,6 @@ class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
     );
   }
 
-  protected onOptionClick(event: MouseEvent): void {
-    if (
-      (event.target as Element).localName !== 'sbb-autocomplete-grid-option' ||
-      (event.target as SbbOptionElement).disabled
-    ) {
-      return;
-    }
-    this.close();
-  }
-
   public override connectedCallback(): void {
     super.connectedCallback();
     const signal = this.abort.signal;