Skip to content

Commit

Permalink
fix(nav): put 'presentation' role on nav items
Browse files Browse the repository at this point in the history
For the non-ng-container-based markup a `role="presentation"` should be added on the `li` elements.

Fixes #4398
  • Loading branch information
maxokorokov committed Apr 3, 2023
1 parent 1fc07b6 commit e88c5c5
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export {
NgbNavContent,
NgbNavContentContext,
NgbNavItem,
NgbNavItemRole,
NgbNavLink,
NgbNavLinkButton,
NgbNavLinkBase,
Expand Down
5 changes: 4 additions & 1 deletion src/nav/nav.module.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { NgModule } from '@angular/core';

import { NgbNav, NgbNavContent, NgbNavItem, NgbNavLink, NgbNavLinkButton, NgbNavLinkBase } from './nav';
import { NgbNav, NgbNavContent, NgbNavItem, NgbNavItemRole, NgbNavLink, NgbNavLinkButton, NgbNavLinkBase } from './nav';

import { NgbNavOutlet, NgbNavPane } from './nav-outlet';

export {
NgbNav,
NgbNavContent,
NgbNavContentContext,
NgbNavItem,
NgbNavItemRole,
NgbNavLink,
NgbNavLinkButton,
NgbNavLinkBase,
Expand All @@ -20,6 +22,7 @@ const NGB_NAV_DIRECTIVES = [
NgbNavContent,
NgbNav,
NgbNavItem,
NgbNavItemRole,
NgbNavLink,
NgbNavLinkButton,
NgbNavLinkBase,
Expand Down
13 changes: 8 additions & 5 deletions src/nav/nav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ describe('nav', () => {
`);

expect(getNav(fixture).getAttribute('role')).toBe('tablist');
expect(getItems(fixture)[0].getAttribute('role')).toBe('presentation');
expect(getLinks(fixture)[0].getAttribute('role')).toBe('tab');
expect(getContent(fixture).getAttribute('role')).toBe('tabpanel');
});
Expand All @@ -140,24 +141,26 @@ describe('nav', () => {
`);

expect(getNav(fixture).getAttribute('role')).toBeNull();
expect(getItems(fixture)[0].getAttribute('role')).toBeNull();
expect(getLinks(fixture)[0].getAttribute('role')).toBeNull();
expect(getContent(fixture).getAttribute('role')).toBeNull();
});

it(`should allow overriding any A11Y roles`, () => {
const fixture = createTestComponent(`
<ul ngbNav #n="ngbNav" role="list" class="nav-tabs">
<li ngbNavItem>
<button ngbNavLink role="alert"></button>
<li ngbNavItem role="myItemRole">
<button ngbNavLink role="myLinkRole"></button>
<ng-template ngbTabContent></ng-template>
</li>
</ul>
<div [ngbNavOutlet]="n" paneRole="myRole"></div>
<div [ngbNavOutlet]="n" paneRole="myPaneRole"></div>
`);

expect(getNav(fixture).getAttribute('role')).toBe('list');
expect(getLinks(fixture)[0].getAttribute('role')).toBe('alert');
expect(getContent(fixture).getAttribute('role')).toBe('myRole');
expect(getItems(fixture)[0].getAttribute('role')).toBe('myItemRole');
expect(getLinks(fixture)[0].getAttribute('role')).toBe('myLinkRole');
expect(getContent(fixture).getAttribute('role')).toBe('myPaneRole');
});

it(`should set orientation CSS and 'aria-orientation' correctly`, () => {
Expand Down
14 changes: 14 additions & 0 deletions src/nav/nav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ export class NgbNavContent {
constructor(public templateRef: TemplateRef<any>) {}
}

/**
* This directive applies a specific role on a non-container based ngbNavItem.
*/
@Directive({
selector: '[ngbNavItem]:not(ng-container)',
standalone: true,
host: {
'[attr.role]': `role ? role : nav.roles ? 'presentation' : undefined`,
},
})
export class NgbNavItemRole {
constructor(@Attribute('role') public role: string, @Inject(forwardRef(() => NgbNav)) public nav: NgbNav) {}
}

/**
* The directive used to group nav link and related nav content. As well as set nav identifier and some options.
*
Expand Down

0 comments on commit e88c5c5

Please sign in to comment.