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

SideNav - A11y improvements #2536

Merged
merged 9 commits into from
Nov 5, 2024
Merged
7 changes: 7 additions & 0 deletions .changeset/modern-otters-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@hashicorp/design-system-components": patch
---

`SideNav` - Made a11y related improvements including:
- Changed `List::Title` to h3 & added visually hidden h2 to AppSideNav
- Replaced aria-label for `ToggleButton` with aria-labelledby and aria-expanded
2 changes: 2 additions & 0 deletions packages/components/src/components/hds/side-nav/base.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
}}
{{! IMPORTANT: we need to add "squishies" here (~) because otherwise the whitespace added by Ember causes the empty element to still have visible padding - See https://handlebarsjs.com/guide/expressions.html#whitespace-control }}
<div class="hds-side-nav" ...attributes>
<h2 class="sr-only" id="hds-side-nav-header">Application local navigation</h2>

<div class="hds-side-nav__wrapper">
{{yield to="root"}}
<div class="hds-side-nav__wrapper-header">
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/components/hds/side-nav/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
{{! template-lint-enable no-invalid-interactive}}
<Hds::SideNav::ToggleButton
aria-label={{this.ariaLabel}}
aria-labelledby="hds-side-nav-header"
aria-expanded={{if this.isMinimized "false" "true"}}
@icon={{if this.isMinimized "chevrons-right" "chevrons-left"}}
{{on "click" this.toggleMinimizedStatus}}
/>
Expand Down
34 changes: 25 additions & 9 deletions packages/components/src/components/hds/side-nav/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import Component from '@glimmer/component';
import { deprecate } from '@ember/debug';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { registerDestructor } from '@ember/destroyable';
Expand All @@ -21,7 +22,10 @@ export interface HdsSideNavSignature {
a11yRefocusNavigationText?: string;
a11yRefocusRouteChangeValidator?: string;
a11yRefocusExcludeAllQueryParams?: boolean;
ariaLabel?: string;
/**
* @deprecated The `@ariaLabel` argument for "Hds::SideNav" has been deprecated. It is replaced by aria-labelledby and aria-expanded on the toggle button
*/
ariaLabel?: string | undefined;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
onToggleMinimizedStatus?: (arg: boolean) => void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -73,6 +77,23 @@ export default class HdsSideNav extends Component<HdsSideNavSignature> {
registerDestructor(this, (): void => {
this.removeEventListeners();
});

if (args.ariaLabel !== undefined) {
deprecate(
'The `@ariaLabel` argument for "Hds::SideNav" has been deprecated. It is replaced by aria-labelledby and aria-expanded on the toggle button',
false,
{
id: 'hds.sidenav',
until: '5.0.0',
url: 'https://helios.hashicorp.design/components/sidenav?tab=version%20history#4140',
for: '@hashicorp/design-system-components',
since: {
available: '4.14.0',
enabled: '5.0.0',
},
}
);
}
}

addEventListeners(): void {
Expand Down Expand Up @@ -107,15 +128,10 @@ export default class HdsSideNav extends Component<HdsSideNavSignature> {
}

/**
* @param ariaLabel
* @type {string}
* @default 'close menu'
* @deprecated The `@ariaLabel` argument for "Hds::SideNav" has been deprecated. It is replaced by aria-labelledby and aria-expanded on the toggle button
*/
get ariaLabel(): string {
if (this.isMinimized) {
return this.args.ariaLabel ?? 'Open menu';
}
return this.args.ariaLabel ?? 'Close menu';
get ariaLabel(): string | undefined {
return this.args.ariaLabel;
}

get classNames(): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
SPDX-License-Identifier: MPL-2.0
}}

<nav class="hds-side-nav__list-wrapper" ...attributes>
<nav class="hds-side-nav__list-wrapper" aria-labelledby="hds-side-nav-header" ...attributes>
{{yield (hash ExtraBefore=(component "hds/yield"))}}
<ul class="hds-side-nav__list" role="list">
<ul class="hds-side-nav__list" role="list" aria-labelledby={{this.titleIds}}>
{{yield
(hash
Item=(component "hds/side-nav/list/item")
BackLink=(component "hds/side-nav/list/back-link")
Title=(component "hds/side-nav/list/title")
Title=(component "hds/side-nav/list/title" didInsertTitle=this.didInsertTitle)
Link=(component "hds/side-nav/list/link")
)
}}
Expand Down
18 changes: 14 additions & 4 deletions packages/components/src/components/hds/side-nav/list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* SPDX-License-Identifier: MPL-2.0
*/

import TemplateOnlyComponent from '@ember/component/template-only';

import Component from '@glimmer/component';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import type { ComponentLike } from '@glint/template';
import type { HdsYieldSignature } from '../../yield';
import type { HdsSideNavListItemSignature } from './item';
Expand All @@ -28,6 +29,15 @@ export interface HdsSideNavListSignature {
Element: HTMLElement;
}

const HdsSideNavList = TemplateOnlyComponent<HdsSideNavListSignature>();
export default class HdsSideNavList extends Component<HdsSideNavListSignature> {
@tracked _titleIds: string[] = [];

get titleIds(): string {
return this._titleIds.join(' ');
}

export default HdsSideNavList;
@action
didInsertTitle(titleId: string): void {
this._titleIds = [...this._titleIds, titleId];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,10 @@
}}

<Hds::SideNav::List::Item>
<div class="hds-side-nav__list-title hds-typography-body-100 hds-font-weight-semibold" ...attributes>{{~yield~}}</div>
<div
class="hds-side-nav__list-title hds-typography-body-100 hds-font-weight-semibold"
id={{this.titleId}}
{{did-insert this.didInsertTitle}}
...attributes
>{{~yield~}}</div>
</Hds::SideNav::List::Item>
22 changes: 18 additions & 4 deletions packages/components/src/components/hds/side-nav/list/title.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,30 @@
* SPDX-License-Identifier: MPL-2.0
*/

import TemplateOnlyComponent from '@ember/component/template-only';
import { guidFor } from '@ember/object/internals';
import { action } from '@ember/object';
import Component from '@glimmer/component';

export interface HdsSideNavListTitleSignature {
Args: {
didInsertTitle?: (titleId: string) => void;
};
Blocks: {
default: [];
};
Element: HTMLDivElement;
}

const HdsSideNavListTitle =
TemplateOnlyComponent<HdsSideNavListTitleSignature>();
export default class HdsSideNavListTitle extends Component<HdsSideNavListTitleSignature> {
/* Generate a unique ID for each Title */
titleId = 'title-' + guidFor(this);

@action
didInsertTitle(element: HTMLElement): void {
const { didInsertTitle } = this.args;

export default HdsSideNavListTitle;
if (typeof didInsertTitle === 'function') {
didInsertTitle(element.id);
}
}
}
4 changes: 4 additions & 0 deletions showcase/tests/acceptance/components/hds/side-nav-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ module('Acceptance | Component | hds/side-nav', function (hooks) {
enabled: false,
selectors: [['.shw-placeholder']],
},
'landmark-unique': {
enabled: false,
selectors: [['.hds-side-nav__list-wrapper']],
},
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ module('Integration | Component | hds/side-nav/index', function (hooks) {
assert.dom('#test-side-nav').hasClass('hds-side-nav--is-not-minimized');
assert
.dom('.hds-side-nav__toggle-button')
.hasAttribute('aria-label', 'Close menu');
.hasAttribute('aria-expanded', 'true');
assert
.dom('.hds-side-nav__toggle-button .hds-icon')
.hasClass('hds-icon-chevrons-left');
Expand All @@ -265,7 +265,7 @@ module('Integration | Component | hds/side-nav/index', function (hooks) {
assert.dom('#test-side-nav').hasClass('hds-side-nav--is-minimized');
assert
.dom('.hds-side-nav__toggle-button')
.hasAttribute('aria-label', 'Open menu');
.hasAttribute('aria-expanded', 'false');
assert
.dom('.hds-side-nav__toggle-button .hds-icon')
.hasClass('hds-icon-chevrons-right');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ This is the full-fledged component (responsive and animated).
<C.Property @name="isMinimized" @type="boolean" @default="false">
Controls if the App Side Nav is rendered collapsed or expanded when initialized. This allows an application to preserve the collapsed/expanded state across sessions. After the initial render, this argument is altered based on user interactions (collapse/expand the App Side Nav or resize the window) and it is not a suitable way of controlling the App Side Nav state from outside after render (it’s an internal state).
</C.Property>
<C.Property @name="toggleButtonAriaLabel" @type="string">
Accepts a localized string; the fallback is set to `Open menu` if the menu is closed, and `Close menu` if the menu is open.
</C.Property>
<C.Property @name="onToggleMinimizedStatus" @type="function">
Callback function invoked when the `AppSideNav` is collapsed or expanded. The function receives a boolean argument stating if the `AppSideNav` is minimized or not.
</C.Property>
Expand Down
1 change: 1 addition & 0 deletions website/docs/components/side-nav/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ navigation:
</section>

<section data-tab="Version history">
@include "partials/version-history/4.14.0.md"
@include "partials/version-history/4.10.0.md"
@include "partials/version-history/4.8.0.md"
</section>
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ This is the full-fledged component (responsive and animated).
</C.Property>
</Doc::ComponentApi>
</C.Property>
<C.Property @name="ariaLabel" @type="string">
Accepts a localized string; the fallback is set to `Open menu` if the menu is closed, and `Close menu` if the menu is open.
<C.Property @name="ariaLabel" @type="string" @deprecated={{true}}>
Accepts a localized string; the fallback is set to `Open menu` if the menu is closed, and `Close menu` if the menu is open.<br /><br />Note: The purpose of this argument has been replaced by an `aria-labelledby` attribute referencing the "hds-side-nav-header" `h2` combined with the use of `aria-expanded` on the Toggle button tab.
</C.Property>
<C.Property @name="onToggleMinimizedStatus" @type="function">
Callback function invoked when the `SideNav` is collapsed or expanded. The function receives a boolean argument stating if the `SideNav` is minimized or not.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## 4.14.0

### Updated

`SideNav`

- Made a11y related improvements including:
- Changed `List::Title` to h3 & added visually hidden h2 to AppSideNav
- Replaced aria-label for `ToggleButton` with aria-labelledby and aria-expanded
- Fixed bug with hidden panels sometimes causing unnecessary overflow scrolling.

### Deprecated

`SideNav`

Deprecated the `ariaLabel` argument as it is no longer needed. Its purpose is replaced by an added `aria-labelledby` attribute referencing the "hds-side-nav-header" `h2` combined with the use of `aria-expanded` on the Toggle button tab.