Skip to content

Commit 83561d7

Browse files
fix(navigation-primary): hamburger standalones toggle state (#2462)
* fix(navigation-primary): toggle state and keyboard navigation correctly closing hamburger menu * test(navigation-primary): improve testing around compact standalone toggle states * chore(navigation-primary): add changeset * refactor(navigation-primary): focus and menu states * fix(navigation-primary): only close hamburger * fix(navigation-primary): dont close hamburger unless compact * fix(navigation-primary): resolve flickr on click vs keyboard nav * fix(navigation-primary): revert removing the exception on closePrimaryDropdowns it is used * docs: update elements/rh-navigation-primary/rh-navigation-primary-item.ts * docs: update .changeset/heavy-hornets-throw.md * docs: update elements/rh-navigation-primary/rh-navigation-primary-item.ts --------- Co-authored-by: Benny Powers - עם ישראל חי! <[email protected]>
1 parent 706278b commit 83561d7

File tree

4 files changed

+135
-102
lines changed

4 files changed

+135
-102
lines changed

.changeset/heavy-hornets-throw.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@rhds/elements": patch
3+
---
4+
5+
`<rh-navigation-primary>`: improved keyboard accessibility of the dropdown toggle
6+

elements/rh-navigation-primary/rh-navigation-primary-item.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export class RhNavigationPrimaryItem extends LitElement {
3636
@query('details')
3737
private _details!: HTMLDetailsElement;
3838

39+
@query('summary')
40+
private _summary!: HTMLElement;
41+
42+
@consume({ context, subscribe: true })
43+
@state()
44+
private compact?: boolean;
45+
3946
@property({ type: Boolean, reflect: true }) open = false;
4047

4148
/* Summary text for dropdown variants only */
@@ -57,9 +64,6 @@ export class RhNavigationPrimaryItem extends LitElement {
5764
/** Icon set for the `icon` property - 'ui' by default */
5865
@property({ attribute: 'icon-set' }) iconSet?: IconSetName;
5966

60-
@consume({ context, subscribe: true })
61-
@state()
62-
private compact?: boolean;
6367

6468
protected firstUpdated(): void {
6569
// ensure we update initially on client hydration
@@ -95,7 +99,7 @@ export class RhNavigationPrimaryItem extends LitElement {
9599
hamburger: hamburger,
96100
dehydrated: !this.#hydrated,
97101
})}">${this.variant === 'dropdown' ? html`
98-
<details @toggle="${this.#detailsToggle}">
102+
<details @toggle="${this.#detailsToggle}" ?open="${this.open}">
99103
<summary>${hamburger ? '' : html`
100104
<slot name="icon">${!this.icon ? '' : html`
101105
<rh-icon icon="${ifDefined(this.icon)}" set="${ifDefined(this.iconSet)}"></rh-icon>`}
@@ -117,12 +121,18 @@ export class RhNavigationPrimaryItem extends LitElement {
117121
this.dispatchEvent(new Event('toggle', { bubbles: true }));
118122
}
119123

120-
public hide() {
121-
this._details.open = false;
124+
/** @summary hides the dropdown */
125+
public async hide() {
126+
this.open = false;
127+
this.requestUpdate();
128+
await this.updateComplete;
122129
}
123130

124-
public show() {
125-
this._details.open = true;
131+
/** @summary shows the dropdown */
132+
public async show() {
133+
this.open = true;
134+
this.requestUpdate();
135+
await this.updateComplete;
126136
}
127137
}
128138

elements/rh-navigation-primary/rh-navigation-primary.ts

Lines changed: 63 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ export class RhNavigationPrimary extends LitElement {
8989
@query('#hamburger')
9090
private _hamburger!: HTMLDetailsElement;
9191

92+
@query('summary')
93+
private _hamburgerSummary!: HTMLElement;
94+
9295
/**
9396
* Sets the mobile toggle (hamburger) text, used for translations, defaults to 'Menu'
9497
*/
@@ -178,15 +181,16 @@ export class RhNavigationPrimary extends LitElement {
178181
</a>
179182
</slot>
180183
</div>
181-
<details id="hamburger" ?open="${this._hamburgerOpen}" @toggle="${this.#hamburgerToggle}">
182-
<summary>
184+
<details id="hamburger" ?open="${this._hamburgerOpen}" @toggle="${this.#hamburgerToggle}" @focusout="${this.#onHamburgerFocusOut}">
185+
<summary @blur="${this.#onHamburgerSummaryBlur}">
183186
<rh-icon icon="menu-bars" set="ui"></rh-icon>
184187
<div id="summary">${this.mobileToggleLabel}</div>
185188
<rh-icon icon="caret-down" set="microns"></rh-icon>
186189
</summary>
187-
<div id="details-content" role="list">
190+
<div id="details-content" role="list" >
188191
<slot></slot>
189192
</div>
193+
190194
</details>
191195
<div id="secondary">
192196
<div id="event" role="list"><slot name="event"></slot></div>
@@ -228,22 +232,23 @@ export class RhNavigationPrimary extends LitElement {
228232
this.#closeOverlay();
229233
}
230234

231-
#primaryDropdowns(): RhNavigationPrimaryItem[] {
235+
#primaryItems(): RhNavigationPrimaryItem[] {
232236
return Array.from(
233237
this.querySelectorAll(
234-
'rh-navigation-primary-item[variant="dropdown"]:not([slot="dropdowns"])',
238+
'rh-navigation-primary-item:not([slot])',
235239
)
236240
);
237241
}
238242

239-
#secondaryDropdowns(): RhNavigationPrimaryItem[] {
243+
#openDropdownItems(): RhNavigationPrimaryItem[] {
240244
return Array.from(
241245
this.querySelectorAll(
242-
'rh-navigation-primary-item[variant="dropdown"][slot="dropdowns"]',
246+
'rh-navigation-primary-item[variant="dropdown"][open]',
243247
)
244248
);
245249
}
246250

251+
247252
async #onDropdownToggle(event: Event) {
248253
const item = event.target as RhNavigationPrimaryItem;
249254
// if the event came from a secondary link in a compact mode we'll want to close the hamburger first if it is open
@@ -282,6 +287,46 @@ export class RhNavigationPrimary extends LitElement {
282287
}
283288
}
284289

290+
#hamburgerContains(item: Node): boolean {
291+
const primaryItems = this.#primaryItems();
292+
return primaryItems.some(pi => pi.contains(item));
293+
}
294+
295+
#onHamburgerSummaryBlur(event: FocusEvent) {
296+
if (event.relatedTarget) {
297+
if (this.#hamburgerContains(event.relatedTarget as Node)) {
298+
return;
299+
}
300+
if (this.compact) {
301+
this.#closeHamburger();
302+
}
303+
}
304+
}
305+
306+
#onHamburgerFocusOut(event: FocusEvent) {
307+
if (event.relatedTarget) {
308+
if (event.relatedTarget === this._hamburgerSummary) {
309+
return;
310+
}
311+
if (this.#hamburgerContains(event.relatedTarget as Node)) {
312+
return;
313+
}
314+
if (this.compact) {
315+
this.#closeHamburger();
316+
}
317+
}
318+
}
319+
320+
async #onFocusout(event: FocusEvent) {
321+
const target = event.relatedTarget as HTMLElement;
322+
if (target?.closest('rh-navigation-primary') === this || target === null) {
323+
// if the focus is still inside the rh-navigation-secondary exit
324+
return;
325+
} else {
326+
this.close();
327+
}
328+
}
329+
285330
#onKeydown(event: KeyboardEvent) {
286331
switch (event.key) {
287332
case 'Escape': {
@@ -299,102 +344,29 @@ export class RhNavigationPrimary extends LitElement {
299344
}
300345
break;
301346
}
302-
case 'Tab':
303-
this.#onTabKeydown(event);
304-
break;
305347
default:
306348
break;
307349
}
308350
}
309351

310352
#onKeyup(event: KeyboardEvent) {
311353
switch (event.key) {
312-
case 'Tab':
313-
this.#onTabKeyup(event);
314-
break;
315-
default:
354+
case 'Tab': {
355+
this.#onTabUp(event);
316356
break;
317-
}
318-
}
319-
320-
321-
async #onFocusout(event: FocusEvent) {
322-
const target = event.relatedTarget as HTMLElement;
323-
if (target?.closest('rh-navigation-primary') === this || target === null) {
324-
// if the focus is still inside the rh-navigation-secondary exit
325-
return;
326-
} else {
327-
this.close();
328-
}
329-
}
330-
331-
#onTabKeydown(event: KeyboardEvent) {
332-
// target is the element we are leaving with tab press
333-
const target = event.target as HTMLElement;
334-
// get target parent dropdown
335-
const primaryDropdowns = this.#primaryDropdowns();
336-
const secondaryDropdowns = this.#secondaryDropdowns();
337-
// target can be in one of the two dropdown collections, but only 1.
338-
const dropdownContainsTarget =
339-
primaryDropdowns.find(dropdown => dropdown.contains(target))
340-
?? secondaryDropdowns.find(dropdown => dropdown.contains(target));
341-
if (dropdownContainsTarget) {
342-
const focusableChildElements =
343-
Array.from(RhNavigationPrimary.focusableChildElements(dropdownContainsTarget));
344-
345-
if (focusableChildElements.length > 0) {
346-
const {
347-
0: firstChild,
348-
[focusableChildElements.length - 1]: lastChild,
349-
} = focusableChildElements;
350-
351-
if (event.shiftKey) {
352-
if (event.shiftKey && firstChild === target) {
353-
return;
354-
}
355-
// if target is self, close self
356-
if (event.shiftKey && target === dropdownContainsTarget) {
357-
dropdownContainsTarget.hide();
358-
return;
359-
}
360-
} else {
361-
if (!firstChild) {
362-
return;
363-
}
364-
if (!lastChild) {
365-
return;
366-
} else {
367-
if (lastChild === target) {
368-
dropdownContainsTarget.hide();
369-
return;
370-
}
371-
}
372-
}
373-
} else {
374-
this.#closePrimaryDropdowns();
375-
this.#closeSecondaryDropdowns();
376357
}
377358
}
378359
}
379360

380-
#onTabKeyup(event: KeyboardEvent) {
381-
if (this.compact && this._hamburgerOpen) {
382-
const secondaryDropdowns = this.#secondaryDropdowns();
383-
const target = event.target as HTMLElement;
384-
if (event.shiftKey && target === this) {
385-
this.#closeHamburger();
386-
} else {
387-
if (secondaryDropdowns.some(dropdown => dropdown.contains(target))) {
388-
this.#closeHamburger();
389-
}
390-
}
361+
#onTabUp(event: KeyboardEvent) {
362+
// target is the element we are entering with tab up press
363+
const target = event.target as HTMLElement;
364+
if (!this.#openDropdownItems().some(item => item.contains(target))) {
365+
this.#closePrimaryDropdowns();
366+
this.#closeSecondaryDropdowns();
391367
}
392368
}
393369

394-
/**
395-
* close all open dropdowns in primary slot
396-
* @param except
397-
*/
398370
#closePrimaryDropdowns(except?: RhNavigationPrimaryItem) {
399371
this.#openPrimaryDropdowns.forEach((dropdown: RhNavigationPrimaryItem) => {
400372
if (dropdown !== except) {
@@ -431,14 +403,16 @@ export class RhNavigationPrimary extends LitElement {
431403
#hamburgerToggle(event: Event) {
432404
if (event instanceof ToggleEvent) {
433405
if (event.newState === 'open') {
434-
// if we are compact mode and any secondary link dropdowns are open, close them
406+
this.#openHamburger();
407+
// if we are compact mode and any secondary link dropdowns are open, close them
435408
if (this.compact && this.#openSecondaryDropdowns.size > 0) {
436409
this.#closeSecondaryDropdowns();
437410
}
438411
if (this.compact) {
439412
this.#openOverlay();
440413
}
441414
} else {
415+
this.#closeHamburger();
442416
if (this.#openPrimaryDropdowns.size > 0) {
443417
this.#closePrimaryDropdowns();
444418
}

elements/rh-navigation-primary/test/rh-navigation-primary.spec.ts

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,25 @@ const [
2828
const NAV = html`
2929
<rh-navigation-primary>
3030
<rh-navigation-primary-item variant="dropdown" summary="${HamburgerItem1Name}">
31-
Item 1 Content
31+
${HamburgerItem1Name} Content
3232
</rh-navigation-primary-item>
3333
3434
<rh-navigation-primary-item variant="dropdown" summary="${HamburgerItem2Name}">
35-
Item 2 Content
35+
${HamburgerItem2Name} Content
3636
</rh-navigation-primary-item>
3737
3838
<rh-navigation-primary-item variant="dropdown" summary="${HamburgerItem3Name}">
39-
Item 3 Content
39+
${HamburgerItem3Name} Content
4040
</rh-navigation-primary-item>
4141
4242
<rh-navigation-primary-item variant="dropdown">
4343
<span slot="summary">${HamburgerItem4Name}</span>
44-
Item 4 Content
44+
${HamburgerItem4Name} Content
4545
</rh-navigation-primary-item>
4646
4747
<rh-navigation-primary-item variant="dropdown">
4848
<span slot="summary">${HamburgerItem5Name}</span>
49-
Item 5 Content
49+
${HamburgerItem5Name} Content
5050
</rh-navigation-primary-item>
5151
5252
<rh-navigation-primary-item slot="event">
@@ -185,6 +185,11 @@ describe('<rh-navigation-primary>', function() {
185185
expect(snapshot).to.have.axQuery({ name: 'Menu', expanded: true });
186186
});
187187

188+
it('menu should have focus', async function() {
189+
const snapshot = await a11ySnapshot();
190+
expect(snapshot).to.have.axQuery({ name: 'Menu', focused: true });
191+
});
192+
188193
it('should not hide content behind hamburger menu', async function() {
189194
const snapshot = await a11ySnapshot();
190195
expect(snapshot).to.have.axQuery({ name: HamburgerItem1Name });
@@ -200,6 +205,44 @@ describe('<rh-navigation-primary>', function() {
200205
expect(snapshot).to.have.axQuery({ name: 'Item 7' });
201206
expect(snapshot).to.have.axQuery({ name: 'Item 8' });
202207
});
208+
209+
describe('toggling a standalone dropdown', function() {
210+
beforeEach(async function() {
211+
/**
212+
* We normally would not test by query and click but by tabbing to the standalone dropdown
213+
* we are triggering the hamburger menu to close on keyup this is expected behavior
214+
* so we need to click the standalone dropdown to open it which should close the hamburger menu
215+
* the overlay should still remain open
216+
*/
217+
const item6 = element.querySelector('rh-navigation-primary-item[summary="Item 6"]');
218+
const details = item6?.shadowRoot?.querySelector('details');
219+
const summary = details?.querySelector('summary');
220+
summary?.click();
221+
});
222+
beforeEach(async () => await element.updateComplete);
223+
224+
it('should open the standalone dropdown', async function() {
225+
const snapshot = await a11ySnapshot();
226+
expect(snapshot).to.have.axQuery({ name: 'Item 6', expanded: true });
227+
expect(snapshot).to.have.axQuery({ role: 'text', name: 'Item 6 Content' });
228+
});
229+
230+
it('should close the hamburger menu', async function() {
231+
const snapshot = await a11ySnapshot();
232+
expect(snapshot).to.not.have.axQuery({ name: 'Menu', expanded: false });
233+
expect(snapshot).to.not.have.axQuery({ name: HamburgerItem1Name });
234+
expect(snapshot).to.not.have.axQuery({ name: HamburgerItem2Name });
235+
expect(snapshot).to.not.have.axQuery({ name: HamburgerItem3Name });
236+
expect(snapshot).to.not.have.axQuery({ name: HamburgerItem4Name });
237+
expect(snapshot).to.not.have.axQuery({ name: HamburgerItem5Name });
238+
});
239+
240+
it('should still have an open overlay', async function() {
241+
// query the shadow root for the overlay
242+
const overlay = element.shadowRoot?.querySelector('rh-navigation-primary-overlay[open]');
243+
expect(overlay).to.exist;
244+
});
245+
});
203246
});
204247

205248
describe('toggle closed', function() {

0 commit comments

Comments
 (0)