feat(pagination): improve accessibility

Remove the `nav` element and add the `role="navigation"` attribute to the host. `aria-label` is not needed anymore as it can be put on the host. Add invisible `(current)` text for screen readers. Closes #1294
ng-bootstrap · Mar 27, 2017 · 424c38f · 424c38f
1 parent 5005384
commit 424c38f
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 41 deletions.
diff --git a/demo/src/app/components/pagination/demos/basic/pagination-basic.html b/demo/src/app/components/pagination/demos/basic/pagination-basic.html
@@ -1,5 +1,5 @@
 <div>Default pagination</div>
-<ngb-pagination [collectionSize]="70" [(page)]="page"></ngb-pagination>
+<ngb-pagination [collectionSize]="70" [(page)]="page" aria-label="Default pagination"></ngb-pagination>
 
 <div>directionLinks = false</div>
 <ngb-pagination [collectionSize]="70" [(page)]="page" [directionLinks]="false"></ngb-pagination>

diff --git a/src/pagination/pagination.spec.ts b/src/pagination/pagination.spec.ts
@@ -22,7 +22,7 @@ function expectPages(nativeEl: HTMLElement, pagesDef: string[]): void {
     if (classIndicator === '+') {
       expect(pages[i]).toHaveCssClass('active');
       expect(pages[i]).not.toHaveCssClass('disabled');
-      expect(normalizeText(pages[i].textContent)).toEqual(pageDef.substr(1));
+      expect(normalizeText(pages[i].textContent)).toEqual(pageDef.substr(1) + ' (current)');
     } else if (classIndicator === '-') {
       expect(pages[i]).not.toHaveCssClass('active');
       expect(pages[i]).toHaveCssClass('disabled');
@@ -49,6 +49,10 @@ function getList(nativeEl: HTMLElement) {
   return <HTMLUListElement>nativeEl.querySelector('ul');
 }
 
+function getSpan(nativeEl: HTMLElement): HTMLSpanElement {
+  return <HTMLSpanElement>nativeEl.querySelector('span');
+}
+
 function normalizeText(txt: string): string {
   return txt.trim().replace(/\s+/g, ' ');
 }
@@ -539,7 +543,7 @@ describe('ngb-pagination', () => {
     });
 
     it('should not emit "pageChange" for incorrect input values', fakeAsync(() => {
-         const html = `<ngb-pagination [collectionSize]="collectionSize" [pageSize]="pageSize" [maxSize]="maxSize" 
+         const html = `<ngb-pagination [collectionSize]="collectionSize" [pageSize]="pageSize" [maxSize]="maxSize"
         (pageChange)="onPageChange($event)"></ngb-pagination>`;
          const fixture = createTestComponent(html);
          tick();
@@ -561,7 +565,7 @@ describe('ngb-pagination', () => {
          expect(fixture.componentInstance.onPageChange).not.toHaveBeenCalled();
        }));
     it('should set classes correctly for disabled state', fakeAsync(() => {
-         const html = `<ngb-pagination [collectionSize]="collectionSize" [pageSize]="pageSize" [maxSize]="maxSize" 
+         const html = `<ngb-pagination [collectionSize]="collectionSize" [pageSize]="pageSize" [maxSize]="maxSize"
          [disabled]=true></ngb-pagination>`;
          const fixture = createTestComponent(html);
          tick();

diff --git a/src/pagination/pagination.ts b/src/pagination/pagination.ts
@@ -8,44 +8,46 @@ import {NgbPaginationConfig} from './pagination-config';
 @Component({
   selector: 'ngb-pagination',
   changeDetection: ChangeDetectionStrategy.OnPush,
+  host: {'role': 'navigation'},
   template: `
-    <nav>
-      <ul [class]="'pagination' + (size ? ' pagination-' + size : '')">
-        <li *ngIf="boundaryLinks" class="page-item" 
-          [class.disabled]="!hasPrevious() || disabled">
-          <a aria-label="First" class="page-link" href (click)="!!selectPage(1)" [attr.tabindex]="hasPrevious() ? null : '-1'">
-            <span aria-hidden="true">&laquo;&laquo;</span>
-            <span class="sr-only">First</span>
-          </a>                
-        </li>
-      
-        <li *ngIf="directionLinks" class="page-item" 
-          [class.disabled]="!hasPrevious() || disabled">
-          <a aria-label="Previous" class="page-link" href (click)="!!selectPage(page-1)" [attr.tabindex]="hasPrevious() ? null : '-1'">
-            <span aria-hidden="true">&laquo;</span>
-            <span class="sr-only">Previous</span>
-          </a>
-        </li>
-        <li *ngFor="let pageNumber of pages" class="page-item" [class.active]="pageNumber === page" 
-          [class.disabled]="isEllipsis(pageNumber) || disabled">
-          <a *ngIf="isEllipsis(pageNumber)" class="page-link">...</a>
-          <a *ngIf="!isEllipsis(pageNumber)" class="page-link" href (click)="!!selectPage(pageNumber)">{{pageNumber}}</a>
-        </li>
-        <li *ngIf="directionLinks" class="page-item" [class.disabled]="!hasNext() || disabled">
-          <a aria-label="Next" class="page-link" href (click)="!!selectPage(page+1)" [attr.tabindex]="hasNext() ? null : '-1'">
-            <span aria-hidden="true">&raquo;</span>
-            <span class="sr-only">Next</span>
-          </a>
-        </li>
-        
-        <li *ngIf="boundaryLinks" class="page-item" [class.disabled]="!hasNext() || disabled">
-          <a aria-label="Last" class="page-link" href (click)="!!selectPage(pageCount)" [attr.tabindex]="hasNext() ? null : '-1'">
-            <span aria-hidden="true">&raquo;&raquo;</span>
-            <span class="sr-only">Last</span>
-          </a>                
-        </li>        
-      </ul>
-    </nav>
+    <ul [class]="'pagination' + (size ? ' pagination-' + size : '')">
+      <li *ngIf="boundaryLinks" class="page-item"
+        [class.disabled]="!hasPrevious() || disabled">
+        <a aria-label="First" class="page-link" href (click)="!!selectPage(1)" [attr.tabindex]="hasPrevious() ? null : '-1'">
+          <span aria-hidden="true">&laquo;&laquo;</span>
+          <span class="sr-only">First</span>
+        </a>
+      </li>
+
+      <li *ngIf="directionLinks" class="page-item"
+        [class.disabled]="!hasPrevious() || disabled">
+        <a aria-label="Previous" class="page-link" href (click)="!!selectPage(page-1)" [attr.tabindex]="hasPrevious() ? null : '-1'">
+          <span aria-hidden="true">&laquo;</span>
+          <span class="sr-only">Previous</span>
+        </a>
+      </li>
+      <li *ngFor="let pageNumber of pages" class="page-item" [class.active]="pageNumber === page"
+        [class.disabled]="isEllipsis(pageNumber) || disabled">
+        <a *ngIf="isEllipsis(pageNumber)" class="page-link">...</a>
+        <a *ngIf="!isEllipsis(pageNumber)" class="page-link" href (click)="!!selectPage(pageNumber)">
+          {{pageNumber}}
+          <span *ngIf="pageNumber === page" class="sr-only">(current)</span>
+        </a>
+      </li>
+      <li *ngIf="directionLinks" class="page-item" [class.disabled]="!hasNext() || disabled">
+        <a aria-label="Next" class="page-link" href (click)="!!selectPage(page+1)" [attr.tabindex]="hasNext() ? null : '-1'">
+          <span aria-hidden="true">&raquo;</span>
+          <span class="sr-only">Next</span>
+        </a>
+      </li>
+
+      <li *ngIf="boundaryLinks" class="page-item" [class.disabled]="!hasNext() || disabled">
+        <a aria-label="Last" class="page-link" href (click)="!!selectPage(pageCount)" [attr.tabindex]="hasNext() ? null : '-1'">
+          <span aria-hidden="true">&raquo;&raquo;</span>
+          <span class="sr-only">Last</span>
+        </a>
+      </li>
+    </ul>
   `
 })
 export class NgbPagination implements OnChanges {