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

Improve accessibility by adding more aria support #5658

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/dropdown/dropdown-menu.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export type NzPlacementType = 'bottomLeft' | 'bottomCenter' | 'bottomRight' | 't
[@slideMotion]="'enter'"
[@.disabled]="noAnimation?.nzNoAnimation"
[nzNoAnimation]="noAnimation?.nzNoAnimation"
[attr.id]="id"
(mouseenter)="setMouseState(true)"
(mouseleave)="setMouseState(false)"
>
Expand All @@ -63,6 +64,7 @@ export class NzDropdownMenuComponent implements AfterContentInit {
descendantMenuItemClick$ = this.nzMenuService.descendantMenuItemClick$;
nzOverlayClassName: string = '';
nzOverlayStyle: IndexableObject = {};
id: string | null = null;
@ViewChild(TemplateRef, { static: true }) templateRef!: TemplateRef<NzSafeAny>;

setMouseState(visible: boolean): void {
Expand Down
30 changes: 26 additions & 4 deletions components/dropdown/dropdown.directive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,27 @@ describe('dropdown', () => {
fixture.detectChanges();
expect(fixture.componentInstance.triggerVisible).toHaveBeenCalledTimes(1);
}));
it('should aria states work', fakeAsync(() => {
const fixture = createComponent(NzTestDropdownComponent, [], []);
fixture.componentInstance.trigger = 'click';

fixture.detectChanges();
const dropdownElement = fixture.debugElement.query(By.directive(NzDropDownDirective)).nativeElement;

expect(dropdownElement.getAttribute('aria-haspopup')).toBe('menu');
expect(dropdownElement.getAttribute('aria-expanded')).toBe('false');
expect(dropdownElement.getAttribute('aria-owns')).toBe(null);

dispatchFakeEvent(dropdownElement, 'click');
tick(1000);
fixture.detectChanges();

const overlayMenu = overlayContainerElement.querySelector('.ant-dropdown')!;
expect(dropdownElement.getAttribute('aria-expanded')).toBe('true');
const id = dropdownElement.getAttribute('aria-owns');
expect(id).not.toBe(null);
expect(overlayMenu.getAttribute('id')).toBe(id);
}));
});

@Component({
Expand All @@ -210,7 +231,8 @@ describe('dropdown', () => {
[nzBackdrop]="backdrop"
[nzOverlayClassName]="className"
[nzOverlayStyle]="overlayStyle"
>Trigger
>
Trigger
</a>
<nz-dropdown-menu #menu="nzDropdownMenu">
<ul nz-menu>
Expand All @@ -232,9 +254,9 @@ export class NzTestDropdownComponent {

@Component({
template: `
<a nz-dropdown [nzDropdownMenu]="menu" [nzClickHide]="false" [(nzVisible)]="visible" (nzVisibleChange)="triggerVisible($event)"
>Hover me</a
>
<a nz-dropdown [nzDropdownMenu]="menu" [nzClickHide]="false" [(nzVisible)]="visible" (nzVisibleChange)="triggerVisible($event)">
Hover me
</a>
<nz-dropdown-menu #menu="nzDropdownMenu">
<ul nz-menu>
<li nz-menu-item class="first-menu">Clicking me will not close the menu.</li>
Expand Down
11 changes: 10 additions & 1 deletion components/dropdown/dropdown.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,16 @@ import { auditTime, distinctUntilChanged, filter, map, mapTo, switchMap, takeUnt
import { NzDropdownMenuComponent, NzPlacementType } from './dropdown-menu.component';

const listOfPositions = [POSITION_MAP.bottomLeft, POSITION_MAP.bottomRight, POSITION_MAP.topRight, POSITION_MAP.topLeft];
let dropdowns = 0;

@Directive({
selector: '[nz-dropdown]',
exportAs: 'nzDropdown',
host: {
'[class.ant-dropdown-trigger]': 'true'
'[class.ant-dropdown-trigger]': 'true',
'[attr.aria-owns]': 'nzVisible? dropdownId:null',
'[attr.aria-expanded]': 'nzVisible',
'aria-haspopup': 'menu'
}
})
export class NzDropDownDirective implements AfterViewInit, OnDestroy, OnChanges, OnInit {
Expand Down Expand Up @@ -65,6 +69,7 @@ export class NzDropDownDirective implements AfterViewInit, OnDestroy, OnChanges,
@Input() nzOverlayStyle: IndexableObject = {};
@Input() nzPlacement: NzPlacementType = 'bottomLeft';
@Output() readonly nzVisibleChange: EventEmitter<boolean> = new EventEmitter();
dropdownId: string | null = null;

setDropdownMenuValue<T extends keyof NzDropdownMenuComponent>(key: T, value: NzDropdownMenuComponent[T]): void {
if (this.nzDropdownMenu) {
Expand Down Expand Up @@ -134,6 +139,10 @@ export class NzDropDownDirective implements AfterViewInit, OnDestroy, OnChanges,
if (visible) {
/** set up overlayRef **/
if (!this.overlayRef) {
if (this.nzDropdownMenu) {
this.dropdownId = `dropdown-${dropdowns++}`;
this.nzDropdownMenu.id = this.dropdownId;
}
/** new overlay **/
this.overlayRef = this.overlay.create({
positionStrategy: this.positionStrategy,
Expand Down
6 changes: 6 additions & 0 deletions components/icon/icon.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,

ngOnInit(): void {
this.renderer.setAttribute(this.el, 'class', `anticon ${this.el.className}`.trim());
this.renderer.setAttribute(this.el, 'role', 'img');
}

/**
Expand All @@ -105,6 +106,7 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
* Replacement of `changeIcon` for more modifications.
*/
private changeIcon2(): void {
this.setAriaLabel();
this.setClassName();
this._changeIcon().then(svgOrRemove => {
if (svgOrRemove) {
Expand All @@ -131,6 +133,10 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
}
}

private setAriaLabel(): void {
this.renderer.setAttribute(this.el, 'aria-label', this.type as string);
}

private setClassName(): void {
if (this.cacheClassName) {
this.renderer.removeClass(this.el, this.cacheClassName);
Expand Down
15 changes: 15 additions & 0 deletions components/icon/icon.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ describe('nz-icon', () => {
expect(icons[0].nativeElement.classList.contains('anticon-question')).not.toBe(true);
});

it('should get icon aria label back', () => {
fixture.detectChanges();
expect(icons[0].nativeElement.getAttribute('role')).toBe('img');
expect(icons[0].nativeElement.getAttribute('aria-label')).toBe('question');
expect(icons[1].nativeElement.getAttribute('aria-label')).toBe('loading');
});

it('should change aria label when type changes', () => {
testComponent.type = 'question-circle';
fixture.detectChanges();
expect(icons[0].nativeElement.getAttribute('role')).toBe('img');
expect(icons[0].nativeElement.getAttribute('aria-label')).toBe('question-circle');
expect(icons[0].nativeElement.getAttribute('aria-label')).not.toBe('question');
});

it('should support spin and cancel', fakeAsync(() => {
fixture.detectChanges();
tick(1000);
Expand Down
14 changes: 14 additions & 0 deletions components/input-number/input-number.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,36 @@ import { InputBoolean, isNotNil } from 'ng-zorro-antd/core/util';
template: `
<div class="ant-input-number-handler-wrap">
<span
role="button"
aria-label="Increase Value"
unselectable="unselectable"
class="ant-input-number-handler ant-input-number-handler-up"
(mousedown)="up($event)"
(mouseup)="stop()"
(mouseleave)="stop()"
[class.ant-input-number-handler-up-disabled]="disabledUp"
[attr.aria-disabled]="nzDisabled || disabledUp ? true : null"
>
<i nz-icon nzType="up" class="ant-input-number-handler-up-inner"></i>
</span>
<span
role="button"
aria-label="Decrease Value"
unselectable="unselectable"
class="ant-input-number-handler ant-input-number-handler-down"
(mousedown)="down($event)"
(mouseup)="stop()"
(mouseleave)="stop()"
[class.ant-input-number-handler-down-disabled]="disabledDown"
[attr.aria-disabled]="nzDisabled || disabledDown ? true : null"
>
<i nz-icon nzType="down" class="ant-input-number-handler-down-inner"></i>
</span>
</div>
<div class="ant-input-number-input-wrap">
<input
#inputElement
role="spinbutton"
autocomplete="off"
class="ant-input-number-input"
[attr.id]="nzId"
Expand All @@ -66,6 +73,9 @@ import { InputBoolean, isNotNil } from 'ng-zorro-antd/core/util';
[placeholder]="nzPlaceHolder"
[attr.step]="nzStep"
[attr.inputmode]="nzInputMode"
[attr.aria-valuemin]="nzMin"
[attr.aria-valuemax]="nzMax"
[attr.aria-valuenow]="valuenow"
(keydown)="onKeyDown($event)"
(keyup)="stop()"
[ngModel]="displayValue"
Expand Down Expand Up @@ -318,6 +328,10 @@ export class NzInputNumberComponent implements ControlValueAccessor, AfterViewIn
}
}

public get valuenow(): number | null {
return this.value || this.value === 0 ? this.value : null;
}

updateDisplayValue(value: number): void {
const displayValue = isNotNil(this.nzFormatter(value)) ? this.nzFormatter(value) : '';
this.displayValue = displayValue;
Expand Down
66 changes: 64 additions & 2 deletions components/input-number/input-number.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,69 @@ describe('input number', () => {
fixture.detectChanges();
expect(inputNumber.nativeElement.classList).not.toContain('ant-input-number-focused');
});
it('should aria roles and labels work', () => {
fixture.detectChanges();
expect(inputElement.getAttribute('role')).toBe('spinbutton');
expect(upHandler.getAttribute('role')).toBe('button');
expect(downHandler.getAttribute('role')).toBe('button');
expect(upHandler.getAttribute('aria-label')).toBe('Increase Value');
expect(downHandler.getAttribute('aria-label')).toBe('Decrease Value');
});
it('should aria value attributes work', () => {
testComponent.min = 0;
testComponent.max = 10;

fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuemin')).toBe('0');
expect(inputElement.getAttribute('aria-valuemax')).toBe('10');
expect(inputElement.getAttribute('aria-valuenow')).toBe(null);

testComponent.nzInputNumberComponent.onModelChange('5');
fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuenow')).toBe('5');

dispatchFakeEvent(upHandler, 'mousedown');
fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuenow')).toBe('6');

dispatchFakeEvent(downHandler, 'mousedown');
fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuenow')).toBe('5');

testComponent.min = 3;
testComponent.max = 15;
fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuemin')).toBe('3');
expect(inputElement.getAttribute('aria-valuemax')).toBe('15');
expect(inputElement.getAttribute('aria-valuenow')).toBe('5');

testComponent.nzInputNumberComponent.onModelChange('');
fixture.detectChanges();
expect(inputElement.getAttribute('aria-valuenow')).toBe(null);
});
it('should aria disabled work', () => {
fixture.detectChanges();
expect(upHandler.getAttribute('aria-disabled')).not.toBe('true');
expect(downHandler.getAttribute('aria-disabled')).not.toBe('true');

testComponent.max = 1;
testComponent.nzInputNumberComponent.onModelChange('1');
fixture.detectChanges();
expect(upHandler.getAttribute('aria-disabled')).toBe('true');
expect(downHandler.getAttribute('aria-disabled')).not.toBe('true');

testComponent.min = -1;
testComponent.nzInputNumberComponent.onModelChange('-1');
fixture.detectChanges();
expect(upHandler.getAttribute('aria-disabled')).not.toBe('true');
expect(downHandler.getAttribute('aria-disabled')).toBe('true');

testComponent.nzInputNumberComponent.onModelChange('0');
testComponent.disabled = true;
fixture.detectChanges();
expect(upHandler.getAttribute('aria-disabled')).toBe('true');
expect(downHandler.getAttribute('aria-disabled')).toBe('true');
});
});

describe('input number form', () => {
Expand Down Expand Up @@ -454,8 +517,7 @@ describe('input number', () => {
[nzParser]="parser"
[nzPrecision]="precision"
[nzPrecisionMode]="precisionMode"
>
</nz-input-number>
></nz-input-number>
`
})
export class NzTestInputNumberBasicComponent {
Expand Down
4 changes: 3 additions & 1 deletion components/menu/menu-item.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import { NzSubmenuService } from './submenu.service';
'[class.ant-menu-item-selected]': `!isMenuInsideDropDown && nzSelected`,
'[class.ant-menu-item-disabled]': `!isMenuInsideDropDown && nzDisabled`,
'[style.paddingLeft.px]': 'nzPaddingLeft || inlinePaddingLeft',
'(click)': 'clickMenuItem($event)'
'(click)': 'clickMenuItem($event)',
'[attr.aria-disabled]': 'nzDisabled? true:null',
role: 'menuitem'
}
})
export class NzMenuItemDirective implements OnInit, OnChanges, OnDestroy, AfterContentInit {
Expand Down
9 changes: 7 additions & 2 deletions components/menu/menu.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import { NzIsMenuInsideDropDownToken, NzMenuServiceLocalToken } from './menu.tok
import { NzMenuModeType, NzMenuThemeType } from './menu.types';
import { NzSubMenuComponent } from './submenu.component';

let menus = 0;

export function MenuServiceFactory(serviceInsideDropDown: MenuService, serviceOutsideDropDown: MenuService): MenuService {
return serviceInsideDropDown ? serviceInsideDropDown : serviceOutsideDropDown;
}
Expand Down Expand Up @@ -74,7 +76,8 @@ export function MenuDropDownTokenFactory(isMenuInsideDropDownToken: boolean): bo
'[class.ant-menu-vertical]': `!isMenuInsideDropDown && actualMode === 'vertical'`,
'[class.ant-menu-horizontal]': `!isMenuInsideDropDown && actualMode === 'horizontal'`,
'[class.ant-menu-inline]': `!isMenuInsideDropDown && actualMode === 'inline'`,
'[class.ant-menu-inline-collapsed]': `!isMenuInsideDropDown && nzInlineCollapsed`
'[class.ant-menu-inline-collapsed]': `!isMenuInsideDropDown && nzInlineCollapsed`,
role: 'menu'
}
})
export class NzMenuDirective implements AfterContentInit, OnInit, OnChanges, OnDestroy {
Expand Down Expand Up @@ -116,7 +119,9 @@ export class NzMenuDirective implements AfterContentInit, OnInit, OnChanges, OnD
private nzMenuService: MenuService,
@Inject(NzIsMenuInsideDropDownToken) public isMenuInsideDropDown: boolean,
private cdr: ChangeDetectorRef
) {}
) {
nzMenuService.setMenuId(++menus);
}

ngOnInit(): void {
combineLatest([this.inlineCollapsed$, this.mode$])
Expand Down
8 changes: 8 additions & 0 deletions components/menu/menu.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export class MenuService {
mode$ = new BehaviorSubject<NzMenuModeType>('vertical');
inlineIndent$ = new BehaviorSubject<number>(24);
isChildSubMenuOpen$ = new BehaviorSubject<boolean>(false);
subMenus: number = 0;
menuId: number = 0;

onDescendantMenuItemClick(menu: NzSafeAny): void {
this.descendantMenuItemClick$.next(menu);
Expand All @@ -38,4 +40,10 @@ export class MenuService {
setInlineIndent(indent: number): void {
this.inlineIndent$.next(indent);
}
setMenuId(id: number): void {
this.menuId = id;
}
getNewSubmenuId(): number {
return ++this.subMenus;
}
}
Loading