Skip to content

Commit

Permalink
fix(vertical-nav): remove aria-label from vertical nav group buttons
Browse files Browse the repository at this point in the history
This `aria-label` is unnecessary because the vertical nav group button
has visible text or could have screen reader text. This is similar to
the accordion toggle button which does not have an `aria-label`.

Also, this `aria-label` was not always unique which can cause problems.

VPAT-14152

BREAKING CHANGE:
- The `clrVerticalNavGroupLabel` input has been removed.
- The `verticalNavGroupToggle` common string has been removed.
  • Loading branch information
kevinbuhmann committed Dec 19, 2022
1 parent c3cabf0 commit e6884d7
Show file tree
Hide file tree
Showing 16 changed files with 20 additions and 91 deletions.
2 changes: 0 additions & 2 deletions .storybook/stories/vertical-nav/vertical-nav-group.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const defaultStory: Story = args => ({
<div class="main-container">
<clr-vertical-nav [clrVerticalNavCollapsible]="true">
<clr-vertical-nav-group
[clrVerticalNavGroupLabel]="clrVerticalNavGroupLabel"
[clrVerticalNavGroupExpanded]="clrVerticalNavGroupExpanded"
(clrVerticalNavGroupExpandedChange)="clrVerticalNavGroupExpandedChange($event)"
>
Expand Down Expand Up @@ -56,7 +55,6 @@ const defaultParameters: Parameters = {
component: ClrVerticalNavGroup,
argTypes: {
// inputs
clrVerticalNavGroupLabel: { defaultValue: 'Toggle vertical navigation group', control: { type: 'text' } },
clrVerticalNavGroupExpanded: { defaultValue: false, control: { type: 'boolean' } },
// outputs
clrVerticalNavGroupExpandedChange: { control: { disable: true } },
Expand Down
6 changes: 1 addition & 5 deletions projects/angular/clarity.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,6 @@ export interface ClrCommonStrings {
timelineStepSuccess: string;
totalPages: string;
// (undocumented)
verticalNavGroupToggle: string;
// (undocumented)
verticalNavToggle: string;
warning: string;
wizardStepError: string;
Expand Down Expand Up @@ -4203,8 +4201,6 @@ export class ClrVerticalNavGroup implements AfterContentInit, OnDestroy {
// (undocumented)
expandGroup(): void;
// (undocumented)
groupLabel: string;
// (undocumented)
ngAfterContentInit(): void;
// (undocumented)
ngOnDestroy(): void;
Expand All @@ -4213,7 +4209,7 @@ export class ClrVerticalNavGroup implements AfterContentInit, OnDestroy {
// (undocumented)
set userExpandedInput(value: boolean | string);
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<ClrVerticalNavGroup, "clr-vertical-nav-group", never, { "groupLabel": "clrVerticalNavGroupLabel"; "userExpandedInput": "clrVerticalNavGroupExpanded"; }, { "expandedChange": "clrVerticalNavGroupExpandedChange"; }, never, ["[clrVerticalNavLink]", "[clrVerticalNavIcon]", "*", "[clrIfExpanded], clr-vertical-nav-group-children"], false, never>;
static ɵcmp: i0.ɵɵComponentDeclaration<ClrVerticalNavGroup, "clr-vertical-nav-group", never, { "userExpandedInput": "clrVerticalNavGroupExpanded"; }, { "expandedChange": "clrVerticalNavGroupExpandedChange"; }, never, ["[clrVerticalNavLink]", "[clrVerticalNavIcon]", "*", "[clrIfExpanded], clr-vertical-nav-group-children"], false, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<ClrVerticalNavGroup, never>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,7 @@

<div class="nav-group-content">
<ng-content select="[clrVerticalNavLink]"></ng-content>
<button
class="nav-group-trigger"
type="button"
[attr.aria-expanded]="expanded"
[attr.aria-label]="groupLabel"
(click)="toggleExpand()"
>
<button class="nav-group-trigger" type="button" [attr.aria-expanded]="expanded" (click)="toggleExpand()">
<ng-content select="[clrVerticalNavIcon]"></ng-content>
<div class="nav-group-text">
<ng-content></ng-content>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,36 +150,6 @@ export default function (): void {
});
});

describe('Nav Group Internals with clrVerticalNavGroupLabel', () => {
let navGroup: ClrVerticalNavGroup;
let toggleBtn: HTMLButtonElement;

beforeEach(() => {
fixture = TestBed.createComponent(IfExpandedTestComponent);
fixture.detectChanges();
compiled = fixture.nativeElement;
navGroup = fixture.componentInstance.navGroup;
toggleBtn = compiled.querySelector('.nav-group-trigger');
});

afterEach(() => {
fixture.destroy();
});

it('defaults aria-label of nav group toggle button to common strings', () => {
expect(toggleBtn.hasAttribute('aria-label')).toBe(true);
expect(toggleBtn.getAttribute('aria-label')).toBe(navGroup.commonStrings.keys.verticalNavGroupToggle);
});

it('overrides default aria-label if clrVerticalNavGroup is set', fakeAsync(function () {
navGroup.groupLabel = 'ohai';
fixture.detectChanges();
tick();
expect(toggleBtn.hasAttribute('aria-label')).toBe(true);
expect(toggleBtn.getAttribute('aria-label')).toBe('ohai');
}));
});

describe('Template API', () => {
let navGroup: ClrVerticalNavGroup;
let expandService: IfExpandService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ export class ClrVerticalNavGroup implements AfterContentInit, OnDestroy {
}
}

@Input('clrVerticalNavGroupLabel')
groupLabel = this.commonStrings.keys.verticalNavGroupToggle;

@Input('clrVerticalNavGroupExpanded')
set userExpandedInput(value: boolean | string) {
value = !!value;
Expand Down
1 change: 0 additions & 1 deletion projects/angular/src/utils/i18n/common-strings.default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ export const commonStringsDefault: ClrCommonStrings = {
responsiveNavOverflowOpen: 'Navigation overflow menu',
responsiveNavOverflowClose: 'Navigation overflow menu',
//Vertical Nav
verticalNavGroupToggle: 'Toggle vertical navigation group',
verticalNavToggle: 'Toggle vertical navigation',
// Timeline steps
timelineStepNotStarted: 'Not started',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,6 @@ export interface ClrCommonStrings {
responsiveNavOverflowClose: string;
// Vertical Nav
verticalNavToggle: string;
verticalNavGroupToggle: string;
/**
* Timeline Steps
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</header>
<div class="content-container">
<clr-vertical-nav>
<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./beatles']"
routerLinkActive="active"
Expand All @@ -45,7 +45,7 @@
</ng-template>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./killers']"
routerLinkActive="active"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@
</a>

<ng-container *ngIf="item['children']">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<a
href="javascript://"
clrVerticalNavLink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ <h4>With Nav Group Text</h4>
<div class="content-container">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}
<clr-vertical-nav-group-children *clrIfExpanded>
Expand Down Expand Up @@ -84,10 +81,7 @@ <h4>With Nav Group Links</h4>
<div class="content-container">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<a href="javascript://" clrVerticalNavLink>
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ <h4>With Nav Group Text</h4>
<div class="content-container nested-menus-text">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}

Expand Down Expand Up @@ -85,10 +82,7 @@ <h4>With Nav Group Links</h4>
<div class="content-container nested-menus-links">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<a href="javascript://" clrVerticalNavLink>
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@
<div class="content-container partial-nested-icon-menu">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}
<clr-vertical-nav-group-children *clrIfExpanded>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@
<div class="content-container partial-nested-menu">
<clr-vertical-nav [clrVerticalNavCollapsible]="true" [(clrVerticalNavCollapsed)]="collapse">
<ng-container *ngFor="let item of case.items">
<clr-vertical-nav-group
*ngIf="item['children']"
[clrVerticalNavGroupLabel]="'Toggle ' + item.label.toLowerCase() + ' nav group'"
>
<clr-vertical-nav-group *ngIf="item['children']">
<cds-icon [attr.shape]="item.icon" *ngIf="item['icon']" clrVerticalNavIcon></cds-icon>
{{item.label}}
<clr-vertical-nav-group-children *clrIfExpanded>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
[clrVerticalNavCollapsed]="navCollapsed"
(clrVerticalNavCollapsedChange)="updateNavCollapsed($event)"
>
<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./beatles']"
routerLinkActive="active"
Expand All @@ -50,7 +50,7 @@
</ng-template>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./killers']"
routerLinkActive="active"
Expand Down Expand Up @@ -81,7 +81,7 @@
[clrVerticalNavCollapsed]="navCollapsed"
(clrVerticalNavCollapsedChange)="updateNavCollapsed($event)"
>
<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<cds-icon shape="home" clrVerticalNavIcon></cds-icon>
The Beatles
<ng-template [clrIfExpanded]="groupExpand" (clrIfExpandedChange)="updateGroupExpand($event)">
Expand All @@ -95,7 +95,7 @@
</ng-template>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<cds-icon shape="home" clrVerticalNavIcon></cds-icon>
The Killers
<ng-template clrIfExpanded>
Expand All @@ -120,7 +120,7 @@
[clrVerticalNavCollapsed]="navCollapsed"
(clrVerticalNavCollapsedChange)="updateNavCollapsed($event)"
>
<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a hidden aria-hidden="true" [routerLink]="['./beatles']"></a>
<cds-icon shape="home" clrVerticalNavIcon></cds-icon>
The Beatles
Expand All @@ -135,7 +135,7 @@
</ng-template>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a hidden aria-hidden="true" [routerLink]="['./killers']"></a>
<cds-icon shape="home" clrVerticalNavIcon></cds-icon>
The Killers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
[clrVerticalNavCollapsed]="navCollapsed"
(clrVerticalNavCollapsedChange)="updateNavCollapsed($event)"
>
<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./beatles']"
routerLinkActive="active"
Expand All @@ -50,7 +50,7 @@
</ng-template>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./killers']"
routerLinkActive="active"
Expand Down Expand Up @@ -85,7 +85,6 @@
routerLinkActive="active"
[clrVerticalNavGroupExpanded]="groupExpand"
(clrVerticalNavGroupExpandedChange)="updateGroupExpand($event)"
[clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'"
>
<a
[routerLink]="['./beatles']"
Expand All @@ -103,7 +102,7 @@
</clr-vertical-nav-group-children>
</clr-vertical-nav-group>

<clr-vertical-nav-group routerLinkActive="active" [clrVerticalNavGroupLabel]="'Toggle The Killers nav group'">
<clr-vertical-nav-group routerLinkActive="active">
<a
[routerLink]="['./killers']"
routerLinkActive="active"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
routerLinkActive="active"
[clrVerticalNavGroupExpanded]="groupExpand"
(clrVerticalNavGroupExpandedChange)="updateGroupExpand($event)"
[clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'"
>
<a
[routerLink]="['./beatles']"
Expand Down Expand Up @@ -86,7 +85,6 @@
routerLinkActive="active"
[clrVerticalNavGroupExpanded]="groupExpand"
(clrVerticalNavGroupExpandedChange)="updateGroupExpand($event)"
[clrVerticalNavGroupLabel]="'Toggle The Beatles nav group'"
>
<cds-icon shape="home" clrVerticalNavIcon></cds-icon>
The Beatles
Expand Down

0 comments on commit e6884d7

Please sign in to comment.