Skip to content

Commit 54bcad1

Browse files
fix(material-experimental/chips): remove icon responds to invalid keys
Fixes an issue where pressing any non-arrow key while the chip remove icon is focused removes the chip. This should only happen for ENTER or SPACE. This issue was introduced when MDC refactored the chip foundation. The handleTrailingIconInteraction method, which used to call shouldHandleInteraction to detect if the keydown event was ENTER or SPACE, was replaced with handleTrailingActionInteraction, which handles all keydown events. This change was made in github.com/material-components/material-components-web/pull/5890, and MDC chips were migrated in github.com//pull/19318.
1 parent 96c24f5 commit 54bcad1

File tree

2 files changed

+38
-12
lines changed

2 files changed

+38
-12
lines changed

src/material-experimental/mdc-chips/chip-remove.spec.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
import {Component, DebugElement} from '@angular/core';
88
import {By} from '@angular/platform-browser';
99
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
10-
import {SPACE, ENTER} from '@angular/cdk/keycodes';
10+
import {SPACE, ENTER, X} from '@angular/cdk/keycodes';
1111
import {MatChip, MatChipsModule} from './index';
1212

1313
describe('MDC-based Chip Remove', () => {
@@ -156,6 +156,30 @@ describe('MDC-based Chip Remove', () => {
156156
expect(event.defaultPrevented).toBe(false);
157157
});
158158

159+
it('should remove the chip when ENTER or SPACE is pressed without a modifier key', () => {
160+
const buttonElement = chipNativeElement.querySelector('button')!;
161+
162+
testChip.removable = true;
163+
fixture.detectChanges();
164+
165+
dispatchKeyboardEvent(buttonElement, 'keydown', SPACE);
166+
fixture.detectChanges();
167+
168+
expect(chipNativeElement.classList.contains('mdc-chip--exit')).toBe(true);
169+
});
170+
171+
it('should not remove the chip when a key other than ENTER or SPACE is pressed', () => {
172+
const buttonElement = chipNativeElement.querySelector('button')!;
173+
174+
testChip.removable = true;
175+
fixture.detectChanges();
176+
177+
dispatchKeyboardEvent(buttonElement, 'keydown', X);
178+
fixture.detectChanges();
179+
180+
expect(chipNativeElement.classList.contains('mdc-chip--exit')).toBe(false);
181+
});
182+
159183
it('should have a focus indicator', () => {
160184
const buttonElement = chipNativeElement.querySelector('button')!;
161185

src/material-experimental/mdc-chips/chip.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -375,24 +375,26 @@ export class MatChip extends _MatChipMixinBase implements AfterContentInit, Afte
375375
this.removeIcon.interaction
376376
.pipe(takeUntil(this._destroyed))
377377
.subscribe(event => {
378+
if (this.disabled) {
379+
return;
380+
}
381+
378382
// The MDC chip foundation calls stopPropagation() for any trailing icon interaction
379383
// event, even ones it doesn't handle, so we want to avoid passing it keyboard events
380384
// for which we have a custom handler. Note that we assert the type of the event using
381385
// the `type`, because `instanceof KeyboardEvent` can throw during server-side rendering.
382386
const isKeyboardEvent = event.type.startsWith('key');
387+
const isClickEvent = event.type.startsWith('click');
383388

384-
if (this.disabled || (isKeyboardEvent &&
385-
this.HANDLED_KEYS.indexOf((event as KeyboardEvent).keyCode) !== -1)) {
386-
return;
387-
}
388-
389-
this._chipFoundation.handleTrailingActionInteraction();
390-
391-
if (isKeyboardEvent && !hasModifierKey(event as KeyboardEvent)) {
389+
if (isClickEvent) {
390+
this._chipFoundation.handleTrailingActionInteraction();
391+
} else if (isKeyboardEvent) {
392392
const keyCode = (event as KeyboardEvent).keyCode;
393-
394-
// Prevent default space and enter presses so we don't scroll the page or submit forms.
395-
if (keyCode === SPACE || keyCode === ENTER) {
393+
if ((keyCode === SPACE || keyCode === ENTER) &&
394+
!hasModifierKey(event as KeyboardEvent)) {
395+
this._chipFoundation.handleTrailingActionInteraction();
396+
// Prevent default space and enter presses so we don't scroll the page or submit
397+
// forms.
396398
event.preventDefault();
397399
}
398400
}

0 commit comments

Comments
 (0)