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

feat(dialog): add is-alert-dialog option #2445

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/cuddly-bottles-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': minor
---

[dialog] add an option to set role="alertdialog" instead of the default role="dialog"
68 changes: 67 additions & 1 deletion docs/components/dialog/use-cases.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ Its purpose is to make it easy to use our Overlay System declaratively.
```js script
import { html } from '@mdjs/mdjs-preview';
import '@lion/ui/define/lion-dialog.js';

import '@lion/ui/define/lion-form.js';
import '@lion/ui/define/lion-input.js';
import { demoStyle } from './src/demoStyle.js';
import './src/styled-dialog-content.js';
import './src/slots-dialog-content.js';
Expand All @@ -23,6 +24,71 @@ import './src/external-dialog.js';
</lion-dialog>
```

## Alert dialog

In some cases the dialog should act like an [alertdialog](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/examples/alertdialog/), which is a combination of an alert and dialog. If that is the case, you can add `is-alert-dialog` attribute, which sets the correct role on the dialog.

```js preview-story
export const alertDialog = () => {
const submitHandler = ev => {
const formData = ev.target.serializedValue;
console.log('formData', formData);
if (!ev.target.hasFeedbackFor?.includes('error')) {
fetch('/api/foo/', {
method: 'POST',
body: JSON.stringify(formData),
});
}
};
const resetHandler = ev => {
ev.target.dispatchEvent(new Event('close-overlay', { bubbles: true }));
ev.target.dispatchEvent(new Event('form-reset', { bubbles: true }));
};
const formResetHandler = ev => {
ev.currentTarget.resetGroup();
};
return html`
<style>
${demoStyle} .button__group {
display: flex;
align-items: center;
}
.button-submit {
margin-top: 4px;
margin-bottom: 4px;
}
.dialog {
margin-bottom: 4px;
}
</style>
<lion-form @submit="${submitHandler}" @form-reset="${formResetHandler}">
<form>
<lion-input name="firstName" label="First Name"></lion-input>
<lion-input name="lastName" label="Last Name"></lion-input>
<div class="button__group">
<button class="button-submit">Submit</button>
<lion-dialog is-alert-dialog class="dialog">
<button type="button" slot="invoker">Reset</button>
<div slot="content" class="demo-box">
Are you sure you want to clear the input field?
<button orange type="button" @click="${resetHandler}">Yes</button>
<button
grey
type="button"
@click="${ev =>
ev.target.dispatchEvent(new Event('close-overlay', { bubbles: true }))}"
>
No
</button>
</div>
</lion-dialog>
</div>
</form>
</lion-form>
`;
};
```
## External trigger
```js preview-story
Expand Down
24 changes: 24 additions & 0 deletions docs/fundamentals/systems/overlays/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,30 @@ export const placementGlobal = () => {
};
```

## isAlertDialog

In some cases the dialog should act like an [alertdialog](https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/examples/alertdialog/), which is a combination of an alert and dialog. If that is the case, you can add `is-alert-dialog` attribute, which sets the correct role on the dialog.

```js preview-story
export const alertDialog = () => {
const placementModeGlobalConfig = { placementMode: 'global', isAlertDialog: true };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the best practice here would be to combine it with withModalDialog()

return html`
<demo-el-using-overlaymixin .config="${placementModeGlobalConfig}">
<button slot="invoker">Click me to open the alert dialog!</button>
<div slot="content" class="demo-overlay">
Hello! You can close this notification here:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As application of the role is dependent on content, maybe the content should reflect this a bit more.

Suggested change
Hello! You can close this notification here:
Are you sure you want to perform this action?

<button
class="close-button"
@click="${e => e.target.dispatchEvent(new Event('close-overlay', { bubbles: true }))}"
>
</button>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a "No" and "Yes" button here?

</demo-el-using-overlaymixin>
`;
};
```

## isTooltip (placementMode: 'local')

As specified in the [overlay rationale](./rationale.md) there are only two official types of overlays: dialogs and tooltips. And their main differences are:
Expand Down
13 changes: 13 additions & 0 deletions packages/ui/components/dialog/src/LionDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,26 @@ import { html, LitElement } from 'lit';
import { OverlayMixin, withModalDialogConfig } from '@lion/ui/overlays.js';

export class LionDialog extends OverlayMixin(LitElement) {
/** @type {any} */
static get properties() {
return {
isAlertDialog: { type: Boolean, attribute: 'is-alert-dialog' },
};
}

constructor() {
super();
this.isAlertDialog = false;
}

/**
* @protected
*/
// eslint-disable-next-line class-methods-use-this
_defineOverlayConfig() {
return {
...withModalDialogConfig(),
isAlertDialog: this.isAlertDialog,
};
}

Expand Down
56 changes: 56 additions & 0 deletions packages/ui/components/dialog/test/lion-dialog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,27 @@ describe('lion-dialog', () => {
});

describe('Accessibility', () => {
it('passes a11y audit', async () => {
const el = await fixture(html`
<lion-dialog>
<button slot="invoker">Invoker</button>
<div slot="content" class="dialog" aria-label="Dialog">Hey there</div>
</lion-dialog>
`);
await expect(el).to.be.accessible();
});

it('passes a11y audit when opened', async () => {
const el = await fixture(html`
<lion-dialog opened>
<button slot="invoker">Invoker</button>
<div slot="content" class="dialog" aria-label="Dialog">Hey there</div>
</lion-dialog>
`);
// error expected since we put role="none" on the dialog itself, which is valid but not recognized by Axe
await expect(el).to.be.accessible({ ignoredRules: ['aria-allowed-role'] });
});

it('does not add [aria-expanded] to invoker button', async () => {
const el = await fixture(
html` <lion-dialog>
Expand All @@ -187,6 +208,41 @@ describe('lion-dialog', () => {
await aTimeout(0);
expect(invokerButton.getAttribute('aria-expanded')).to.equal(null);
});

it('has role="dialog" by default', async () => {
const el = await fixture(
html` <lion-dialog>
<div slot="content" class="dialog">Hey there</div>
<button slot="invoker">Popup button</button>
</lion-dialog>`,
);
const contentNode = /** @type {HTMLElement} */ (el.querySelector('[slot="content"]'));

expect(contentNode.getAttribute('role')).to.equal('dialog');
});

it('has role="alertdialog" by when "is-alert-dialog" is set', async () => {
const el = await fixture(
html` <lion-dialog is-alert-dialog>
<div slot="content" class="dialog">Hey there</div>
<button slot="invoker">Popup button</button>
</lion-dialog>`,
);
const contentNode = /** @type {HTMLElement} */ (el.querySelector('[slot="content"]'));

expect(contentNode.getAttribute('role')).to.equal('alertdialog');
});

it('passes a11y audit when opened and role="alertdialog"', async () => {
const el = await fixture(html`
<lion-dialog opened is-alert-dialog>
<button slot="invoker">Invoker</button>
<div slot="content" class="dialog" aria-label="Dialog">Hey there</div>
</lion-dialog>
`);
// error expected since we put role="none" on the dialog itself, which is valid but not recognized by Axe
await expect(el).to.be.accessible({ ignoredRules: ['aria-allowed-role'] });
});
});

describe('Edge cases', () => {
Expand Down
13 changes: 12 additions & 1 deletion packages/ui/components/overlays/src/OverlayController.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export class OverlayController extends EventTarget {
hidesOnOutsideEsc: false,
hidesOnOutsideClick: false,
isTooltip: false,
isAlertDialog: false,
invokerRelation: 'description',
visibilityTriggerFunction: undefined,
handlesAccessibility: false,
Expand Down Expand Up @@ -381,6 +382,14 @@ export class OverlayController extends EventTarget {
return /** @type {boolean} */ (this.config?.isTooltip);
}

/**
* The alertdialog role is to be used on modal alert dialogs that interrupt a user's workflow
* to communicate an important message and require a response.
*/
get isAlertDialog() {
return /** @type {boolean} */ (this.config?.isAlertDialog);
}

/**
* By default, the tooltip content is a 'description' for the invoker (uses aria-describedby).
* Setting this property to 'label' makes the content function as a label (via aria-labelledby)
Expand Down Expand Up @@ -672,7 +681,9 @@ export class OverlayController extends EventTarget {
if (this.invokerNode && !isModal) {
this.invokerNode.setAttribute('aria-expanded', `${this.isShown}`);
}
if (!this.contentNode.getAttribute('role')) {
if (this.isAlertDialog) {
this.contentNode.setAttribute('role', 'alertdialog');
} else if (!this.contentNode.getAttribute('role')) {
this.contentNode.setAttribute('role', 'dialog');
}
}
Expand Down
16 changes: 16 additions & 0 deletions packages/ui/components/overlays/test/OverlayController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1876,6 +1876,22 @@ describe('OverlayController', () => {
it.skip('adds attributes inert and aria-hidden="true" on all siblings of rootNode if an overlay is shown', async () => {});
it.skip('disables pointer events and selection on inert elements', async () => {});

describe('Alert dialog', () => {
it('sets role="alertdialog" when isAlertDialog is set', async () => {
const invokerNode = /** @type {HTMLElement} */ (
await fixture('<div role="button">invoker</div>')
);
const ctrl = new OverlayController({
...withLocalTestConfig(),
handlesAccessibility: true,
isAlertDialog: true,
invokerNode,
});

expect(ctrl.contentNode?.getAttribute('role')).to.equal('alertdialog');
});
});

describe('Tooltip', () => {
it('adds [aria-describedby] on invoker', async () => {
const invokerNode = /** @type {HTMLElement} */ (
Expand Down
6 changes: 4 additions & 2 deletions packages/ui/components/overlays/types/OverlayConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface OverlayConfig {
contentNode?: HTMLElement;
/** The wrapper element of contentNode, used to supply inline positioning styles. When a Popper arrow is needed, it acts as parent of the arrow node. Will be automatically created for global and non projected contentNodes. Required when used in shadow dom mode or when Popper arrow is supplied. Essential for allowing webcomponents to style their projected contentNodes */
contentWrapperNode?: HTMLElement;
/** The element that is placed behin the contentNode. When not provided and `hasBackdrop` is true, a backdropNode will be automatically created */
/** The element that is placed behind the contentNode. When not provided and `hasBackdrop` is true, a backdropNode will be automatically created */
backdropNode?: HTMLElement;
/** The element that should be called `.focus()` on after dialog closes */
elementToFocusAfterHide?: HTMLElement;
Expand All @@ -59,7 +59,7 @@ export interface OverlayConfig {
trapsKeyboardFocus?: boolean;
/** Hides the overlay when pressing [ esc ] */
hidesOnEsc?: boolean;
/** Hides the overlay when clicking next to it, exluding invoker */
/** Hides the overlay when clicking next to it, excluding invoker */
hidesOnOutsideClick?: boolean;
/** Hides the overlay when pressing esc, even when contentNode has no focus */
hidesOnOutsideEsc?: boolean;
Expand All @@ -82,6 +82,8 @@ export interface OverlayConfig {

/** Has a totally different interaction- and accessibility pattern from all other overlays. Will behave as role="tooltip" element instead of a role="dialog" element */
isTooltip?: boolean;
/** The alertdialog role is to be used on modal alert dialogs that interrupt a user's workflow to communicate an important message and require a response. */
isAlertDialog?: boolean;
/** By default, the tooltip content is a 'description' for the invoker (uses aria-describedby) Setting this property to 'label' makes the content function as a label (via aria-labelledby) */
invokerRelation?: 'label' | 'description';

Expand Down
Loading