From 73ab5a0d0c38ad5fd2068a4a20a90915fc5c7fc8 Mon Sep 17 00:00:00 2001 From: Matt Goo Date: Mon, 15 Oct 2018 14:03:03 -0700 Subject: [PATCH] feat(chips): make deselect and toggleSelect private. Update handleChipInteraction/Removal API (#3617) BREAKING CHANGE: deselect and toggleSelect are private methods. handleChipInteraction and handleChipRemoval now accept chipId instead of an event. --- packages/mdc-chips/README.md | 8 +-- packages/mdc-chips/chip-set/foundation.js | 30 ++++---- packages/mdc-chips/chip-set/index.js | 6 +- .../mdc-chips/mdc-chip-set.foundation.test.js | 70 +++++-------------- test/unit/mdc-chips/mdc-chip-set.test.js | 27 ++++--- 5 files changed, 55 insertions(+), 86 deletions(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index 384832607d7..03892d8a18f 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -380,11 +380,9 @@ Method Signature | Description --- | --- `getSelectedChipIds() => boolean` | Returns an array of the IDs of all selected chips `select(chipId: string) => void` | Selects the chip with the given id -`deselect(chipId: string) => void` | Deselects the chip with the given id -`toggleSelect(chipId: string) => void` | Toggles selection of the chip with the given id -`handleChipInteraction(evt: Event) => void` | Handles a custom `MDCChip:interaction` event on the root element -`handleChipSelection(evt: Event) => void` | Handles a custom `MDCChip:selection` event on the root element -`handleChipRemoval(evt: Event) => void` | Handles a custom `MDCChip:removal` event on the root element +`handleChipInteraction(chipId: string) => void` | Handles a custom `MDCChip:interaction` event on the root element +`handleChipSelection(chipId: string, selected: boolean) => void` | Handles a custom `MDCChip:selection` event on the root element +`handleChipRemoval(chipId: string) => void` | Handles a custom `MDCChip:removal` event on the root element #### `MDCChipSetFoundation` Event Handlers diff --git a/packages/mdc-chips/chip-set/foundation.js b/packages/mdc-chips/chip-set/foundation.js index 390b21ab6b9..208ca65fdbb 100644 --- a/packages/mdc-chips/chip-set/foundation.js +++ b/packages/mdc-chips/chip-set/foundation.js @@ -78,11 +78,12 @@ class MDCChipSetFoundation extends MDCFoundation { /** * Toggles selection of the chip with the given id. + * @private * @param {string} chipId */ - toggleSelect(chipId) { + toggleSelect_(chipId) { if (this.selectedChipIds_.indexOf(chipId) >= 0) { - this.deselect(chipId); + this.deselect_(chipId); } else { this.select(chipId); } @@ -108,9 +109,10 @@ class MDCChipSetFoundation extends MDCFoundation { /** * Deselects the chip with the given id. + * @private * @param {string} chipId */ - deselect(chipId) { + deselect_(chipId) { const index = this.selectedChipIds_.indexOf(chipId); if (index >= 0) { this.selectedChipIds_.splice(index, 1); @@ -120,36 +122,34 @@ class MDCChipSetFoundation extends MDCFoundation { /** * Handles a chip interaction event - * @param {!MDCChipInteractionEventType} evt + * @param {string} chipId */ - handleChipInteraction(evt) { - const {chipId} = evt.detail; + handleChipInteraction(chipId) { if (this.adapter_.hasClass(cssClasses.CHOICE) || this.adapter_.hasClass(cssClasses.FILTER)) { - this.toggleSelect(chipId); + this.toggleSelect_(chipId); } } /** * Handles a chip selection event, used to handle discrepancy when selection state is set directly on the Chip. - * @param {!MDCChipSelectionEventType} evt + * @param {string} chipId + * @param {boolean} selected */ - handleChipSelection(evt) { - const {chipId, selected} = evt.detail; + handleChipSelection(chipId, selected) { const chipIsSelected = this.selectedChipIds_.indexOf(chipId) >= 0; if (selected && !chipIsSelected) { this.select(chipId); } else if (!selected && chipIsSelected) { - this.deselect(chipId); + this.deselect_(chipId); } } /** * Handles the event when a chip is removed. - * @param {!MDCChipRemovalEventType} evt + * @param {string} chipId */ - handleChipRemoval(evt) { - const {chipId} = evt.detail; - this.deselect(chipId); + handleChipRemoval(chipId) { + this.deselect_(chipId); this.adapter_.removeChip(chipId); } } diff --git a/packages/mdc-chips/chip-set/index.js b/packages/mdc-chips/chip-set/index.js index b41fe1408e4..05b3ca4ee8b 100644 --- a/packages/mdc-chips/chip-set/index.js +++ b/packages/mdc-chips/chip-set/index.js @@ -77,9 +77,9 @@ class MDCChipSet extends MDCComponent { } }); - this.handleChipInteraction_ = (evt) => this.foundation_.handleChipInteraction(evt); - this.handleChipSelection_ = (evt) => this.foundation_.handleChipSelection(evt); - this.handleChipRemoval_ = (evt) => this.foundation_.handleChipRemoval(evt); + this.handleChipInteraction_ = (evt) => this.foundation_.handleChipInteraction(evt.detail.chipId); + this.handleChipSelection_ = (evt) => this.foundation_.handleChipSelection(evt.detail.chipId, evt.detail.selected); + this.handleChipRemoval_ = (evt) => this.foundation_.handleChipRemoval(evt.detail.chipId); this.root_.addEventListener( MDCChipFoundation.strings.INTERACTION_EVENT, this.handleChipInteraction_); this.root_.addEventListener( diff --git a/test/unit/mdc-chips/mdc-chip-set.foundation.test.js b/test/unit/mdc-chips/mdc-chip-set.foundation.test.js index 334652be987..cb12f637cbf 100644 --- a/test/unit/mdc-chips/mdc-chip-set.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.foundation.test.js @@ -106,18 +106,18 @@ test('in filter chips, #select does nothing if chip is already selected', () => assert.equal(foundation.getSelectedChipIds().length, 1); }); -test('in filter chips, #deselect deselects selected chips', () => { +test('in filter chips, #handleChipInteraction deselects chip if in selectedChipId', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true); - foundation.select('chipA'); - foundation.select('chipB'); + foundation.handleChipInteraction('chipA'); + foundation.handleChipInteraction('chipB'); assert.equal(foundation.getSelectedChipIds().length, 2); - foundation.deselect('chipB'); + foundation.handleChipInteraction('chipB'); td.verify(mockAdapter.setSelected('chipB', false)); assert.equal(foundation.getSelectedChipIds().length, 1); - foundation.deselect('chipA'); + foundation.handleChipInteraction('chipA'); td.verify(mockAdapter.setSelected('chipA', false)); assert.equal(foundation.getSelectedChipIds().length, 0); }); @@ -127,11 +127,7 @@ test('#handleChipInteraction selects chip if the chip set is a filter chip set', td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(false); td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true); - foundation.handleChipInteraction({ - detail: { - chipId: 'chipA', - }, - }); + foundation.handleChipInteraction('chipA'); td.verify(mockAdapter.setSelected('chipA', true)); }); @@ -140,11 +136,7 @@ test('#handleChipInteraction selects chip if the chip set is a choice chip set', td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true); td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(false); - foundation.handleChipInteraction({ - detail: { - chipId: 'chipA', - }, - }); + foundation.handleChipInteraction('chipA'); td.verify(mockAdapter.setSelected('chipA', true)); }); @@ -153,11 +145,7 @@ test('#handleChipInteraction does nothing if the chip set is neither choice nor td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(false); td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(false); - foundation.handleChipInteraction({ - detail: { - chipId: 'chipA', - }, - }); + foundation.handleChipInteraction('chipA'); td.verify(mockAdapter.setSelected('chipA', true), {times: 0}); }); @@ -166,12 +154,7 @@ test('#handleChipSelection selects an unselected chip if selected is true', () = foundation.selectedChipIds_ = []; foundation.select = td.func(); - foundation.handleChipSelection({ - detail: { - chipId: 'chipA', - selected: true, - }, - }); + foundation.handleChipSelection('chipA', true); td.verify(foundation.select('chipA')); }); @@ -180,12 +163,7 @@ test('#handleChipSelection does nothing if selected is true and the chip is alre foundation.selectedChipIds_ = ['chipA']; foundation.select = td.func(); - foundation.handleChipSelection({ - detail: { - chipId: 'chipA', - selected: true, - }, - }); + foundation.handleChipSelection('chipA', true); td.verify(foundation.select('chipA'), {times: 0}); }); @@ -193,37 +171,21 @@ test('#handleChipSelection deselects a selected chip if selected is false', () = const {foundation} = setupTest(); foundation.selectedChipIds_ = ['chipA']; - foundation.deselect = td.func(); - foundation.handleChipSelection({ - detail: { - chipId: 'chipA', - selected: false, - }, - }); - td.verify(foundation.deselect('chipA')); + foundation.handleChipSelection('chipA', false); + assert.equal(foundation.selectedChipIds_.length, 0); }); test('#handleChipSelection does nothing if selected is false and the chip is not selected', () => { const {foundation} = setupTest(); - foundation.selectedChipIds_ = []; - foundation.deselect = td.func(); - foundation.handleChipSelection({ - detail: { - chipId: 'chipA', - selected: false, - }, - }); - td.verify(foundation.deselect('chipA'), {times: 0}); + foundation.selectedChipIds_ = ['chipB']; + foundation.handleChipSelection('chipA', false); + assert.equal(foundation.selectedChipIds_.length, 1); }); test('#handleChipRemoval removes chip', () => { const {foundation, mockAdapter} = setupTest(); - foundation.handleChipRemoval({ - detail: { - chipId: 'chipA', - }, - }); + foundation.handleChipRemoval('chipA'); td.verify(mockAdapter.removeChip('chipA')); }); diff --git a/test/unit/mdc-chips/mdc-chip-set.test.js b/test/unit/mdc-chips/mdc-chip-set.test.js index 36c51d54310..d339c96c698 100644 --- a/test/unit/mdc-chips/mdc-chip-set.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.test.js @@ -89,15 +89,24 @@ test('#destroy cleans up child chip components', () => { test('#initialSyncWithDOM sets up event handlers', () => { const {root, mockFoundation} = setupMockFoundationTest(); - - domEvents.emit(root, MDCChipFoundation.strings.INTERACTION_EVENT); - td.verify(mockFoundation.handleChipInteraction(td.matchers.anything()), {times: 1}); - - domEvents.emit(root, MDCChipFoundation.strings.SELECTION_EVENT); - td.verify(mockFoundation.handleChipSelection(td.matchers.anything()), {times: 1}); - - domEvents.emit(root, MDCChipFoundation.strings.REMOVAL_EVENT); - td.verify(mockFoundation.handleChipRemoval(td.matchers.anything()), {times: 1}); + const {INTERACTION_EVENT, REMOVAL_EVENT, SELECTION_EVENT} = MDCChipFoundation.strings; + const evtData = { + chipId: 'chipA', selected: true, + }; + const evt1 = document.createEvent('CustomEvent'); + const evt2 = document.createEvent('CustomEvent'); + const evt3 = document.createEvent('CustomEvent'); + evt1.initCustomEvent(INTERACTION_EVENT, true, true, evtData); + evt2.initCustomEvent(REMOVAL_EVENT, true, true, evtData); + evt3.initCustomEvent(SELECTION_EVENT, true, true, evtData); + + root.dispatchEvent(evt1); + root.dispatchEvent(evt2); + root.dispatchEvent(evt3); + + td.verify(mockFoundation.handleChipInteraction('chipA'), {times: 1}); + td.verify(mockFoundation.handleChipSelection('chipA', true), {times: 1}); + td.verify(mockFoundation.handleChipRemoval('chipA'), {times: 1}); }); test('#destroy removes event handlers', () => {