From aa44ab96cd677e4d4146eb91240de8d347255c47 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 24 Dec 2019 10:11:43 +0200 Subject: [PATCH] fix(tabs): don't start auto-scroll when right clicking on buttons Currently we use `mousedown` to figure out when to start auto-scrolling the tab header, but the event is also dispatched for right clicks. This can be weird, because accidental right clicks that open the system dropdown can start scrolling without being able to stop it easily. These changes make it so we only start for left button clicks. --- src/material-experimental/mdc-tabs/tab-header.html | 4 ++-- src/material-experimental/mdc-tabs/tab-header.spec.ts | 11 +++++++++++ .../mdc-tabs/tab-nav-bar/tab-nav-bar.html | 4 ++-- src/material/tabs/paginated-tab-header.ts | 8 +++++++- src/material/tabs/tab-header.html | 4 ++-- src/material/tabs/tab-header.spec.ts | 11 +++++++++++ src/material/tabs/tab-nav-bar/tab-nav-bar.html | 4 ++-- 7 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/material-experimental/mdc-tabs/tab-header.html b/src/material-experimental/mdc-tabs/tab-header.html index 115739bee519..64dd17e7cf6a 100644 --- a/src/material-experimental/mdc-tabs/tab-header.html +++ b/src/material-experimental/mdc-tabs/tab-header.html @@ -6,7 +6,7 @@ [matRippleDisabled]="_disableScrollBefore || disableRipple" [class.mat-mdc-tab-header-pagination-disabled]="_disableScrollBefore" (click)="_handlePaginatorClick('before')" - (mousedown)="_handlePaginatorPress('before')" + (mousedown)="_handlePaginatorPress('before', $event)" (touchend)="_stopInterval()">
@@ -31,7 +31,7 @@ mat-ripple [matRippleDisabled]="_disableScrollAfter || disableRipple" [class.mat-mdc-tab-header-pagination-disabled]="_disableScrollAfter" - (mousedown)="_handlePaginatorPress('after')" + (mousedown)="_handlePaginatorPress('after', $event)" (click)="_handlePaginatorClick('after')" (touchend)="_stopInterval()">
diff --git a/src/material-experimental/mdc-tabs/tab-header.spec.ts b/src/material-experimental/mdc-tabs/tab-header.spec.ts index b684efb045d7..a80fbee2cf64 100644 --- a/src/material-experimental/mdc-tabs/tab-header.spec.ts +++ b/src/material-experimental/mdc-tabs/tab-header.spec.ts @@ -7,6 +7,7 @@ import { dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent, + createMouseEvent, } from '@angular/cdk/testing/private'; import {CommonModule} from '@angular/common'; import {Component, ViewChild} from '@angular/core'; @@ -458,6 +459,16 @@ describe('MDC-based MatTabHeader', () => { expect(header.scrollDistance).toBe(previousDistance); })); + it('should not scroll when pressing the right mouse button', fakeAsync(() => { + expect(header.scrollDistance).toBe(0, 'Expected to start off not scrolled.'); + + dispatchEvent(nextButton, createMouseEvent('mousedown', undefined, undefined, 2)); + fixture.detectChanges(); + tick(3000); + + expect(header.scrollDistance).toBe(0, 'Expected not to have scrolled after a while.'); + })); + /** * Asserts that auto scrolling using the next button works. * @param startEventName Name of the event that is supposed to start the scrolling. diff --git a/src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.html b/src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.html index 8b77be104601..84e155b3bde9 100644 --- a/src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.html +++ b/src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.html @@ -5,7 +5,7 @@ mat-ripple [matRippleDisabled]="_disableScrollBefore || disableRipple" [class.mat-mdc-tab-header-pagination-disabled]="_disableScrollBefore" (click)="_handlePaginatorClick('before')" - (mousedown)="_handlePaginatorPress('before')" + (mousedown)="_handlePaginatorPress('before', $event)" (touchend)="_stopInterval()">
@@ -24,7 +24,7 @@ aria-hidden="true" mat-ripple [matRippleDisabled]="_disableScrollAfter || disableRipple" [class.mat-mdc-tab-header-pagination-disabled]="_disableScrollAfter" - (mousedown)="_handlePaginatorPress('after')" + (mousedown)="_handlePaginatorPress('after', $event)" (click)="_handlePaginatorClick('after')" (touchend)="_stopInterval()">
diff --git a/src/material/tabs/paginated-tab-header.ts b/src/material/tabs/paginated-tab-header.ts index 7da835e2df24..406fa4ec703d 100644 --- a/src/material/tabs/paginated-tab-header.ts +++ b/src/material/tabs/paginated-tab-header.ts @@ -549,7 +549,13 @@ export abstract class MatPaginatedTabHeader implements AfterContentChecked, Afte * Starts scrolling the header after a certain amount of time. * @param direction In which direction the paginator should be scrolled. */ - _handlePaginatorPress(direction: ScrollDirection) { + _handlePaginatorPress(direction: ScrollDirection, mouseEvent?: MouseEvent) { + // Don't start auto scrolling for right mouse button clicks. Note that we shouldn't have to + // null check the `button`, but we do it so we don't break tests that use fake events. + if (mouseEvent && mouseEvent.button != null && mouseEvent.button !== 0) { + return; + } + // Avoid overlapping timers. this._stopInterval(); diff --git a/src/material/tabs/tab-header.html b/src/material/tabs/tab-header.html index 39abcf94cd2c..e3febcb6f045 100644 --- a/src/material/tabs/tab-header.html +++ b/src/material/tabs/tab-header.html @@ -4,7 +4,7 @@ mat-ripple [matRippleDisabled]="_disableScrollBefore || disableRipple" [class.mat-tab-header-pagination-disabled]="_disableScrollBefore" (click)="_handlePaginatorClick('before')" - (mousedown)="_handlePaginatorPress('before')" + (mousedown)="_handlePaginatorPress('before', $event)" (touchend)="_stopInterval()">
@@ -28,7 +28,7 @@ aria-hidden="true" mat-ripple [matRippleDisabled]="_disableScrollAfter || disableRipple" [class.mat-tab-header-pagination-disabled]="_disableScrollAfter" - (mousedown)="_handlePaginatorPress('after')" + (mousedown)="_handlePaginatorPress('after', $event)" (click)="_handlePaginatorClick('after')" (touchend)="_stopInterval()">
diff --git a/src/material/tabs/tab-header.spec.ts b/src/material/tabs/tab-header.spec.ts index 6687231904e0..cf47ce07fd26 100644 --- a/src/material/tabs/tab-header.spec.ts +++ b/src/material/tabs/tab-header.spec.ts @@ -7,6 +7,7 @@ import { dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent, + createMouseEvent, } from '@angular/cdk/testing/private'; import {CommonModule} from '@angular/common'; import {Component, ViewChild} from '@angular/core'; @@ -458,6 +459,16 @@ describe('MatTabHeader', () => { expect(header.scrollDistance).toBe(previousDistance); })); + it('should not scroll when pressing the right mouse button', fakeAsync(() => { + expect(header.scrollDistance).toBe(0, 'Expected to start off not scrolled.'); + + dispatchEvent(nextButton, createMouseEvent('mousedown', undefined, undefined, 2)); + fixture.detectChanges(); + tick(3000); + + expect(header.scrollDistance).toBe(0, 'Expected not to have scrolled after a while.'); + })); + /** * Asserts that auto scrolling using the next button works. * @param startEventName Name of the event that is supposed to start the scrolling. diff --git a/src/material/tabs/tab-nav-bar/tab-nav-bar.html b/src/material/tabs/tab-nav-bar/tab-nav-bar.html index 8d65b9139400..e905dce2d78f 100644 --- a/src/material/tabs/tab-nav-bar/tab-nav-bar.html +++ b/src/material/tabs/tab-nav-bar/tab-nav-bar.html @@ -4,7 +4,7 @@ mat-ripple [matRippleDisabled]="_disableScrollBefore || disableRipple" [class.mat-tab-header-pagination-disabled]="_disableScrollBefore" (click)="_handlePaginatorClick('before')" - (mousedown)="_handlePaginatorPress('before')" + (mousedown)="_handlePaginatorPress('before', $event)" (touchend)="_stopInterval()">
@@ -23,7 +23,7 @@ aria-hidden="true" mat-ripple [matRippleDisabled]="_disableScrollAfter || disableRipple" [class.mat-tab-header-pagination-disabled]="_disableScrollAfter" - (mousedown)="_handlePaginatorPress('after')" + (mousedown)="_handlePaginatorPress('after', $event)" (click)="_handlePaginatorClick('after')" (touchend)="_stopInterval()">