Skip to content

Commit 7ed2da4

Browse files
web-padawanweb-padawan
authored
refactor: set aria-modal and tabindex on the popover (#10020)
1 parent 3d2e5a9 commit 7ed2da4

File tree

6 files changed

+123
-63
lines changed

6 files changed

+123
-63
lines changed

‎packages/popover/src/vaadin-popover-overlay.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class PopoverOverlay extends PopoverOverlayMixin(
3838
render() {
3939
return html`
4040
<div id="backdrop" part="backdrop" hidden ?hidden="${!this.withBackdrop}"></div>
41-
<div part="overlay" id="overlay" tabindex="0">
41+
<div part="overlay" id="overlay">
4242
<div part="arrow"></div>
4343
<div part="content" id="content"><slot></slot></div>
4444
</div>
@@ -76,6 +76,24 @@ class PopoverOverlay extends PopoverOverlayMixin(
7676
return this.owner;
7777
}
7878

79+
/**
80+
* Override method from OverlayFocusMixin to use owner as focus trap root
81+
* @protected
82+
* @override
83+
*/
84+
get _focusTrapRoot() {
85+
return this.owner;
86+
}
87+
88+
/**
89+
* Override method from OverlayFocusMixin to not set `aria-hidden`
90+
* @protected
91+
* @override
92+
*/
93+
get _useAriaHidden() {
94+
return false;
95+
}
96+
7997
/**
8098
* Override method from `OverlayMixin` to always add outside
8199
* click listener so that it can be used by modeless popover.

‎packages/popover/src/vaadin-popover.js

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,21 @@ class Popover extends PopoverPositionMixin(
215215

216216
static get styles() {
217217
return css`
218-
:host {
219-
display: contents;
218+
:host([opened]),
219+
:host([opening]),
220+
:host([closing]) {
221+
display: block !important;
222+
position: absolute;
223+
outline: none;
224+
}
225+
226+
:host,
227+
:host([hidden]) {
228+
display: none !important;
229+
}
230+
231+
:host(:focus) ::part(overlay) {
232+
outline: var(--vaadin-focus-ring-width) solid var(--vaadin-focus-ring-color);
220233
}
221234
`;
222235
}
@@ -531,6 +544,16 @@ class Popover extends PopoverPositionMixin(
531544

532545
this._overlayElement = this.$.overlay;
533546

547+
this.setAttribute('tabindex', '0');
548+
549+
this.addEventListener('focusin', (e) => {
550+
this.__onFocusIn(e);
551+
});
552+
553+
this.addEventListener('focusout', (e) => {
554+
this.__onFocusOut(e);
555+
});
556+
534557
if (!this.hasAttribute('role')) {
535558
this.role = 'dialog';
536559
}
@@ -564,6 +587,14 @@ class Popover extends PopoverPositionMixin(
564587
this.removeAttribute('aria-labelledby');
565588
}
566589
}
590+
591+
if (props.has('modal')) {
592+
if (this.modal) {
593+
this.setAttribute('aria-modal', 'true');
594+
} else {
595+
this.removeAttribute('aria-modal');
596+
}
597+
}
567598
}
568599

569600
/** @protected */
@@ -688,20 +719,18 @@ class Popover extends PopoverPositionMixin(
688719

689720
/** @private */
690721
__onGlobalTab(event) {
691-
const overlayPart = this._overlayElement.$.overlay;
692-
693-
// Move focus to the popover content on target element Tab
722+
// Move focus to the popover on target element Tab
694723
if (this.target && isElementFocused(this.__getTargetFocusable())) {
695724
event.preventDefault();
696-
overlayPart.focus();
725+
this.focus();
697726
return;
698727
}
699728

700729
// Move focus to the next element after target on content Tab
701-
const lastFocusable = this.__getLastFocusable(overlayPart);
730+
const lastFocusable = this.__getLastFocusable(this);
702731
if (lastFocusable && isElementFocused(lastFocusable)) {
703732
const focusable = this.__getNextBodyFocusable(this.__getTargetFocusable());
704-
if (focusable && focusable !== overlayPart) {
733+
if (focusable && focusable !== this) {
705734
event.preventDefault();
706735
focusable.focus();
707736
return;
@@ -711,7 +740,7 @@ class Popover extends PopoverPositionMixin(
711740
// Prevent focusing the popover content on previous element Tab
712741
const activeElement = getDeepActiveElement();
713742
const nextFocusable = this.__getNextBodyFocusable(activeElement);
714-
if (nextFocusable === overlayPart && lastFocusable) {
743+
if (nextFocusable === this && lastFocusable) {
715744
// Move focus to the last overlay focusable and do NOT prevent keydown
716745
// to move focus outside the popover content (e.g. to the URL bar).
717746
lastFocusable.focus();
@@ -720,16 +749,14 @@ class Popover extends PopoverPositionMixin(
720749

721750
/** @private */
722751
__onGlobalShiftTab(event) {
723-
const overlayPart = this._overlayElement.$.overlay;
724-
725752
// Prevent restoring focus after target blur on Shift + Tab
726753
if (this.target && isElementFocused(this.__getTargetFocusable()) && this.__shouldRestoreFocus) {
727754
this.__shouldRestoreFocus = false;
728755
return;
729756
}
730757

731-
// Move focus back to the target on overlay content Shift + Tab
732-
if (this.target && isElementFocused(overlayPart)) {
758+
// Move focus back to the target on popover Shift + Tab
759+
if (this.target && isElementFocused(this)) {
733760
event.preventDefault();
734761
this.__getTargetFocusable().focus();
735762
return;
@@ -738,7 +765,7 @@ class Popover extends PopoverPositionMixin(
738765
// Move focus back to the popover on next element Shift + Tab
739766
const nextFocusable = this.__getNextBodyFocusable(this.__getTargetFocusable());
740767
if (nextFocusable && isElementFocused(nextFocusable)) {
741-
const lastFocusable = this.__getLastFocusable(overlayPart);
768+
const lastFocusable = this.__getLastFocusable(this);
742769
if (lastFocusable) {
743770
event.preventDefault();
744771
lastFocusable.focus();
@@ -834,7 +861,7 @@ class Popover extends PopoverPositionMixin(
834861
}
835862

836863
/** @private */
837-
__onOverlayFocusIn() {
864+
__onFocusIn() {
838865
this.__focusInside = true;
839866

840867
// When using Tab to move focus, restoring focus is reset. However, if pressing Tab
@@ -845,7 +872,7 @@ class Popover extends PopoverPositionMixin(
845872
}
846873

847874
/** @private */
848-
__onOverlayFocusOut(event) {
875+
__onFocusOut(event) {
849876
// Do not close the popover on overlay focusout if it's not the last one.
850877
// This covers the following cases of nested overlay based components:
851878
// 1. Moving focus to the nested overlay (e.g. vaadin-select, vaadin-menu-bar)
@@ -857,7 +884,6 @@ class Popover extends PopoverPositionMixin(
857884
if (
858885
(this.__hasTrigger('focus') && this.__mouseDownInside) ||
859886
event.relatedTarget === this.target ||
860-
event.relatedTarget === this._overlayElement ||
861887
this.contains(event.relatedTarget)
862888
) {
863889
return;
@@ -946,7 +972,7 @@ class Popover extends PopoverPositionMixin(
946972
/** @private */
947973
__onOverlayOpened() {
948974
if (this.autofocus && !this.modal) {
949-
this._overlayElement.$.overlay.focus();
975+
this.focus();
950976
}
951977
}
952978

‎packages/popover/test/a11y.test.js

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
11
import { expect } from '@vaadin/chai-plugins';
22
import { sendKeys } from '@vaadin/test-runner-commands';
3-
import {
4-
esc,
5-
fixtureSync,
6-
focusout,
7-
nextRender,
8-
nextUpdate,
9-
oneEvent,
10-
outsideClick,
11-
tab,
12-
} from '@vaadin/testing-helpers';
3+
import { fixtureSync, focusout, nextRender, nextUpdate, oneEvent, outsideClick, tab } from '@vaadin/testing-helpers';
134
import sinon from 'sinon';
145
import './not-animated-styles.js';
156
import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
@@ -226,16 +217,16 @@ describe('a11y', () => {
226217
let spy;
227218

228219
beforeEach(() => {
229-
spy = sinon.spy(overlay.$.overlay, 'focus');
220+
spy = sinon.spy(popover, 'focus');
230221
});
231222

232-
it('should not move focus to the overlay content when opened by default', async () => {
223+
it('should not focus the popover when opened by default', async () => {
233224
target.click();
234225
await oneEvent(overlay, 'vaadin-overlay-open');
235226
expect(spy).to.not.be.called;
236227
});
237228

238-
it('should move focus to the overlay content when opened if autofocus is true', async () => {
229+
it('should focus the popover when opened if autofocus is true', async () => {
239230
popover.autofocus = true;
240231
target.click();
241232
await oneEvent(overlay, 'vaadin-overlay-open');
@@ -255,31 +246,31 @@ describe('a11y', () => {
255246

256247
it('should restore focus on Esc with trigger set to focus', async () => {
257248
const focusSpy = sinon.spy(target, 'focus');
258-
overlay.$.overlay.focus();
259-
esc(overlay.$.overlay);
249+
popover.focus();
250+
await sendKeys({ press: 'Escape' });
260251
await nextRender();
261252

262253
expect(focusSpy).to.be.calledOnce;
263254
});
264255

265256
it('should not re-open when restoring focus on Esc with trigger set to focus', async () => {
266-
overlay.$.overlay.focus();
267-
esc(overlay.$.overlay);
257+
popover.focus();
258+
await sendKeys({ press: 'Escape' });
268259
await nextRender();
269260

270261
expect(popover.opened).to.be.false;
271262
});
272263

273264
it('should not re-open when restoring focus on outside click with trigger set to focus', async () => {
274-
overlay.$.overlay.focus();
265+
popover.focus();
275266
outsideClick();
276267
await nextRender();
277268

278269
expect(popover.opened).to.be.false;
279270
});
280271

281272
it('should re-open when re-focusing after closing on outside click with trigger set to focus', async () => {
282-
overlay.$.overlay.focus();
273+
popover.focus();
283274
outsideClick();
284275
await nextRender();
285276

@@ -302,8 +293,8 @@ describe('a11y', () => {
302293

303294
it('should restore focus on Esc with trigger set to click', async () => {
304295
const focusSpy = sinon.spy(target, 'focus');
305-
overlay.$.overlay.focus();
306-
esc(overlay.$.overlay);
296+
popover.focus();
297+
await sendKeys({ press: 'Escape' });
307298
await nextRender();
308299

309300
expect(focusSpy).to.be.calledOnce;
@@ -321,8 +312,8 @@ describe('a11y', () => {
321312
const focusSpy = sinon.spy(target, 'focus');
322313
tab(target);
323314
focusout(target, overlay);
324-
overlay.$.overlay.focus();
325-
esc(overlay.$.overlay);
315+
popover.focus();
316+
await sendKeys({ press: 'Escape' });
326317
await nextRender();
327318

328319
expect(focusSpy).to.be.calledOnce;
@@ -340,8 +331,8 @@ describe('a11y', () => {
340331
await oneEvent(overlay, 'vaadin-overlay-open');
341332

342333
const focusSpy = sinon.spy(target, 'focus');
343-
overlay.$.overlay.focus();
344-
esc(overlay.$.overlay);
334+
popover.focus();
335+
await sendKeys({ press: 'Escape' });
345336
await nextRender();
346337

347338
expect(focusSpy).to.not.be.called;
@@ -389,8 +380,8 @@ describe('a11y', () => {
389380
await nextRender();
390381

391382
const focusSpy = sinon.spy(target, 'focus');
392-
overlay.$.overlay.focus();
393-
esc(overlay.$.overlay);
383+
popover.focus();
384+
await sendKeys({ press: 'Escape' });
394385
await nextRender();
395386

396387
expect(focusSpy).to.not.be.called;
@@ -400,8 +391,8 @@ describe('a11y', () => {
400391
mouseenter(target);
401392

402393
const focusSpy = sinon.spy(target, 'focus');
403-
overlay.$.overlay.focus();
404-
esc(overlay.$.overlay);
394+
popover.focus();
395+
await sendKeys({ press: 'Escape' });
405396
await nextRender();
406397

407398
expect(focusSpy).to.be.calledOnce;
@@ -432,10 +423,10 @@ describe('a11y', () => {
432423
await oneEvent(overlay, 'vaadin-overlay-open');
433424
});
434425

435-
it('should focus the overlay content part on target Tab', async () => {
426+
it('should focus the popover on target Tab', async () => {
436427
target.focus();
437428

438-
const spy = sinon.spy(overlay.$.overlay, 'focus');
429+
const spy = sinon.spy(popover, 'focus');
439430
await sendKeys({ press: 'Tab' });
440431

441432
expect(spy).to.be.calledOnce;
@@ -483,13 +474,13 @@ describe('a11y', () => {
483474
expect(spy).to.be.calledOnce;
484475
});
485476

486-
it('should not focus the overlay part on the next element Tab', async () => {
477+
it('should not focus the popover on the next element Tab', async () => {
487478
input.focus();
488479

489480
await sendKeys({ press: 'Tab' });
490481

491482
const activeElement = getDeepActiveElement();
492-
expect(activeElement).to.not.equal(overlay.$.overlay);
483+
expect(activeElement).to.not.equal(popover);
493484
});
494485

495486
it('should focus previous element on target Shift Tab while opened', async () => {
@@ -543,18 +534,18 @@ describe('a11y', () => {
543534
expect(activeElement).to.equal(input);
544535
});
545536

546-
it('should focus the overlay content part on focusable content Shift Tab', async () => {
537+
it('should focus the popover on focusable content Shift Tab', async () => {
547538
// Move focus to the overlay
548539
await sendKeys({ press: 'Tab' });
549540

550541
// Move focus to the input inside the overlay
551542
await sendKeys({ press: 'Tab' });
552543

553-
// Move focus back to the overlay part
544+
// Move focus back to the popover
554545
await sendKeys({ press: 'Shift+Tab' });
555546

556547
const activeElement = getDeepActiveElement();
557-
expect(activeElement).to.equal(overlay.$.overlay);
548+
expect(activeElement).to.equal(popover);
558549
});
559550
});
560551
});

‎packages/popover/test/basic.test.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,15 @@ describe('popover', () => {
3030
});
3131

3232
describe('host element', () => {
33-
it('should set display: contents on the host element by default', () => {
34-
expect(getComputedStyle(popover).display).to.equal('contents');
33+
it('should use display: none when not opened', () => {
34+
expect(getComputedStyle(popover).display).to.equal('none');
35+
});
36+
37+
['opened', 'opening', 'closing'].forEach((state) => {
38+
it(`should use display: block when ${state} attribute is set`, () => {
39+
popover.setAttribute(state, '');
40+
expect(getComputedStyle(popover).display).to.equal('block');
41+
});
3542
});
3643

3744
it('should reflect opened property to attribute', async () => {
@@ -480,7 +487,7 @@ describe('popover', () => {
480487
await nextUpdate(popover);
481488

482489
target.click();
483-
await nextRender();
490+
await oneEvent(overlay, 'vaadin-overlay-open');
484491
});
485492

486493
it('should set pointer-events on backdrop to none when non modal', () => {

0 commit comments

Comments
 (0)