Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sbb-alert, sbb-alert-group): remove dismissal event and method #3216

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 19 additions & 29 deletions src/elements/alert/alert-group/alert-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import { html } from 'lit/static-html.js';
import type { SbbTransparentButtonElement } from '../../button.js';
import { fixture } from '../../core/testing/private.js';
import { waitForCondition, EventSpy, waitForLitRender } from '../../core/testing.js';
import type { SbbAlertElement } from '../alert.js';
import { SbbAlertElement } from '../alert.js';

import { SbbAlertGroupElement } from './alert-group.js';

import '../alert.js';

describe(`sbb-alert-group`, () => {
let element: SbbAlertGroupElement;

Expand All @@ -26,12 +24,18 @@ describe(`sbb-alert-group`, () => {
accessibility-title="${accessibilityTitle}"
accessibility-title-level="${accessibilityTitleLevel}"
>
<sbb-alert title-content="Interruption" href="www.sbb.ch">First</sbb-alert>
<sbb-alert title-content="Interruption" href="www.sbb.ch">Second</sbb-alert>
<sbb-alert title-content="Interruption" href="www.sbb.ch" id="alert1">First</sbb-alert>
<sbb-alert title-content="Interruption" href="www.sbb.ch" id="alert2">Second</sbb-alert>
</sbb-alert-group>
`);
const didDismissAlertSpy = new EventSpy(SbbAlertGroupElement.events.didDismissAlert);
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();

// Then two alerts should be rendered and accessibility title should be displayed
expect(element.querySelectorAll('sbb-alert').length).to.be.equal(2);
Expand All @@ -40,61 +44,47 @@ describe(`sbb-alert-group`, () => {
expect(alertGroupTitle.localName).to.be.equal(`h${accessibilityTitleLevel}`);

// When clicking on close button of the first alert
const closeButton = element
.querySelector<SbbAlertElement>('sbb-alert')!
.shadowRoot!.querySelector<SbbTransparentButtonElement>(
'.sbb-alert__close-button-wrapper sbb-transparent-button',
)!;
const closeButton = alert1.shadowRoot!.querySelector<SbbTransparentButtonElement>(
'.sbb-alert__close-button-wrapper sbb-transparent-button',
)!;
closeButton.focus();
closeButton.click();
await waitForLitRender(element);
await alert1ClosedEventSpy.calledOnce();

// Then one alert should be removed from sbb-alert-group, tabindex should be set to 0,
// focus should be on sbb-alert-group and accessibility title should still be rendered.
// Moreover, didDismissAlert event should have been fired.
await waitForCondition(
() =>
didDismissAlertSpy.events.length === 1 &&
element.querySelectorAll('sbb-alert').length === 1,
);
expect(didDismissAlertSpy.count).to.be.equal(1);
await waitForCondition(() => element.querySelectorAll('sbb-alert').length === 1);
expect(element.querySelectorAll('sbb-alert').length).to.be.equal(1);
expect(element.tabIndex).to.be.equal(0);
expect(document.activeElement!.id).to.be.equal(alertGroupId);
expect(
element.shadowRoot!.querySelector('.sbb-alert-group__title')!.textContent!.trim(),
).to.be.equal(accessibilityTitle);
expect(emptySpy.count).not.to.be.greaterThan(0);
expect(emptySpy.count).to.be.equal(0);

// When clicking on close button of the second alert
element
.querySelector<SbbAlertElement>('sbb-alert')!
alert2
.shadowRoot!.querySelector<SbbTransparentButtonElement>(
'.sbb-alert__close-button-wrapper sbb-transparent-button',
)!
.click();
await waitForLitRender(element);
await alert2ClosedEventSpy.calledOnce();

// Then the alert should be removed from sbb-alert-group, tabindex should be set to 0,
// focus should be on sbb-alert-group, accessibility title should be removed and empty event should be fired.
await waitForCondition(
() =>
didDismissAlertSpy.events.length === 2 &&
element.querySelectorAll('sbb-alert').length === 0,
);
expect(didDismissAlertSpy.count).to.be.equal(2);
await emptySpy.calledOnce();
expect(element.querySelectorAll('sbb-alert').length).to.be.equal(0);
expect(element.tabIndex).to.be.equal(0);
expect(document.activeElement!.id).to.be.equal(alertGroupId);
expect(element.shadowRoot!.querySelector('.sbb-alert-group__title')).to.be.null;
expect(emptySpy.count).to.be.greaterThan(0);

// When clicking away
await sendMouse({ type: 'click', position: [0, 0] });
await waitForLitRender(element);

// Then the active element id should be unset and tabindex should be removed
await waitForCondition(() => document.activeElement!.id === '');
expect(document.activeElement!.id).to.be.equal('');
expect(element.tabIndex).to.be.equal(-1);
});
Expand Down
2 changes: 1 addition & 1 deletion src/elements/alert/alert-group/alert-group.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const meta: Meta = {
decorators: [withActions as Decorator],
parameters: {
actions: {
handles: [SbbAlertGroupElement.events.didDismissAlert, SbbAlertGroupElement.events.empty],
handles: [SbbAlertGroupElement.events.empty],
jeripeierSBB marked this conversation as resolved.
Show resolved Hide resolved
},
docs: {
extractComponentDescription: () => readme,
Expand Down
16 changes: 0 additions & 16 deletions src/elements/alert/alert-group/alert-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ import style from './alert-group.scss?lit&inline';
*
* @slot - Use the unnamed slot to add `sbb-alert` elements to the `sbb-alert-group`.
* @slot accessibility-title - title for this `sbb-alert-group` which is only visible for screen reader users.
* @event {CustomEvent<SbbAlertElement>} didDismissAlert - Emits when an alert was removed from DOM.
* @event {CustomEvent<void>} empty - Emits when `sbb-alert-group` becomes empty.
*/
export
@customElement('sbb-alert-group')
class SbbAlertGroupElement extends SbbHydrationMixin(LitElement) {
public static override styles: CSSResultGroup = style;
public static readonly events = {
didDismissAlert: 'didDismissAlert',
empty: 'empty',
} as const;

Expand All @@ -50,12 +48,6 @@ class SbbAlertGroupElement extends SbbHydrationMixin(LitElement) {
/** Whether the group currently has any alerts. */
@state() private accessor _hasAlerts: boolean = false;

/** Emits when an alert was removed from DOM. */
private _didDismissAlert: EventEmitter<SbbAlertElement> = new EventEmitter(
this,
SbbAlertGroupElement.events.didDismissAlert,
);

/** Emits when `sbb-alert-group` becomes empty. */
private _empty: EventEmitter<void> = new EventEmitter(this, SbbAlertGroupElement.events.empty);

Expand All @@ -64,13 +56,6 @@ class SbbAlertGroupElement extends SbbHydrationMixin(LitElement) {
public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener(
SbbAlertElement.events.dismissalRequested,
(e) => (e.target as SbbAlertElement).close(),
{
signal,
},
);
this.addEventListener(SbbAlertElement.events.didClose, (e) => this._alertClosed(e), {
signal,
});
Expand All @@ -79,7 +64,6 @@ class SbbAlertGroupElement extends SbbHydrationMixin(LitElement) {
private _alertClosed(event: Event): void {
const target = event.target as SbbAlertElement;
const hasFocusInsideAlertGroup = document.activeElement === target;
this._didDismissAlert.emit(target);

// Restore focus
if (hasFocusInsideAlertGroup) {
Expand Down
9 changes: 4 additions & 5 deletions src/elements/alert/alert-group/readme.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
The `sbb-alert-group` manages the dismissal and accessibility of one or multiple
The `sbb-alert-group` manages the accessibility of one or multiple
[sbb-alert](/docs/elements-sbb-alert-sbb-alert--docs) and also its visual gap between each other.

```html
Expand Down Expand Up @@ -51,10 +51,9 @@ and therefore interrupts screen reader flow, to immediately read out the alert c

## Events

| Name | Type | Description | Inherited From |
| ----------------- | ------------------------------ | ------------------------------------------- | -------------- |
| `didDismissAlert` | `CustomEvent<SbbAlertElement>` | Emits when an alert was removed from DOM. | |
| `empty` | `CustomEvent<void>` | Emits when `sbb-alert-group` becomes empty. | |
| Name | Type | Description | Inherited From |
| ------- | ------------------- | ------------------------------------------- | -------------- |
| `empty` | `CustomEvent<void>` | Emits when `sbb-alert-group` becomes empty. | |

## Slots

Expand Down
29 changes: 24 additions & 5 deletions src/elements/alert/alert/alert.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assert, expect } from '@open-wc/testing';
import { assert, aTimeout, expect } from '@open-wc/testing';
import { html } from 'lit/static-html.js';

import { fixture } from '../../core/testing/private.js';
Expand All @@ -19,7 +19,6 @@ describe(`sbb-alert`, () => {
const didOpenSpy = new EventSpy(SbbAlertElement.events.didOpen);
const willCloseSpy = new EventSpy(SbbAlertElement.events.willClose);
const didCloseSpy = new EventSpy(SbbAlertElement.events.didClose);
const dismissalSpy = new EventSpy(SbbAlertElement.events.dismissalRequested);

const alert: SbbAlertElement = await fixture(
html`<sbb-alert title-content="disruption">Interruption</sbb-alert>`,
Expand All @@ -30,16 +29,36 @@ describe(`sbb-alert`, () => {
await didOpenSpy.calledOnce();
expect(didOpenSpy.count).to.be.equal(1);

alert.requestDismissal();
expect(dismissalSpy.count).to.be.equal(1);

alert.close();

await didCloseSpy.calledOnce();
expect(willCloseSpy.count).to.be.equal(1);
expect(didCloseSpy.count).to.be.equal(1);
});

it('should respect canceled willClose event', async () => {
const didOpenSpy = new EventSpy(SbbAlertElement.events.didOpen);
const willCloseSpy = new EventSpy(SbbAlertElement.events.willClose);
const didCloseSpy = new EventSpy(SbbAlertElement.events.didClose);

const alert: SbbAlertElement = await fixture(
html`<sbb-alert title-content="disruption">Interruption</sbb-alert>`,
);

alert.addEventListener(SbbAlertElement.events.willClose, (ev) => ev.preventDefault());

await didOpenSpy.calledOnce();

alert.close();

await willCloseSpy.calledOnce();
expect(willCloseSpy.count).to.be.equal(1);

// Wait a period to ensure the didCLose event was not dispatched.
await aTimeout(10);
expect(didCloseSpy.count).to.be.equal(0);
});

it('should hide close button in readonly mode', async () => {
alert = await fixture(
html`<sbb-alert title-content="Interruption" readonly>Alert content</sbb-alert>`,
Expand Down
22 changes: 4 additions & 18 deletions src/elements/alert/alert/alert.stories.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,23 @@
import { withActions } from '@storybook/addon-actions/decorator';
import type { InputType } from '@storybook/types';
import type { Meta, StoryObj, ArgTypes, Args, Decorator } from '@storybook/web-components';
import type { Args, ArgTypes, Decorator, Meta, StoryObj } from '@storybook/web-components';
import type { TemplateResult } from 'lit';
import { html, nothing } from 'lit';
import { html } from 'lit';

import { sbbSpread } from '../../../storybook/helpers/spread.js';

import { SbbAlertElement } from './alert.js';
import readme from './readme.md?raw';

const Default = ({ 'content-slot-text': contentSlotText, ...args }: Args): TemplateResult => html`
<sbb-alert
${sbbSpread(args)}
@dismissalRequested=${(e: Event) => (e.target! as SbbAlertElement).close()}
>${contentSlotText}</sbb-alert
>
<sbb-alert ${sbbSpread(args)}>${contentSlotText}</sbb-alert>
`;

const DefaultWithOtherContent = (args: Args): TemplateResult => {
return html`
<div>
${Default(args)}
<p>Other Content on the page.</p>
${!args.readonly
? html`<p>
Dismissal event of the alert has to be caught by the consumer and the alert has to be
manually removed from DOM. See 'sbb-alert-group' for demonstration.
</p>`
: nothing}
</div>
`;
};
Expand Down Expand Up @@ -218,11 +208,7 @@ const meta: Meta = {
decorators: [withActions as Decorator],
parameters: {
actions: {
handles: [
SbbAlertElement.events.willOpen,
SbbAlertElement.events.didOpen,
SbbAlertElement.events.dismissalRequested,
],
handles: [SbbAlertElement.events.willOpen, SbbAlertElement.events.didOpen],
jeripeierSBB marked this conversation as resolved.
Show resolved Hide resolved
},
docs: {
extractComponentDescription: () => readme,
Expand Down
23 changes: 2 additions & 21 deletions src/elements/alert/alert/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { customElement, property } from 'lit/decorators.js';
import { type LinkTargetType, SbbOpenCloseBaseElement } from '../../core/base-elements.js';
import { SbbLanguageController } from '../../core/controllers.js';
import { forceType } from '../../core/decorators.js';
import { EventEmitter } from '../../core/eventing.js';
import { i18nCloseAlert, i18nFindOutMore } from '../../core/i18n.js';
import { SbbIconNameMixin } from '../../icon.js';
import type { SbbTitleLevel } from '../../title.js';
Expand All @@ -26,7 +25,6 @@ import '../../title.js';
* @event {CustomEvent<void>} didOpen - Emits when the opening animation ends.
* @event {CustomEvent<void>} willClose - Emits when the closing animation starts. Can be canceled.
* @event {CustomEvent<void>} didClose - Emits when the closing animation ends.
* @event {CustomEvent<void>} dismissalRequested - Emits when dismissal of an alert was requested.
*/
export
@customElement('sbb-alert')
Expand All @@ -37,7 +35,6 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {
didOpen: 'didOpen',
willClose: 'willClose',
didClose: 'didClose',
dismissalRequested: 'dismissalRequested',
} as const;

/**
Expand Down Expand Up @@ -96,24 +93,8 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {
/** The enabled animations. */
@property({ reflect: true }) public accessor animation: 'open' | 'close' | 'all' | 'none' = 'all';

/**
* Emits when dismissal of an alert was requested.
* @deprecated
*/
private _dismissalRequested: EventEmitter<void> = new EventEmitter(
this,
SbbAlertElement.events.dismissalRequested,
);

private _language = new SbbLanguageController(this);

/** Requests dismissal of the alert.
* @deprecated in favour of 'willClose' and 'didClose' events
*/
public requestDismissal(): void {
this._dismissalRequested.emit();
}

/** Open the alert. */
public open(): void {
this.willOpen.emit();
Expand All @@ -122,7 +103,7 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {

/** Close the alert. */
public close(): void {
if (this.willClose.emit()) {
if (this.state === 'opened' && this.willClose.emit()) {
this.state = 'closing';
}
}
Expand Down Expand Up @@ -194,7 +175,7 @@ class SbbAlertElement extends SbbIconNameMixin(SbbOpenCloseBaseElement) {
negative
size=${this.size === 'l' ? 'm' : this.size}
icon-name="cross-small"
@click=${() => this.requestDismissal()}
@click=${() => this.close()}
aria-label=${i18nCloseAlert[this._language.current]}
class="sbb-alert__close-button"
></sbb-transparent-button>
Expand Down
Loading
Loading