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

igx-carousel accessibility improvements #8864

Merged
merged 21 commits into from
Feb 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8948f1b
feat(IgxCarousel): enhance accessibility #8202
ddincheva Jan 26, 2021
8914fc5
feat(IgxCarousel): enhance aria attributes in the slide component #8202
ddincheva Jan 26, 2021
afc5cd7
chore(*): add up more resource string to carousel component
ddincheva Jan 26, 2021
f66b471
Merge branch 'master' of https://github.com/IgniteUI/igniteui-angular…
ddincheva Jan 26, 2021
98ef669
Merge branch 'master' of https://github.com/IgniteUI/igniteui-angular…
ddincheva Jan 27, 2021
1a234c2
fix(IgxSlideComponnet): update aria attributes correctly #8202
ddincheva Jan 28, 2021
4922db6
fix(IgxCarousel): add correct aria attributes #8202
ddincheva Jan 28, 2021
7044b7b
test(Carousel): add basic aria tests #8202
ddincheva Jan 28, 2021
5bb6492
Merge branch 'master' of https://github.com/IgniteUI/igniteui-angular…
ddincheva Jan 28, 2021
768d6e3
chore(*): fix linting errors
ddincheva Jan 28, 2021
f510895
chore(*): fix linting error
ddincheva Jan 28, 2021
e50eb02
Merge branch 'master' into ddincheva/carouselAria
kdinev Jan 29, 2021
b29bca0
chore(IgxCarousel): address lint warnings
ddincheva Jan 29, 2021
9b9a8e9
fix(IgxCarousel): auto rotation is turned on-the live region is disab…
ddincheva Jan 29, 2021
f5d3907
chore(*): addressing requested changes
ddincheva Feb 1, 2021
93391e8
chore(*): addressing requested changes
ddincheva Feb 1, 2021
c3bc83d
Merge branch 'master' into ddincheva/carouselAria
zdrawku Feb 1, 2021
a469ba2
chore(Igxcarousel): labelledby is set only if the label is visible
ddincheva Feb 1, 2021
65dbd99
chore(*): removing forgotten semicolon
ddincheva Feb 1, 2021
5e5cd3c
chore(Slide): remove aria-selected attribute
ddincheva Feb 1, 2021
e5bdd72
Merge branch 'ddincheva/carouselAria' of https://github.com/IgniteUI/…
ddincheva Feb 1, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,32 @@
</a>
</ng-template>


<div *ngIf="showIndicators" [ngClass]="indicatorsOrientationClass">
<div *ngIf="showIndicators" [ngClass]="indicatorsOrientationClass" [attr.role]="'tablist'">
<div *ngFor="let slide of slides"
class="igx-carousel-indicators__indicator"
(click)="select(slide)"
[attr.aria-label]="setAriaLabel(slide)"
[id]="'tab-'+ slide.index + '-' + total"
[attr.role]="'tab'"
[attr.aria-label]="resourceStrings.igx_carousel_slide + ' ' + (slide.index + 1) + ' ' + resourceStrings.igx_carousel_of + ' ' + this.total"
[attr.aria-controls]="'panel-' + slide.index"
[attr.aria-selected]="slide.active">
<ng-container *ngTemplateOutlet="getIndicatorTemplate; context: {$implicit: slide};"></ng-container>
</div>
</div>

<div *ngIf="showIndicatorsLabel" [ngClass]="indicatorsOrientationClass">
<span class="igx-carousel__label">{{getCarouselLabel}}</span>
<span [id]="labelId" class="igx-carousel__label">{{getCarouselLabel}}</span>
</div>

<div class="igx-carousel__inner" role="list">
<div class="igx-carousel__inner" [attr.aria-live]="!interval || stoppedByInteraction ? 'polite' : 'off'">
<ng-content></ng-content>
</div>

<div *ngIf="navigation && slides.length" role="button" tabindex="0" class="igx-carousel__arrow--prev" (click)="prev()">
<div *ngIf="navigation && slides.length" role="button" tabindex="0" class="igx-carousel__arrow--prev" [attr.aria-label]="resourceStrings.igx_carousel_previous_slide" (keydown.enter)="prev()" (click)="prev()">
<ng-container *ngTemplateOutlet="getPrevButtonTemplate; context: {$implicit: prevButtonDisabled};"></ng-container>
</div>

<div *ngIf="navigation && slides.length" role="button" tabindex="0" class="igx-carousel__arrow--next" (click)="next()">
<div *ngIf="navigation && slides.length" role="button" tabindex="0" class="igx-carousel__arrow--next" [attr.aria-label]="resourceStrings.igx_carousel_next_slide" (keydown.enter)="next()" (click)="next()">
<ng-container *ngTemplateOutlet="getNextButtonTemplate; context: {$implicit: nextButtonDisabled};"></ng-container>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,41 @@ describe('Carousel', () => {
});


it('should apply correctly aria attributes to carousel component', () => {
const expectedRole = 'region';
const expectedRoleDescription = 'carousel';
const tabIndex = carousel.nativeElement.getAttribute('tabindex');

expect(tabIndex).toBeNull();
expect(carousel.nativeElement.getAttribute('role')).toEqual(expectedRole);
expect(carousel.nativeElement.getAttribute('aria-roledescription')).toEqual(expectedRoleDescription);

const indicators = carousel.nativeElement.querySelector(HelperTestFunctions.INDICATORS_BOTTOM_CLASS);

expect(indicators).toBeDefined();
expect(indicators.getAttribute('role')).toEqual('tablist');

const tabs = carousel.nativeElement.querySelectorAll('[role="tab"]');
expect(tabs.length).toEqual(4);
});

it('should apply correctly aria attributes to slide components', () => {
carousel.loop = false;
carousel.select(carousel.get(1));
fixture.detectChanges();

const expectedRole = 'tabpanel';
const slide = carousel.slides.find(s => s.active);
const tabIndex = slide.nativeElement.getAttribute('tabindex');

expect(+tabIndex).toBe(0);
expect(slide.nativeElement.getAttribute('role')).toEqual(expectedRole);

const tabs = carousel.nativeElement.querySelectorAll('[role="tab"]');
const slides = carousel.nativeElement.querySelectorAll('[role="tabpanel"]');

expect(slides.length).toEqual(tabs.length);
});
});

describe('Templates Tests: ', () => {
Expand Down Expand Up @@ -887,7 +922,7 @@ class HelperTestFunctions {
deltaY: 0,
duration: 100,
velocity,
preventDefault: ( ( e: any ) => { })
preventDefault: ( () => { })
};

carouselElement.triggerEventHandler(event, panOptions);
Expand Down Expand Up @@ -1013,7 +1048,7 @@ class CarouselDynamicSlidesComponent {
this.addNewSlide();
}

addNewSlide() {
public addNewSlide() {
this.slides.push(
{ text: 'Slide 1', active: false },
{ text: 'Slide 2', active: false },
Expand Down
73 changes: 30 additions & 43 deletions projects/igniteui-angular/src/lib/carousel/carousel.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,6 @@ export class CarouselHammerConfig extends HammerGestureConfig {
})

export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
/**
* Returns the `role` attribute of the carousel.
* ```typescript
* let carouselRole = this.carousel.role;
* ```
*
* @memberof IgxCarouselComponent
*/
@HostBinding('attr.role') public role = 'region';

/**
* Sets the `id` of the carousel.
Expand All @@ -118,29 +109,24 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
@HostBinding('attr.id')
@Input()
public id = `igx-carousel-${NEXT_ID++}`;

/**
* Returns the `aria-label` of the carousel.
*
* ```typescript
* let carousel = this.carousel.ariaLabel;
* ```
*
*/
@HostBinding('attr.aria-label')
public ariaLabel = 'carousel';

/**
* Returns the `tabIndex` of the carousel component.
* Returns the `role` attribute of the carousel.
* ```typescript
* let tabIndex = this.carousel.tabIndex;
* let carouselRole = this.carousel.role;
* ```
*
* @memberof IgxCarouselComponent
*/
@HostBinding('attr.tabindex')
get tabIndex() {
return 0;
@HostBinding('attr.role') public role = 'region';

/** @hidden */
@HostBinding('attr.aria-roledescription')
public roleDescription = 'carousel';

/** @hidden */
@HostBinding('attr.aria-labelledby')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's set this aria attr only when the label is visible - which is happening when the total slides are more than the maximumIndicatorsCount (5)

public get labelId() {
return this.showIndicatorsLabel ? `${this.id}-label` : null;
}

/**
Expand All @@ -161,7 +147,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
* ```
*/
@HostBinding('style.touch-action')
get touchAction() {
public get touchAction() {
return this.gesturesSupport ? 'pan-y' : 'auto';
}

Expand Down Expand Up @@ -398,11 +384,15 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
@ViewChild('defaultPrevButton', { read: TemplateRef, static: true })
private defaultPrevButton: TemplateRef<any>;

/**
* @hidden
* @internal
*/
public stoppedByInteraction: boolean;
private _interval: number;
private _resourceStrings = CurrentResourceStrings.CarouselResStrings;
private lastInterval: any;
private playing: boolean;
private stoppedByInteraction: boolean;
private destroyed: boolean;
private destroy$ = new Subject<any>();
private differ: IterableDiffer<IgxSlideComponent> | null = null;
Expand All @@ -420,14 +410,14 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
* By default it uses EN resources.
*/
@Input()
set resourceStrings(value: ICarouselResourceStrings) {
public set resourceStrings(value: ICarouselResourceStrings) {
this._resourceStrings = Object.assign({}, this._resourceStrings, value);
}

/**
* An accessor that returns the resource strings.
*/
get resourceStrings(): ICarouselResourceStrings {
public get resourceStrings(): ICarouselResourceStrings {
return this._resourceStrings;
}

Expand Down Expand Up @@ -484,7 +474,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
* @memberOf IgxCarouselComponent
*/
public get total(): number {
return this.slides.length;
return this.slides?.length;
}

/**
Expand Down Expand Up @@ -530,7 +520,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
*
* @memberof IgxCarouselComponent
*/
get nativeElement(): any {
public get nativeElement(): any {
return this.element.nativeElement;
}

Expand All @@ -543,7 +533,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
* @memberof IgxCarouselComponent
*/
@Input()
get interval(): number {
public get interval(): number {
return this._interval;
}

Expand All @@ -556,7 +546,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
*
* @memberof IgxCarouselComponent
*/
set interval(value: number) {
public set interval(value: number) {
this._interval = +value;
this.restartInterval();
}
Expand All @@ -566,13 +556,14 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
this.differ = this.iterableDiffers.find([]).create(null);
}


/** @hidden */
@HostListener('keydown.arrowright', ['$event'])
public onKeydownArrowRight(event) {
if (this.keyboardSupport) {
event.preventDefault();
this.next();
requestAnimationFrame(() => this.nativeElement.focus());
requestAnimationFrame(() => this.slides.find(s => s.active).nativeElement.focus());
}
}

Expand All @@ -582,7 +573,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
if (this.keyboardSupport) {
event.preventDefault();
this.prev();
requestAnimationFrame(() => this.nativeElement.focus());
requestAnimationFrame(() => this.slides.find(s => s.active).nativeElement.focus());
}
}

Expand All @@ -608,7 +599,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
if (this.keyboardSupport && this.slides.length > 0) {
event.preventDefault();
this.slides.first.active = true;
requestAnimationFrame(() => this.nativeElement.focus());
requestAnimationFrame(() => this.slides.find(s => s.active).nativeElement.focus());
}
}

Expand All @@ -618,7 +609,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
if (this.keyboardSupport && this.slides.length > 0) {
event.preventDefault();
this.slides.last.active = true;
requestAnimationFrame(() => this.nativeElement.focus());
requestAnimationFrame(() => this.slides.find(s => s.active).nativeElement.focus());
}
}

Expand Down Expand Up @@ -713,11 +704,6 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
}
}

/** @hidden */
public setAriaLabel(slide) {
return `Item ${slide.index + 1} of ${this.total}`;
}

/**
* Returns the slide corresponding to the provided `index` or null.
* ```typescript
Expand Down Expand Up @@ -1104,6 +1090,7 @@ export class IgxCarouselComponent implements OnDestroy, AfterContentInit {
this.slides.reduce((any, c, ind) => c.index = ind, 0); // reset slides indexes
diff.forEachAddedItem((record: IterableChangeRecord<IgxSlideComponent>) => {
const slide = record.item;
slide.total = this.total;
this.onSlideAdded.emit({ carousel: this, slide });
if (slide.active) {
this.currentSlide = slide;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Directive, TemplateRef } from '@angular/core';
import { Directive } from '@angular/core';

@Directive({
selector: '[igxCarouselIndicator]'
Expand Down
45 changes: 23 additions & 22 deletions projects/igniteui-angular/src/lib/carousel/slide.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, OnDestroy, Input, HostBinding, Output, EventEmitter, ElementRef, ChangeDetectorRef } from '@angular/core';
import { Component, OnDestroy, Input, HostBinding, Output, EventEmitter, ElementRef, AfterContentChecked } from '@angular/core';
import { Subject } from 'rxjs';

export enum Direction { NONE, NEXT, PREV }
Expand All @@ -20,7 +20,7 @@ export enum Direction { NONE, NEXT, PREV }
templateUrl: 'slide.component.html'
})

export class IgxSlideComponent implements OnDestroy {
export class IgxSlideComponent implements AfterContentChecked, OnDestroy {
/**
* Gets/sets the `index` of the slide inside the carousel.
* ```html
Expand All @@ -45,6 +45,9 @@ export class IgxSlideComponent implements OnDestroy {
*/
@Input() public direction: Direction;

@Input()
public total: number;

/**
* Returns the `tabIndex` of the slide component.
* ```typescript
Expand All @@ -54,35 +57,28 @@ export class IgxSlideComponent implements OnDestroy {
* @memberof IgxSlideComponent
*/
@HostBinding('attr.tabindex')
get tabIndex() {
public get tabIndex() {
return this.active ? 0 : null;
}

/**
* Returns the `aria-selected` of the slide.
*
* ```typescript
* let slide = this.slide.ariaSelected;
* ```
*
* @hidden
*/
@HostBinding('attr.aria-selected')
public get ariaSelected(): boolean {
return this.active;
}
@HostBinding('attr.id')
public id: string;

/**
* Returns the `aria-live` of the slide.
*
* ```typescript
* let slide = this.slide.ariaLive;
* ```
* Returns the `role` of the slide component.
* By default is set to `tabpanel`
*
* @memberof IgxSlideComponent
*/
@HostBinding('attr.aria-selected')
public get ariaLive() {
return this.active ? 'polite' : null;
}
@HostBinding('attr.role')
public tab = 'tabpanel';

/** @hidden */
@HostBinding('attr.aria-labelledby')
public ariaLabelledBy;

/**
* Returns the class of the slide component.
Expand Down Expand Up @@ -155,6 +151,11 @@ export class IgxSlideComponent implements OnDestroy {
return this._destroy$;
}

public ngAfterContentChecked() {
this.id = `panel-${this.index}`;
this.ariaLabelledBy = `tab-${this.index}-${this.total}`;
}

/**
* @hidden
*/
Expand Down
Loading