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: set login overlay role to dialog, use title as aria-label (#7723) (CP: 24.4) #7741

Merged
merged 1 commit into from
Sep 3, 2024
Merged
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
23 changes: 2 additions & 21 deletions packages/login/src/vaadin-lit-login-overlay-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@
*/
import { html, LitElement } from 'lit';
import { defineCustomElement } from '@vaadin/component-base/src/define.js';
import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';
import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js';
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
import { LoginOverlayWrapperMixin } from './vaadin-login-overlay-wrapper-mixin.js';
import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles.js';

/**
Expand All @@ -18,7 +17,7 @@ import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles
* @extends HTMLElement
* @private
*/
class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolylitMixin(LitElement)))) {
class LoginOverlayWrapper extends LoginOverlayWrapperMixin(ThemableMixin(PolylitMixin(LitElement))) {
static get is() {
return 'vaadin-login-overlay-wrapper';
}
Expand All @@ -27,24 +26,6 @@ class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolylitMix
return [overlayStyles, loginOverlayWrapperStyles];
}

static get properties() {
return {
/**
* Title of the application.
*/
title: {
type: String,
},

/**
* Application description. Displayed under the title.
*/
description: {
type: String,
},
};
}

/** @protected */
render() {
return html`
Expand Down
1 change: 1 addition & 0 deletions packages/login/src/vaadin-lit-login-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class LoginOverlay extends LoginOverlayMixin(ElementMixin(ThemableMixin(PolylitM
.opened="${this.opened}"
.title="${this.title}"
.description="${this.description}"
role="dialog"
focus-trap
with-backdrop
theme="${ifDefined(this._theme)}"
Expand Down
71 changes: 71 additions & 0 deletions packages/login/src/vaadin-login-overlay-wrapper-mixin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* @license
* Copyright (c) 2018 - 2024 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
import { SlotObserver } from '@vaadin/component-base/src/slot-observer';
import { generateUniqueId } from '@vaadin/component-base/src/unique-id-utils';
import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';

/**
* @polymerMixin
* @mixes DirMixin
* @mixes OverlayMixin
*/
export const LoginOverlayWrapperMixin = (superClass) =>
class LoginOverlayWrapperMixin extends OverlayMixin(DirMixin(superClass)) {
static get properties() {
return {
/**
* Title of the application.
*/
title: {
type: String,
observer: '_titleChanged',
},

/**
* Application description. Displayed under the title.
*/
description: {
type: String,
},
};
}

/** @protected */
ready() {
super.ready();

// Use slot observer instead of slot controller since the latter
// does not work well with teleporting (it removes custom title).
const slot = this.shadowRoot.querySelector('slot[name="title"]');
this._titleSlotObserver = new SlotObserver(slot, () => {
const title = slot.assignedElements({ flatten: true })[0];
if (!title) {
return;
}

// Only set ID on the custom slotted title and link it using
// aria-labelledby as the default title is in the shadow DOM.
if (title.getAttribute('part') === 'title') {
this.setAttribute('aria-label', this.title);
this.removeAttribute('aria-labelledby');
} else {
if (!title.id) {
title.id = `login-overlay-title-${generateUniqueId()}`;
}
this.removeAttribute('aria-label');
this.setAttribute('aria-labelledby', title.id);
}
});
}

/** @private */
_titleChanged(title) {
if (title && this.hasAttribute('aria-label')) {
this.setAttribute('aria-label', title);
}
}
};
26 changes: 3 additions & 23 deletions packages/login/src/vaadin-login-overlay-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
*/
import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
import { defineCustomElement } from '@vaadin/component-base/src/define.js';
import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';
import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js';
import { registerStyles, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
import { LoginOverlayWrapperMixin } from './vaadin-login-overlay-wrapper-mixin.js';
import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles.js';

registerStyles('vaadin-login-overlay-wrapper', [overlayStyles, loginOverlayWrapperStyles], {
Expand All @@ -19,34 +18,15 @@ registerStyles('vaadin-login-overlay-wrapper', [overlayStyles, loginOverlayWrapp
* An element used internally by `<vaadin-login-overlay>`. Not intended to be used separately.
*
* @extends HTMLElement
* @mixes DirMixin
* @mixes OverlayMixin
* @mixes LoginOverlayWrapperMixin
* @mixes ThemableMixin
* @private
*/
class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolymerElement))) {
class LoginOverlayWrapper extends LoginOverlayWrapperMixin(ThemableMixin(PolymerElement)) {
static get is() {
return 'vaadin-login-overlay-wrapper';
}

static get properties() {
return {
/**
* Title of the application.
*/
title: {
type: String,
},

/**
* Application description. Displayed under the title.
*/
description: {
type: String,
},
};
}

static get template() {
return html`
<div id="backdrop" part="backdrop" hidden$="[[!withBackdrop]]"></div>
Expand Down
1 change: 1 addition & 0 deletions packages/login/src/vaadin-login-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class LoginOverlay extends LoginOverlayMixin(ElementMixin(ThemableMixin(PolymerE
<vaadin-login-overlay-wrapper
id="vaadinLoginOverlayWrapper"
opened="{{opened}}"
role="dialog"
focus-trap
with-backdrop
title="[[title]]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ export const snapshots = {};

snapshots["vaadin-login-overlay host default"] =
`<vaadin-login-overlay-wrapper
aria-label="App name"
focus-trap=""
id="vaadinLoginOverlayWrapper"
opened=""
role="dialog"
with-backdrop=""
>
<vaadin-login-form
Expand Down Expand Up @@ -125,9 +127,11 @@ snapshots["vaadin-login-overlay host default"] =

snapshots["vaadin-login-overlay host i18n"] =
`<vaadin-login-overlay-wrapper
aria-label="Sovelluksen nimi"
focus-trap=""
id="vaadinLoginOverlayWrapper"
opened=""
role="dialog"
with-backdrop=""
>
<vaadin-login-form
Expand Down Expand Up @@ -247,10 +251,12 @@ snapshots["vaadin-login-overlay host i18n"] =

snapshots["vaadin-login-overlay host overlay class"] =
`<vaadin-login-overlay-wrapper
aria-label="App name"
class="custom login-overlay"
focus-trap=""
id="vaadinLoginOverlayWrapper"
opened=""
role="dialog"
with-backdrop=""
>
<vaadin-login-form
Expand Down
44 changes: 44 additions & 0 deletions packages/login/test/login-overlay.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ describe('title and description', () => {
expect(overlay.title).to.be.equal(overlay.i18n.header.title);
expect(overlay.description).to.be.equal(overlay.i18n.header.description);
});

it('should update aria-label when title property changes', async () => {
overlay.title = 'New title';
await nextUpdate(overlay);
expect(overlay.$.vaadinLoginOverlayWrapper.getAttribute('aria-label')).to.equal('New title');
});
});

describe('title slot', () => {
Expand Down Expand Up @@ -219,6 +225,44 @@ describe('title slot', () => {
expect(titleElements.length).to.be.equal(1);
expect(titleElements[0].textContent).to.be.equal('Teleported title');
});

it('should link slotted title using aria-labelledby', () => {
const title = overlayWrapper.querySelector('[slot=title]');
expect(title.id).to.be.ok;
expect(overlayWrapper.hasAttribute('aria-label')).to.be.false;
expect(overlayWrapper.getAttribute('aria-labelledby')).to.equal(title.id);
});

it('should reset aria-labelledby when slotted title is removed', async () => {
overlay.opened = false;
await nextRender();

overlay.querySelector('[slot=title]').remove();

overlay.opened = true;
await nextRender();
expect(overlayWrapper.hasAttribute('aria-labelledby')).to.be.false;
expect(overlayWrapper.getAttribute('aria-label')).to.equal(overlay.title);
});

it('should not override custom id set on the slotted title', async () => {
overlay.opened = false;
await nextRender();

overlay.querySelector('[slot=title]').remove();

// Attach new slotted title with a custom ID
const title = document.createElement('h1');
title.id = 'custom-title';
title.setAttribute('slot', 'title');
overlay.appendChild(title);

overlay.opened = true;
await nextRender();

expect(overlayWrapper.getAttribute('aria-labelledby')).to.equal('custom-title');
expect(title.id).to.equal('custom-title');
});
});

describe('custom-form-area slot', () => {
Expand Down
Loading