From 5e45d77f3e387eff356f5ce93336d4b872c725c4 Mon Sep 17 00:00:00 2001 From: Abhinay Omkar Date: Tue, 3 Dec 2019 18:14:36 -0500 Subject: [PATCH] revert: feat(chips): Consolidate interaction event handlers (#5251) (#5301) --- packages/mdc-chips/README.md | 10 +- packages/mdc-chips/chip/component.ts | 34 +++++- packages/mdc-chips/chip/foundation.ts | 52 ++++----- .../mdc-chips/mdc-chip.foundation.test.js | 110 +++++------------- test/unit/mdc-chips/mdc-chip.test.js | 23 +++- 5 files changed, 107 insertions(+), 122 deletions(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index b7717ee5609..8418fff1d91 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -445,9 +445,10 @@ Method Signature | Description `setShouldRemoveOnTrailingIconClick(shouldRemove: boolean) => void` | Sets whether a trailing icon click should trigger exit/removal of the chip `getDimensions() => ClientRect` | Returns the dimensions of the chip. This is used for applying ripple to the chip. `beginExit() => void` | Begins the exit animation which leads to removal of the chip -`handleClick(evt: Event) => void` | Handles a click event on the root element -`handleKeydown(evt: Event) => void` | Handles a keydown event on the root element +`handleInteraction(evt: Event) => void` | Handles an interaction event on the root element `handleTransitionEnd(evt: Event) => void` | Handles a transition end event on the root element +`handleTrailingIconInteraction(evt: Event) => void` | Handles an interaction event on the trailing icon element +`handleKeydown(evt: Event) => void` | Handles a keydown event on the root element `removeFocus() => void` | Removes focusability from the chip #### `MDCChipFoundation` Event Handlers @@ -456,9 +457,10 @@ When wrapping the Chip foundation, the following events must be bound to the ind Events | Element Selector | Foundation Handler --- | --- | --- -`click` | `.mdc-chip` (root) | `handleClick()` -`keydown` | `.mdc-chip` (root) | `handleKeydown()` +`click`, `keydown` | `.mdc-chip` (root) | `handleInteraction()` +`click`, `keydown` | `.mdc-chip__icon--trailing` (if present) | `handleTrailingIconInteraction()` `transitionend` | `.mdc-chip` (root) | `handleTransitionEnd()` +`keydown` | `.mdc-chip` (root) | `handleKeydown()` #### `MDCChipSetFoundation` diff --git a/packages/mdc-chips/chip/component.ts b/packages/mdc-chips/chip/component.ts index 248e7361029..eb55b255daf 100644 --- a/packages/mdc-chips/chip/component.ts +++ b/packages/mdc-chips/chip/component.ts @@ -33,6 +33,10 @@ import {MDCChipFoundation} from './foundation'; import {MDCChipInteractionEventDetail, MDCChipNavigationEventDetail, MDCChipRemovalEventDetail, MDCChipSelectionEventDetail} from './types'; +type InteractionType = 'click' | 'keydown'; + +const INTERACTION_EVENTS: InteractionType[] = ['click', 'keydown']; + export type MDCChipFactory = (el: Element, foundation?: MDCChipFoundation) => MDCChip; export class MDCChip extends MDCComponent implements MDCRippleCapableSurface { @@ -80,17 +84,20 @@ export class MDCChip extends MDCComponent implements MDCRippl root_!: HTMLElement; // assigned in MDCComponent constructor private leadingIcon_!: Element | null; // assigned in initialize() + private trailingIcon_!: Element | null; // assigned in initialize() private checkmark_!: Element | null; // assigned in initialize() private ripple_!: MDCRipple; // assigned in initialize() private primaryAction_!: Element | null; // assigned in initialize() private trailingAction_!: Element | null; // assigned in initialize() - private handleClick_!: SpecificEventListener<'click'>; // assigned in initialSyncWithDOM() + private handleInteraction_!: SpecificEventListener; // assigned in initialSyncWithDOM() private handleTransitionEnd_!: SpecificEventListener<'transitionend'>; // assigned in initialSyncWithDOM() + private handleTrailingIconInteraction_!: SpecificEventListener; // assigned in initialSyncWithDOM() private handleKeydown_!: SpecificEventListener<'keydown'>; // assigned in initialSyncWithDOM() initialize(rippleFactory: MDCRippleFactory = (el, foundation) => new MDCRipple(el, foundation)) { this.leadingIcon_ = this.root_.querySelector(strings.LEADING_ICON_SELECTOR); + this.trailingIcon_ = this.root_.querySelector(strings.TRAILING_ICON_SELECTOR); this.checkmark_ = this.root_.querySelector(strings.CHECKMARK_SELECTOR); this.primaryAction_ = this.root_.querySelector(strings.PRIMARY_ACTION_SELECTOR); this.trailingAction_ = this.root_.querySelector(strings.TRAILING_ACTION_SELECTOR); @@ -105,21 +112,40 @@ export class MDCChip extends MDCComponent implements MDCRippl } initialSyncWithDOM() { - this.handleClick_ = (evt: MouseEvent) => this.foundation_.handleClick(evt); + this.handleInteraction_ = (evt: MouseEvent | KeyboardEvent) => this.foundation_.handleInteraction(evt); this.handleTransitionEnd_ = (evt: TransitionEvent) => this.foundation_.handleTransitionEnd(evt); + this.handleTrailingIconInteraction_ = (evt: MouseEvent | KeyboardEvent) => + this.foundation_.handleTrailingIconInteraction(evt); this.handleKeydown_ = (evt: KeyboardEvent) => this.foundation_.handleKeydown(evt); - this.listen('click', this.handleClick_); + INTERACTION_EVENTS.forEach((evtType) => { + this.listen(evtType, this.handleInteraction_); + }); this.listen('transitionend', this.handleTransitionEnd_); this.listen('keydown', this.handleKeydown_); + + if (this.trailingIcon_) { + INTERACTION_EVENTS.forEach((evtType) => { + this.trailingIcon_!.addEventListener(evtType, this.handleTrailingIconInteraction_ as EventListener); + }); + } } destroy() { this.ripple_.destroy(); - this.unlisten('click', this.handleClick_); + + INTERACTION_EVENTS.forEach((evtType) => { + this.unlisten(evtType, this.handleInteraction_); + }); this.unlisten('transitionend', this.handleTransitionEnd_); this.unlisten('keydown', this.handleKeydown_); + if (this.trailingIcon_) { + INTERACTION_EVENTS.forEach((evtType) => { + this.trailingIcon_!.removeEventListener(evtType, this.handleTrailingIconInteraction_ as EventListener); + }); + } + super.destroy(); } diff --git a/packages/mdc-chips/chip/foundation.ts b/packages/mdc-chips/chip/foundation.ts index acdc482f0f5..b15571a2a56 100644 --- a/packages/mdc-chips/chip/foundation.ts +++ b/packages/mdc-chips/chip/foundation.ts @@ -141,13 +141,11 @@ export class MDCChipFoundation extends MDCFoundation { /** * Handles an interaction event on the root element. */ - handleClick(evt: MouseEvent) { - const trailingIconIsSource = this.adapter_.eventTargetHasClass(evt.target, cssClasses.TRAILING_ICON); - if (trailingIconIsSource) { - return this.notifyTrailingIconInteractionAndRemove_(evt); + handleInteraction(evt: MouseEvent | KeyboardEvent) { + if (this.shouldHandleInteraction_(evt)) { + this.adapter_.notifyInteraction(); + this.focusPrimaryAction_(); } - - this.notifyInteractionAndFocus_(); } /** @@ -205,24 +203,27 @@ export class MDCChipFoundation extends MDCFoundation { } /** - * Handles a keydown event from the root element. + * Handles an interaction event on the trailing icon element. This is used to + * prevent the ripple from activating on interaction with the trailing icon. */ - handleKeydown(evt: KeyboardEvent) { - const trailingIconIsSource = this.adapter_.eventTargetHasClass(evt.target, cssClasses.TRAILING_ICON); - if (trailingIconIsSource && this.shouldProcessKeydownAsClick_(evt)) { - return this.notifyTrailingIconInteractionAndRemove_(evt); - } - - if (this.shouldProcessKeydownAsClick_(evt)) { - return this.notifyInteractionAndFocus_(); + handleTrailingIconInteraction(evt: MouseEvent | KeyboardEvent) { + if (this.shouldHandleInteraction_(evt)) { + this.adapter_.notifyTrailingIconInteraction(); + this.removeChip_(evt); } + } + /** + * Handles a keydown event from the root element. + */ + handleKeydown(evt: KeyboardEvent) { if (this.shouldRemoveChip_(evt)) { return this.removeChip_(evt); } + const key = evt.key; // Early exit if the key is not usable - if (!navigationKeys.has(evt.key)) { + if (!navigationKeys.has(key)) { return; } @@ -307,20 +308,20 @@ export class MDCChipFoundation extends MDCFoundation { this.adapter_.setPrimaryActionAttr(strings.TAB_INDEX, '-1'); } - private removeChip_(evt: Event) { + private removeChip_(evt: MouseEvent|KeyboardEvent) { evt.stopPropagation(); if (this.shouldRemoveOnTrailingIconClick_) { this.beginExit(); } } - private notifyTrailingIconInteractionAndRemove_(evt: Event) { - this.adapter_.notifyTrailingIconInteraction(); - this.removeChip_(evt); - } + private shouldHandleInteraction_(evt: MouseEvent|KeyboardEvent): boolean { + if (evt.type === 'click') { + return true; + } - private shouldProcessKeydownAsClick_(evt: KeyboardEvent): boolean { - return evt.key === strings.ENTER_KEY || evt.key === strings.SPACEBAR_KEY; + const keyEvt = evt as KeyboardEvent; + return keyEvt.key === strings.ENTER_KEY || keyEvt.key === strings.SPACEBAR_KEY; } private shouldRemoveChip_(evt: KeyboardEvent): boolean { @@ -345,11 +346,6 @@ export class MDCChipFoundation extends MDCFoundation { private notifyIgnoredSelection_(selected: boolean) { this.adapter_.notifySelection(selected, true); } - - private notifyInteractionAndFocus_() { - this.adapter_.notifyInteraction(); - this.focusPrimaryAction_(); - } } // tslint:disable-next-line:no-default-export Needed for backward compatibility with MDC Web v0.44.0 and earlier. diff --git a/test/unit/mdc-chips/mdc-chip.foundation.test.js b/test/unit/mdc-chips/mdc-chip.foundation.test.js index e03809fa0eb..9c88238e24b 100644 --- a/test/unit/mdc-chips/mdc-chip.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip.foundation.test.js @@ -149,19 +149,21 @@ test(`#beginExit adds ${cssClasses.CHIP_EXIT} class`, () => { td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT)); }); -test('#handleKeydown does not emit event on invalid key', () => { +test('#handleInteraction does not emit event on invalid key', () => { const {foundation, mockAdapter} = setupTest(); - const mockKeydown = { + const mockEvt = { type: 'keydown', key: 'Shift', }; - foundation.handleKeydown(mockKeydown); + foundation.handleInteraction(mockEvt); td.verify(mockAdapter.notifyInteraction(), {times: 0}); }); const validEvents = [ { + type: 'click', + }, { type: 'keydown', key: 'Enter', }, { @@ -171,39 +173,23 @@ const validEvents = [ ]; validEvents.forEach((evt) => { - test(`#handleKeydown(${evt}) notifies interaction`, () => { + test(`#handleInteraction(${evt}) notifies interaction`, () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleKeydown(evt); + foundation.handleInteraction(evt); td.verify(mockAdapter.notifyInteraction()); }); - test(`#handleKeydown(${evt}) focuses the primary action`, () => { + test(`#handleInteraction(${evt}) focuses the primary action`, () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleKeydown(evt); + foundation.handleInteraction(evt); td.verify(mockAdapter.setPrimaryActionAttr(strings.TAB_INDEX, '0')); td.verify(mockAdapter.setTrailingActionAttr(strings.TAB_INDEX, '-1')); td.verify(mockAdapter.focusPrimaryAction()); }); }); -test('#handleClick(evt) notifies interaction', () => { - const {foundation, mockAdapter} = setupTest(); - - foundation.handleClick({type: 'click'}); - td.verify(mockAdapter.notifyInteraction()); -}); - -test('#handleClick(evt) focuses the primary action', () => { - const {foundation, mockAdapter} = setupTest(); - - foundation.handleClick({type: 'click'}); - td.verify(mockAdapter.setPrimaryActionAttr(strings.TAB_INDEX, '0')); - td.verify(mockAdapter.setTrailingActionAttr(strings.TAB_INDEX, '-1')); - td.verify(mockAdapter.focusPrimaryAction()); -}); - test('#handleTransitionEnd notifies removal of chip on width transition end', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { @@ -318,106 +304,62 @@ test('#handleTransitionEnd does nothing for width property when not exiting', () td.verify(mockAdapter.removeClassFromLeadingIcon(cssClasses.HIDDEN_LEADING_ICON), {times: 0}); }); -test('#handleKeydown emits no custom event on invalid keys', () => { +test('#handleTrailingIconInteraction emits no event on invalid keys', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { - type: 'keydown', + type: 'keydowb', key: 'Shift', stopPropagation: td.func('stopPropagation'), - target: {}, }; - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); - - foundation.handleKeydown(mockEvt); + foundation.handleTrailingIconInteraction(mockEvt); td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 0}); }); -const validKeys = [ - ' ', // Space - 'Enter', -]; - -validKeys.forEach((key) => { - test(`#handleKeydown() from trailing icon emits custom event on "${key}"`, () => { - const {foundation, mockAdapter} = setupTest(); - const mockEvt = { - type: 'keydown', - stopPropagation: td.func('stopPropagation'), - target: {}, - key, - }; - - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); - - foundation.handleKeydown(mockEvt); - td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 1}); - }); -}); - -test('#handleClick() from trailing icon emits custom event', () => { +test('#handleTrailingIconInteraction emits custom event on click or enter key in trailing icon', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { type: 'click', stopPropagation: td.func('stopPropagation'), - target: {}, }; - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); - - foundation.handleClick(mockEvt); + foundation.handleTrailingIconInteraction(mockEvt); td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 1}); td.verify(mockEvt.stopPropagation(), {times: 1}); + + foundation.handleTrailingIconInteraction(Object.assign(mockEvt, {type: 'keydown', key: ' '})); + td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 2}); + td.verify(mockEvt.stopPropagation(), {times: 2}); + + foundation.handleTrailingIconInteraction(Object.assign(mockEvt, {type: 'keydown', key: 'Enter'})); + td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 3}); + td.verify(mockEvt.stopPropagation(), {times: 3}); }); -test(`#handleClick() from trailing icon adds ${cssClasses.CHIP_EXIT} class by default`, () => { +test(`#handleTrailingIconInteraction adds ${cssClasses.CHIP_EXIT} class by default on click in trailing icon`, () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { type: 'click', stopPropagation: td.func('stopPropagation'), - target: {}, }; - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); + foundation.handleTrailingIconInteraction(mockEvt); - foundation.handleClick(mockEvt); assert.isTrue(foundation.getShouldRemoveOnTrailingIconClick()); td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT)); td.verify(mockEvt.stopPropagation()); }); -validKeys.forEach((key) => { - test(`#handleKeydown({key: "${key}"}) from trailing icon adds ${cssClasses.CHIP_EXIT} class by default`, () => { - const {foundation, mockAdapter} = setupTest(); - const mockEvt = { - type: 'keydown', - stopPropagation: td.func('stopPropagation'), - target: {}, - key, - }; - - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); - - foundation.handleKeydown(mockEvt); - assert.isTrue(foundation.getShouldRemoveOnTrailingIconClick()); - td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT)); - td.verify(mockEvt.stopPropagation()); - }); -}); - -test(`#handleClick() from trailing icon does not add ${cssClasses.CHIP_EXIT} class to trailing icon ` + +test(`#handleTrailingIconInteraction does not add ${cssClasses.CHIP_EXIT} class on click in trailing icon ` + 'if shouldRemoveOnTrailingIconClick_ is false', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { type: 'click', stopPropagation: td.func('stopPropagation'), - target: {}, }; - td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); - foundation.setShouldRemoveOnTrailingIconClick(false); - foundation.handleClick(mockEvt); + foundation.handleTrailingIconInteraction(mockEvt); assert.isFalse(foundation.getShouldRemoveOnTrailingIconClick()); td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT), {times: 0}); diff --git a/test/unit/mdc-chips/mdc-chip.test.js b/test/unit/mdc-chips/mdc-chip.test.js index 5c1f58c1809..0574f7dfd8c 100644 --- a/test/unit/mdc-chips/mdc-chip.test.js +++ b/test/unit/mdc-chips/mdc-chip.test.js @@ -113,7 +113,7 @@ test('#initialSyncWithDOM sets up event handlers', () => { const {root, mockFoundation} = setupMockFoundationTest(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleClick(td.matchers.anything()), {times: 1}); + td.verify(mockFoundation.handleInteraction(td.matchers.anything()), {times: 1}); domEvents.emit(root, 'transitionend'); td.verify(mockFoundation.handleTransitionEnd(td.matchers.anything()), {times: 1}); @@ -122,12 +122,21 @@ test('#initialSyncWithDOM sets up event handlers', () => { td.verify(mockFoundation.handleKeydown(td.matchers.anything()), {times: 1}); }); +test('#initialSyncWithDOM sets up interaction event handler on trailing icon if present', () => { + const root = getFixture(); + const icon = addTrailingIcon(root); + const {mockFoundation} = setupMockFoundationTest(root); + + domEvents.emit(icon, 'click'); + td.verify(mockFoundation.handleTrailingIconInteraction(td.matchers.anything()), {times: 1}); +}); + test('#destroy removes event handlers', () => { const {root, component, mockFoundation} = setupMockFoundationTest(); component.destroy(); domEvents.emit(root, 'click'); - td.verify(mockFoundation.handleClick(td.matchers.anything()), {times: 0}); + td.verify(mockFoundation.handleInteraction(td.matchers.anything()), {times: 0}); domEvents.emit(root, 'transitionend'); td.verify(mockFoundation.handleTransitionEnd(td.matchers.anything()), {times: 0}); @@ -136,6 +145,16 @@ test('#destroy removes event handlers', () => { td.verify(mockFoundation.handleKeydown(td.matchers.anything()), {times: 0}); }); +test('#destroy removes interaction event handler on trailing icon if present', () => { + const root = getFixture(); + const icon = addTrailingIcon(root); + const {component, mockFoundation} = setupMockFoundationTest(root); + + component.destroy(); + domEvents.emit(icon, 'click'); + td.verify(mockFoundation.handleTrailingIconInteraction(td.matchers.anything()), {times: 0}); +}); + test('#destroy destroys ripple', () => { const {component} = setupMockRippleTest(); component.destroy();