Skip to content

Commit 8d3bca5

Browse files
crisbetocrisbeto
authored
fix(material/paginator): ignore clicks on disabled buttons (#30138)
The changes in #29379 made the paginator interactive while they're disabled in order to improve accessibility, but as a result it also allows for the buttons to navigate while they're disabled. These changes add internal checks to ensure that the buttons don't navigate while disabled. I've also cleaned up the logic a bit so we don't have four different places that deal with navigations. Fixes #30124.
1 parent db8f6c0 commit 8d3bca5

File tree

4 files changed

+101
-44
lines changed

4 files changed

+101
-44
lines changed

‎src/material/paginator/paginator.html

+8-8
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@
4444
@if (showFirstLastButtons) {
4545
<button mat-icon-button type="button"
4646
class="mat-mdc-paginator-navigation-first"
47-
(click)="firstPage()"
47+
(click)="_buttonClicked(0, _previousButtonsDisabled())"
4848
[attr.aria-label]="_intl.firstPageLabel"
4949
[matTooltip]="_intl.firstPageLabel"
5050
[matTooltipDisabled]="_previousButtonsDisabled()"
51-
[matTooltipPosition]="'above'"
51+
matTooltipPosition="above"
5252
[disabled]="_previousButtonsDisabled()"
5353
disabledInteractive>
5454
<svg class="mat-mdc-paginator-icon"
@@ -61,11 +61,11 @@
6161
}
6262
<button mat-icon-button type="button"
6363
class="mat-mdc-paginator-navigation-previous"
64-
(click)="previousPage()"
64+
(click)="_buttonClicked(pageIndex - 1, _previousButtonsDisabled())"
6565
[attr.aria-label]="_intl.previousPageLabel"
6666
[matTooltip]="_intl.previousPageLabel"
6767
[matTooltipDisabled]="_previousButtonsDisabled()"
68-
[matTooltipPosition]="'above'"
68+
matTooltipPosition="above"
6969
[disabled]="_previousButtonsDisabled()"
7070
disabledInteractive>
7171
<svg class="mat-mdc-paginator-icon"
@@ -77,11 +77,11 @@
7777
</button>
7878
<button mat-icon-button type="button"
7979
class="mat-mdc-paginator-navigation-next"
80-
(click)="nextPage()"
80+
(click)="_buttonClicked(pageIndex + 1, _nextButtonsDisabled())"
8181
[attr.aria-label]="_intl.nextPageLabel"
8282
[matTooltip]="_intl.nextPageLabel"
8383
[matTooltipDisabled]="_nextButtonsDisabled()"
84-
[matTooltipPosition]="'above'"
84+
matTooltipPosition="above"
8585
[disabled]="_nextButtonsDisabled()"
8686
disabledInteractive>
8787
<svg class="mat-mdc-paginator-icon"
@@ -94,11 +94,11 @@
9494
@if (showFirstLastButtons) {
9595
<button mat-icon-button type="button"
9696
class="mat-mdc-paginator-navigation-last"
97-
(click)="lastPage()"
97+
(click)="_buttonClicked(getNumberOfPages() - 1, _nextButtonsDisabled())"
9898
[attr.aria-label]="_intl.lastPageLabel"
9999
[matTooltip]="_intl.lastPageLabel"
100100
[matTooltipDisabled]="_nextButtonsDisabled()"
101-
[matTooltipPosition]="'above'"
101+
matTooltipPosition="above"
102102
[disabled]="_nextButtonsDisabled()"
103103
disabledInteractive>
104104
<svg class="mat-mdc-paginator-icon"

‎src/material/paginator/paginator.spec.ts

+60-12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import {dispatchMouseEvent} from '@angular/cdk/testing/private';
21
import {ChangeDetectorRef, Component, Provider, Type, ViewChild, inject} from '@angular/core';
32
import {ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
43
import {ThemePalette} from '@angular/material/core';
@@ -135,7 +134,7 @@ describe('MatPaginator', () => {
135134
const paginator = component.paginator;
136135
expect(paginator.pageIndex).toBe(0);
137136

138-
dispatchMouseEvent(getNextButton(fixture), 'click');
137+
getNextButton(fixture).click();
139138

140139
expect(paginator.pageIndex).toBe(1);
141140
expect(component.pageEvent).toHaveBeenCalledWith(
@@ -154,7 +153,7 @@ describe('MatPaginator', () => {
154153
fixture.detectChanges();
155154
expect(paginator.pageIndex).toBe(1);
156155

157-
dispatchMouseEvent(getPreviousButton(fixture), 'click');
156+
getPreviousButton(fixture).click();
158157

159158
expect(paginator.pageIndex).toBe(0);
160159
expect(component.pageEvent).toHaveBeenCalledWith(
@@ -164,12 +163,37 @@ describe('MatPaginator', () => {
164163
}),
165164
);
166165
});
166+
167+
it('should not navigate to the next page when the paginator is disabled', () => {
168+
const fixture = createComponent(MatPaginatorApp);
169+
expect(fixture.componentInstance.paginator.pageIndex).toBe(0);
170+
171+
fixture.componentInstance.disabled = true;
172+
fixture.changeDetectorRef.markForCheck();
173+
fixture.detectChanges();
174+
getNextButton(fixture).click();
175+
expect(fixture.componentInstance.paginator.pageIndex).toBe(0);
176+
});
177+
178+
it('should not navigate to the previous page when the paginator is disabled', () => {
179+
const fixture = createComponent(MatPaginatorApp);
180+
fixture.componentInstance.pageIndex = 1;
181+
fixture.changeDetectorRef.markForCheck();
182+
fixture.detectChanges();
183+
184+
expect(fixture.componentInstance.paginator.pageIndex).toBe(1);
185+
186+
fixture.componentInstance.disabled = true;
187+
fixture.changeDetectorRef.markForCheck();
188+
fixture.detectChanges();
189+
getPreviousButton(fixture).click();
190+
expect(fixture.componentInstance.paginator.pageIndex).toBe(1);
191+
});
167192
});
168193

169194
it('should be able to show the first/last buttons', () => {
170195
const fixture = createComponent(MatPaginatorApp);
171196
expect(getFirstButton(fixture)).withContext('Expected first button to not exist.').toBeNull();
172-
173197
expect(getLastButton(fixture)).withContext('Expected last button to not exist.').toBeNull();
174198

175199
fixture.componentInstance.showFirstLastButtons = true;
@@ -271,7 +295,7 @@ describe('MatPaginator', () => {
271295
it('should be able to go to the last page via the last page button', () => {
272296
expect(paginator.pageIndex).toBe(0);
273297

274-
dispatchMouseEvent(getLastButton(fixture), 'click');
298+
getLastButton(fixture).click();
275299

276300
expect(paginator.pageIndex).toBe(9);
277301
expect(component.pageEvent).toHaveBeenCalledWith(
@@ -287,7 +311,7 @@ describe('MatPaginator', () => {
287311
fixture.detectChanges();
288312
expect(paginator.pageIndex).toBe(3);
289313

290-
dispatchMouseEvent(getFirstButton(fixture), 'click');
314+
getFirstButton(fixture).click();
291315

292316
expect(paginator.pageIndex).toBe(0);
293317
expect(component.pageEvent).toHaveBeenCalledWith(
@@ -305,7 +329,7 @@ describe('MatPaginator', () => {
305329
expect(paginator.hasNextPage()).toBe(false);
306330

307331
component.pageEvent.calls.reset();
308-
dispatchMouseEvent(getNextButton(fixture), 'click');
332+
getNextButton(fixture).click();
309333

310334
expect(component.pageEvent).not.toHaveBeenCalled();
311335
expect(paginator.pageIndex).toBe(9);
@@ -316,11 +340,35 @@ describe('MatPaginator', () => {
316340
expect(paginator.hasPreviousPage()).toBe(false);
317341

318342
component.pageEvent.calls.reset();
319-
dispatchMouseEvent(getPreviousButton(fixture), 'click');
343+
getPreviousButton(fixture).click();
320344

321345
expect(component.pageEvent).not.toHaveBeenCalled();
322346
expect(paginator.pageIndex).toBe(0);
323347
});
348+
349+
it('should not navigate to the last page when the paginator is disabled', () => {
350+
expect(fixture.componentInstance.paginator.pageIndex).toBe(0);
351+
352+
fixture.componentInstance.disabled = true;
353+
fixture.changeDetectorRef.markForCheck();
354+
fixture.detectChanges();
355+
getLastButton(fixture).click();
356+
expect(fixture.componentInstance.paginator.pageIndex).toBe(0);
357+
});
358+
359+
it('should not navigate to the first page when the paginator is disabled', () => {
360+
fixture.componentInstance.pageIndex = 1;
361+
fixture.changeDetectorRef.markForCheck();
362+
fixture.detectChanges();
363+
364+
expect(fixture.componentInstance.paginator.pageIndex).toBe(1);
365+
366+
fixture.componentInstance.disabled = true;
367+
fixture.changeDetectorRef.markForCheck();
368+
fixture.detectChanges();
369+
getFirstButton(fixture).click();
370+
expect(fixture.componentInstance.paginator.pageIndex).toBe(1);
371+
});
324372
});
325373

326374
it('should mark for check when inputs are changed directly', () => {
@@ -569,19 +617,19 @@ describe('MatPaginator', () => {
569617
});
570618
});
571619

572-
function getPreviousButton(fixture: ComponentFixture<any>) {
620+
function getPreviousButton(fixture: ComponentFixture<any>): HTMLButtonElement {
573621
return fixture.nativeElement.querySelector('.mat-mdc-paginator-navigation-previous');
574622
}
575623

576-
function getNextButton(fixture: ComponentFixture<any>) {
624+
function getNextButton(fixture: ComponentFixture<any>): HTMLButtonElement {
577625
return fixture.nativeElement.querySelector('.mat-mdc-paginator-navigation-next');
578626
}
579627

580-
function getFirstButton(fixture: ComponentFixture<any>) {
628+
function getFirstButton(fixture: ComponentFixture<any>): HTMLButtonElement {
581629
return fixture.nativeElement.querySelector('.mat-mdc-paginator-navigation-first');
582630
}
583631

584-
function getLastButton(fixture: ComponentFixture<any>) {
632+
function getLastButton(fixture: ComponentFixture<any>): HTMLButtonElement {
585633
return fixture.nativeElement.querySelector('.mat-mdc-paginator-navigation-last');
586634
}
587635

‎src/material/paginator/paginator.ts

+32-24
Original file line numberDiff line numberDiff line change
@@ -241,48 +241,32 @@ export class MatPaginator implements OnInit, OnDestroy {
241241

242242
/** Advances to the next page if it exists. */
243243
nextPage(): void {
244-
if (!this.hasNextPage()) {
245-
return;
244+
if (this.hasNextPage()) {
245+
this._navigate(this.pageIndex + 1);
246246
}
247-
248-
const previousPageIndex = this.pageIndex;
249-
this.pageIndex = this.pageIndex + 1;
250-
this._emitPageEvent(previousPageIndex);
251247
}
252248

253249
/** Move back to the previous page if it exists. */
254250
previousPage(): void {
255-
if (!this.hasPreviousPage()) {
256-
return;
251+
if (this.hasPreviousPage()) {
252+
this._navigate(this.pageIndex - 1);
257253
}
258-
259-
const previousPageIndex = this.pageIndex;
260-
this.pageIndex = this.pageIndex - 1;
261-
this._emitPageEvent(previousPageIndex);
262254
}
263255

264256
/** Move to the first page if not already there. */
265257
firstPage(): void {
266258
// hasPreviousPage being false implies at the start
267-
if (!this.hasPreviousPage()) {
268-
return;
259+
if (this.hasPreviousPage()) {
260+
this._navigate(0);
269261
}
270-
271-
const previousPageIndex = this.pageIndex;
272-
this.pageIndex = 0;
273-
this._emitPageEvent(previousPageIndex);
274262
}
275263

276264
/** Move to the last page if not already there. */
277265
lastPage(): void {
278266
// hasNextPage being false implies at the end
279-
if (!this.hasNextPage()) {
280-
return;
267+
if (this.hasNextPage()) {
268+
this._navigate(this.getNumberOfPages() - 1);
281269
}
282-
283-
const previousPageIndex = this.pageIndex;
284-
this.pageIndex = this.getNumberOfPages() - 1;
285-
this._emitPageEvent(previousPageIndex);
286270
}
287271

288272
/** Whether there is a previous page. */
@@ -369,4 +353,28 @@ export class MatPaginator implements OnInit, OnDestroy {
369353
length: this.length,
370354
});
371355
}
356+
357+
/** Navigates to a specific page index. */
358+
private _navigate(index: number) {
359+
const previousIndex = this.pageIndex;
360+
361+
if (index !== previousIndex) {
362+
this.pageIndex = index;
363+
this._emitPageEvent(previousIndex);
364+
}
365+
}
366+
367+
/**
368+
* Callback invoked when one of the navigation buttons is called.
369+
* @param targetIndex Index to which the paginator should navigate.
370+
* @param isDisabled Whether the button is disabled.
371+
*/
372+
protected _buttonClicked(targetIndex: number, isDisabled: boolean) {
373+
// Note that normally disabled buttons won't dispatch the click event, but the paginator ones
374+
// do, because we're using `disabledInteractive` to allow them to be focusable. We need to
375+
// check here to avoid the navigation.
376+
if (!isDisabled) {
377+
this._navigate(targetIndex);
378+
}
379+
}
372380
}

‎tools/public_api_guard/material/paginator.md

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export function MAT_PAGINATOR_INTL_PROVIDER_FACTORY(parentIntl: MatPaginatorIntl
3535
// @public
3636
export class MatPaginator implements OnInit, OnDestroy {
3737
constructor(_intl: MatPaginatorIntl, _changeDetectorRef: ChangeDetectorRef, defaults?: MatPaginatorDefaultOptions);
38+
protected _buttonClicked(targetIndex: number, isDisabled: boolean): void;
3839
_changePageSize(pageSize: number): void;
3940
color: ThemePalette;
4041
disabled: boolean;

0 commit comments

Comments
 (0)