From 5729943baf1726e931e26907c78774f2caec404e Mon Sep 17 00:00:00 2001 From: Patty RoDee Date: Tue, 19 Nov 2019 17:29:09 -0800 Subject: [PATCH] feat(chips): Consolidate interaction event handlers (#5251) Consolidate interaction event handlers into discrete root-level handlers: handleClick and handleKeydown. BREAKING CHANGE: the handleInteraction and handleTrailingIconInteraction handlers have been removed from the MDCChipFoundation. The handleClick handler has been added to the MDCChipFoundation --- 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, 122 insertions(+), 107 deletions(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index 8418fff1d91..b7717ee5609 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -445,10 +445,9 @@ 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 -`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 +`handleClick(evt: Event) => void` | Handles a click event on the root element `handleKeydown(evt: Event) => void` | Handles a keydown event on the root element +`handleTransitionEnd(evt: Event) => void` | Handles a transition end event on the root element `removeFocus() => void` | Removes focusability from the chip #### `MDCChipFoundation` Event Handlers @@ -457,10 +456,9 @@ When wrapping the Chip foundation, the following events must be bound to the ind Events | Element Selector | Foundation Handler --- | --- | --- -`click`, `keydown` | `.mdc-chip` (root) | `handleInteraction()` -`click`, `keydown` | `.mdc-chip__icon--trailing` (if present) | `handleTrailingIconInteraction()` -`transitionend` | `.mdc-chip` (root) | `handleTransitionEnd()` +`click` | `.mdc-chip` (root) | `handleClick()` `keydown` | `.mdc-chip` (root) | `handleKeydown()` +`transitionend` | `.mdc-chip` (root) | `handleTransitionEnd()` #### `MDCChipSetFoundation` diff --git a/packages/mdc-chips/chip/component.ts b/packages/mdc-chips/chip/component.ts index eb55b255daf..248e7361029 100644 --- a/packages/mdc-chips/chip/component.ts +++ b/packages/mdc-chips/chip/component.ts @@ -33,10 +33,6 @@ 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 { @@ -84,20 +80,17 @@ 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 handleInteraction_!: SpecificEventListener; // assigned in initialSyncWithDOM() + private handleClick_!: SpecificEventListener<'click'>; // 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); @@ -112,40 +105,21 @@ export class MDCChip extends MDCComponent implements MDCRippl } initialSyncWithDOM() { - this.handleInteraction_ = (evt: MouseEvent | KeyboardEvent) => this.foundation_.handleInteraction(evt); + this.handleClick_ = (evt: MouseEvent) => this.foundation_.handleClick(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); - INTERACTION_EVENTS.forEach((evtType) => { - this.listen(evtType, this.handleInteraction_); - }); + this.listen('click', this.handleClick_); 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(); - - INTERACTION_EVENTS.forEach((evtType) => { - this.unlisten(evtType, this.handleInteraction_); - }); + this.unlisten('click', this.handleClick_); 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 b15571a2a56..acdc482f0f5 100644 --- a/packages/mdc-chips/chip/foundation.ts +++ b/packages/mdc-chips/chip/foundation.ts @@ -141,11 +141,13 @@ export class MDCChipFoundation extends MDCFoundation { /** * Handles an interaction event on the root element. */ - handleInteraction(evt: MouseEvent | KeyboardEvent) { - if (this.shouldHandleInteraction_(evt)) { - this.adapter_.notifyInteraction(); - this.focusPrimaryAction_(); + handleClick(evt: MouseEvent) { + const trailingIconIsSource = this.adapter_.eventTargetHasClass(evt.target, cssClasses.TRAILING_ICON); + if (trailingIconIsSource) { + return this.notifyTrailingIconInteractionAndRemove_(evt); } + + this.notifyInteractionAndFocus_(); } /** @@ -202,28 +204,25 @@ export class MDCChipFoundation extends MDCFoundation { } } - /** - * Handles an interaction event on the trailing icon element. This is used to - * prevent the ripple from activating on interaction with the trailing icon. - */ - 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) { + 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_(); + } + if (this.shouldRemoveChip_(evt)) { return this.removeChip_(evt); } - const key = evt.key; // Early exit if the key is not usable - if (!navigationKeys.has(key)) { + if (!navigationKeys.has(evt.key)) { return; } @@ -308,20 +307,20 @@ export class MDCChipFoundation extends MDCFoundation { this.adapter_.setPrimaryActionAttr(strings.TAB_INDEX, '-1'); } - private removeChip_(evt: MouseEvent|KeyboardEvent) { + private removeChip_(evt: Event) { evt.stopPropagation(); if (this.shouldRemoveOnTrailingIconClick_) { this.beginExit(); } } - private shouldHandleInteraction_(evt: MouseEvent|KeyboardEvent): boolean { - if (evt.type === 'click') { - return true; - } + private notifyTrailingIconInteractionAndRemove_(evt: Event) { + this.adapter_.notifyTrailingIconInteraction(); + this.removeChip_(evt); + } - const keyEvt = evt as KeyboardEvent; - return keyEvt.key === strings.ENTER_KEY || keyEvt.key === strings.SPACEBAR_KEY; + private shouldProcessKeydownAsClick_(evt: KeyboardEvent): boolean { + return evt.key === strings.ENTER_KEY || evt.key === strings.SPACEBAR_KEY; } private shouldRemoveChip_(evt: KeyboardEvent): boolean { @@ -346,6 +345,11 @@ 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 9c88238e24b..e03809fa0eb 100644 --- a/test/unit/mdc-chips/mdc-chip.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip.foundation.test.js @@ -149,21 +149,19 @@ test(`#beginExit adds ${cssClasses.CHIP_EXIT} class`, () => { td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT)); }); -test('#handleInteraction does not emit event on invalid key', () => { +test('#handleKeydown does not emit event on invalid key', () => { const {foundation, mockAdapter} = setupTest(); - const mockEvt = { + const mockKeydown = { type: 'keydown', key: 'Shift', }; - foundation.handleInteraction(mockEvt); + foundation.handleKeydown(mockKeydown); td.verify(mockAdapter.notifyInteraction(), {times: 0}); }); const validEvents = [ { - type: 'click', - }, { type: 'keydown', key: 'Enter', }, { @@ -173,23 +171,39 @@ const validEvents = [ ]; validEvents.forEach((evt) => { - test(`#handleInteraction(${evt}) notifies interaction`, () => { + test(`#handleKeydown(${evt}) notifies interaction`, () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleInteraction(evt); + foundation.handleKeydown(evt); td.verify(mockAdapter.notifyInteraction()); }); - test(`#handleInteraction(${evt}) focuses the primary action`, () => { + test(`#handleKeydown(${evt}) focuses the primary action`, () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleInteraction(evt); + foundation.handleKeydown(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 = { @@ -304,62 +318,106 @@ test('#handleTransitionEnd does nothing for width property when not exiting', () td.verify(mockAdapter.removeClassFromLeadingIcon(cssClasses.HIDDEN_LEADING_ICON), {times: 0}); }); -test('#handleTrailingIconInteraction emits no event on invalid keys', () => { +test('#handleKeydown emits no custom event on invalid keys', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { - type: 'keydowb', + type: 'keydown', key: 'Shift', stopPropagation: td.func('stopPropagation'), + target: {}, }; - foundation.handleTrailingIconInteraction(mockEvt); + td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); + + foundation.handleKeydown(mockEvt); td.verify(mockAdapter.notifyTrailingIconInteraction(), {times: 0}); }); -test('#handleTrailingIconInteraction emits custom event on click or enter key in trailing icon', () => { +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', () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { type: 'click', stopPropagation: td.func('stopPropagation'), + target: {}, }; - foundation.handleTrailingIconInteraction(mockEvt); + td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); + + foundation.handleClick(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(`#handleTrailingIconInteraction adds ${cssClasses.CHIP_EXIT} class by default on click in trailing icon`, () => { +test(`#handleClick() from trailing icon adds ${cssClasses.CHIP_EXIT} class by default`, () => { const {foundation, mockAdapter} = setupTest(); const mockEvt = { type: 'click', stopPropagation: td.func('stopPropagation'), + target: {}, }; - foundation.handleTrailingIconInteraction(mockEvt); + td.when(mockAdapter.eventTargetHasClass(mockEvt.target, cssClasses.TRAILING_ICON)).thenReturn(true); + foundation.handleClick(mockEvt); assert.isTrue(foundation.getShouldRemoveOnTrailingIconClick()); td.verify(mockAdapter.addClass(cssClasses.CHIP_EXIT)); td.verify(mockEvt.stopPropagation()); }); -test(`#handleTrailingIconInteraction does not add ${cssClasses.CHIP_EXIT} class on click in trailing icon ` + +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 ` + '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.handleTrailingIconInteraction(mockEvt); + foundation.handleClick(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 0574f7dfd8c..5c1f58c1809 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.handleInteraction(td.matchers.anything()), {times: 1}); + td.verify(mockFoundation.handleClick(td.matchers.anything()), {times: 1}); domEvents.emit(root, 'transitionend'); td.verify(mockFoundation.handleTransitionEnd(td.matchers.anything()), {times: 1}); @@ -122,21 +122,12 @@ 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.handleInteraction(td.matchers.anything()), {times: 0}); + td.verify(mockFoundation.handleClick(td.matchers.anything()), {times: 0}); domEvents.emit(root, 'transitionend'); td.verify(mockFoundation.handleTransitionEnd(td.matchers.anything()), {times: 0}); @@ -145,16 +136,6 @@ 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();