Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Commit df00c2b

Browse files
allan-chencopybara-github
authored andcommitted
fix(select): set aria-expanded false earlier when menu closes
Fixes behavior on Talkback screenreader PiperOrigin-RevId: 353066311
1 parent 2ed2d82 commit df00c2b

File tree

11 files changed

+96
-35
lines changed

11 files changed

+96
-35
lines changed

packages/mdc-menu-surface/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ Method Signature | Description
178178
Event Name | Data | Description
179179
--- | --- | ---
180180
`MDCMenuSurface:closed` | none | Event emitted after the menu surface is closed.
181+
`MDCMenuSurface:closing` | none | Event emitted when the menu surface is closing, but animation may not have completed yet.
181182
`MDCMenuSurface:opened` | none | Event emitted after the menu surface is opened.
182183

183184
## Usage Within Frameworks

packages/mdc-menu-surface/adapter.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ export interface MDCMenuSurfaceAdapter {
5858
/** Emits an event when the menu surface is closed. */
5959
notifyClose(): void;
6060

61+
/** Emits an event when the menu surface is closing. */
62+
notifyClosing(): void;
63+
6164
/** Emits an event when the menu surface is opened. */
6265
notifyOpen(): void;
6366
}

packages/mdc-menu-surface/component.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ export class MDCMenuSurface extends MDCComponent<MDCMenuSurfaceFoundation> {
155155
hasAnchor: () => !!this.anchorElement,
156156
notifyClose: () =>
157157
this.emit(MDCMenuSurfaceFoundation.strings.CLOSED_EVENT, {}),
158+
notifyClosing: () => {
159+
this.emit(MDCMenuSurfaceFoundation.strings.CLOSING_EVENT, {});
160+
},
158161
notifyOpen: () =>
159162
this.emit(MDCMenuSurfaceFoundation.strings.OPENED_EVENT, {}),
160163
isElementInContainer: (el) => this.root.contains(el),

packages/mdc-menu-surface/constants.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,15 @@ const cssClasses = {
3434
// tslint:disable:object-literal-sort-keys
3535
const strings = {
3636
CLOSED_EVENT: 'MDCMenuSurface:closed',
37+
CLOSING_EVENT: 'MDCMenuSurface:closing',
3738
OPENED_EVENT: 'MDCMenuSurface:opened',
3839
FOCUSABLE_ELEMENTS: [
39-
'button:not(:disabled)', '[href]:not([aria-disabled="true"])', 'input:not(:disabled)',
40-
'select:not(:disabled)', 'textarea:not(:disabled)', '[tabindex]:not([tabindex="-1"]):not([aria-disabled="true"])',
40+
'button:not(:disabled)',
41+
'[href]:not([aria-disabled="true"])',
42+
'input:not(:disabled)',
43+
'select:not(:disabled)',
44+
'textarea:not(:disabled)',
45+
'[tabindex]:not([tabindex="-1"]):not([aria-disabled="true"])',
4146
].join(', '),
4247
};
4348
// tslint:enable:object-literal-sort-keys

packages/mdc-menu-surface/foundation.ts

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte
8181

8282
notifyClose: () => undefined,
8383
notifyOpen: () => undefined,
84+
notifyClosing: () => undefined,
8485
};
8586
// tslint:enable:object-literal-sort-keys
8687
}
@@ -231,6 +232,8 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte
231232
return;
232233
}
233234

235+
this.adapter.notifyClosing();
236+
234237
if (this.isQuickOpen) {
235238
this.isSurfaceOpen = false;
236239
if (!skipRestoreFocus) {
@@ -242,25 +245,25 @@ export class MDCMenuSurfaceFoundation extends MDCFoundation<MDCMenuSurfaceAdapte
242245
MDCMenuSurfaceFoundation.cssClasses.IS_OPEN_BELOW);
243246
this.adapter.notifyClose();
244247

245-
} else {
246-
this.adapter.addClass(
247-
MDCMenuSurfaceFoundation.cssClasses.ANIMATING_CLOSED);
248-
requestAnimationFrame(() => {
249-
this.adapter.removeClass(MDCMenuSurfaceFoundation.cssClasses.OPEN);
250-
this.adapter.removeClass(
251-
MDCMenuSurfaceFoundation.cssClasses.IS_OPEN_BELOW);
252-
this.closeAnimationEndTimerId = setTimeout(() => {
253-
this.closeAnimationEndTimerId = 0;
254-
this.adapter.removeClass(
255-
MDCMenuSurfaceFoundation.cssClasses.ANIMATING_CLOSED);
256-
this.adapter.notifyClose();
257-
}, numbers.TRANSITION_CLOSE_DURATION);
258-
});
248+
return;
249+
}
259250

260-
this.isSurfaceOpen = false;
261-
if (!skipRestoreFocus) {
262-
this.maybeRestoreFocus();
263-
}
251+
this.adapter.addClass(MDCMenuSurfaceFoundation.cssClasses.ANIMATING_CLOSED);
252+
requestAnimationFrame(() => {
253+
this.adapter.removeClass(MDCMenuSurfaceFoundation.cssClasses.OPEN);
254+
this.adapter.removeClass(
255+
MDCMenuSurfaceFoundation.cssClasses.IS_OPEN_BELOW);
256+
this.closeAnimationEndTimerId = setTimeout(() => {
257+
this.closeAnimationEndTimerId = 0;
258+
this.adapter.removeClass(
259+
MDCMenuSurfaceFoundation.cssClasses.ANIMATING_CLOSED);
260+
this.adapter.notifyClose();
261+
}, numbers.TRANSITION_CLOSE_DURATION);
262+
});
263+
264+
this.isSurfaceOpen = false;
265+
if (!skipRestoreFocus) {
266+
this.maybeRestoreFocus();
264267
}
265268
}
266269

packages/mdc-menu-surface/test/component.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,15 @@ describe('MDCMenuSurface', () => {
299299
expect(handler).toHaveBeenCalled();
300300
});
301301

302+
it(`adapter#notifyClosing fires an ${strings.CLOSING_EVENT} custom event`,
303+
() => {
304+
const {root, component} = setupTest();
305+
const handler = jasmine.createSpy('notifyClosing handler');
306+
root.addEventListener(strings.CLOSING_EVENT, handler);
307+
(component.getDefaultFoundation() as any).adapter.notifyClosing();
308+
expect(handler).toHaveBeenCalled();
309+
});
310+
302311
it(`adapter#notifyOpen fires an ${strings.OPENED_EVENT} custom event`, () => {
303312
const {root, component} = setupTest();
304313
const handler = jasmine.createSpy('notifyOpen handler');

packages/mdc-menu-surface/test/foundation.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ describe('MDCMenuSurfaceFoundation', () => {
195195
'hasClass',
196196
'hasAnchor',
197197
'notifyClose',
198+
'notifyClosing',
198199
'notifyOpen',
199200
'isElementInContainer',
200201
'isRtl',
@@ -1006,6 +1007,14 @@ describe('MDCMenuSurfaceFoundation', () => {
10061007
expect(mockAdapter.notifyClose).toHaveBeenCalled();
10071008
});
10081009

1010+
testFoundation(
1011+
'#close emits the closing event immediately',
1012+
({foundation, mockAdapter}) => {
1013+
(foundation as unknown as WithIsSurfaceOpen).isSurfaceOpen = true;
1014+
foundation.close();
1015+
expect(mockAdapter.notifyClosing).toHaveBeenCalled();
1016+
});
1017+
10091018
testFoundation(
10101019
'#close emits the close event when quickOpen is true',
10111020
({foundation, mockAdapter}) => {

packages/mdc-select/component.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,18 +62,14 @@ export class MDCSelect extends MDCComponent<MDCSelectFoundation> {
6262
private outline!: MDCNotchedOutline|null; // assigned in initialize()
6363

6464
// Event handlers
65-
private handleFocus!:
66-
SpecificEventListener<'focus'>; // assigned in initialize()
67-
private handleBlur!:
68-
SpecificEventListener<'blur'>; // assigned in initialize()
69-
private handleClick!:
70-
SpecificEventListener<'click'>; // assigned in initialize()
71-
private handleKeydown!:
72-
SpecificEventListener<'keydown'>; // assigned in initialize()
73-
private handleMenuOpened!: EventListener; // assigned in initialize()
74-
private handleMenuClosed!: EventListener; // assigned in initialize()
75-
private handleMenuItemAction!:
76-
CustomEventListener<MDCMenuItemEvent>; // assigned in initialize()
65+
private handleFocus!: SpecificEventListener<'focus'>;
66+
private handleBlur!: SpecificEventListener<'blur'>;
67+
private handleClick!: SpecificEventListener<'click'>;
68+
private handleKeydown!: SpecificEventListener<'keydown'>;
69+
private handleMenuOpened!: EventListener;
70+
private handleMenuClosed!: EventListener;
71+
private handleMenuClosing!: EventListener;
72+
private handleMenuItemAction!: CustomEventListener<MDCMenuItemEvent>;
7773

7874
initialize(
7975
labelFactory: MDCFloatingLabelFactory = (el) => new MDCFloatingLabel(el),
@@ -155,6 +151,9 @@ export class MDCSelect extends MDCComponent<MDCSelectFoundation> {
155151
this.handleMenuClosed = () => {
156152
this.foundation.handleMenuClosed();
157153
};
154+
this.handleMenuClosing = () => {
155+
this.foundation.handleMenuClosing();
156+
};
158157

159158
this.selectAnchor.addEventListener('focus', this.handleFocus);
160159
this.selectAnchor.addEventListener('blur', this.handleBlur);
@@ -165,6 +164,8 @@ export class MDCSelect extends MDCComponent<MDCSelectFoundation> {
165164
this.selectAnchor.addEventListener('keydown', this.handleKeydown);
166165
this.menu.listen(
167166
menuSurfaceConstants.strings.CLOSED_EVENT, this.handleMenuClosed);
167+
this.menu.listen(
168+
menuSurfaceConstants.strings.CLOSING_EVENT, this.handleMenuClosing);
168169
this.menu.listen(
169170
menuSurfaceConstants.strings.OPENED_EVENT, this.handleMenuOpened);
170171
this.menu.listen(

packages/mdc-select/foundation.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,13 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
243243
this.adapter.focusMenuItemAtIndex(focusItemIndex);
244244
}
245245

246+
handleMenuClosing() {
247+
this.adapter.setSelectAnchorAttr('aria-expanded', 'false');
248+
}
249+
246250
handleMenuClosed() {
247251
this.adapter.removeClass(cssClasses.ACTIVATED);
248252
this.isMenuOpen = false;
249-
this.adapter.setSelectAnchorAttr('aria-expanded', 'false');
250253

251254
// Unfocus the select if menu is closed without a selection
252255
if (!this.adapter.isSelectAnchorFocused()) {

packages/mdc-select/test/component.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,30 @@ describe('MDCSelect', () => {
14391439
document.body.removeChild(fixture);
14401440
});
14411441

1442+
it('menu surface closed event sets aria-expanded to false', () => {
1443+
const hasMockFoundation = true;
1444+
const hasMockMenu = false;
1445+
const hasOutline = false;
1446+
const hasLabel = true;
1447+
const {fixture, menuSurface, mockFoundation} =
1448+
setupTest(hasOutline, hasLabel, hasMockFoundation, hasMockMenu);
1449+
document.body.appendChild(fixture);
1450+
const evtType = MDCMenuSurfaceFoundation.strings.CLOSING_EVENT;
1451+
let evt;
1452+
if (typeof CustomEvent === 'function') {
1453+
evt = new CustomEvent(evtType, {
1454+
bubbles: false,
1455+
});
1456+
} else {
1457+
evt = document.createEvent('CustomEvent');
1458+
evt.initCustomEvent(evtType, false, false, null);
1459+
}
1460+
menuSurface.dispatchEvent(evt);
1461+
expect(mockFoundation.handleMenuClosing).toHaveBeenCalledTimes(1);
1462+
1463+
document.body.removeChild(fixture);
1464+
});
1465+
14421466
it('#constructor instantiates a leading icon if an icon element is present',
14431467
() => {
14441468
const root = getFixture();

packages/mdc-select/test/foundation.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,9 @@ describe('MDCSelectFoundation', () => {
227227
expect((foundation as any).isMenuOpen).toBe(false);
228228
});
229229

230-
it('#handleMenuClosed set aria-expanded attribute to false', () => {
230+
it('#handleMenuClosing set aria-expanded attribute to false', () => {
231231
const {foundation, mockAdapter} = setupTest();
232-
foundation.handleMenuClosed();
232+
foundation.handleMenuClosing();
233233
expect(mockAdapter.setSelectAnchorAttr)
234234
.toHaveBeenCalledWith('aria-expanded', 'false');
235235
});

0 commit comments

Comments
 (0)