From 66f2464e199a54872530ba514e61ff145d35cc80 Mon Sep 17 00:00:00 2001 From: Bonnie Zhou Date: Fri, 6 Apr 2018 13:36:47 -0700 Subject: [PATCH] fix(chips): Manage chip selection for classes added manually (#2391) BREAKING CHANGE: isSelected method added to MDCChip, and related methods added to MDCChipFoundation and MDCChipSetFoundation. --- demos/chips.html | 10 +-- packages/mdc-chips/README.md | 40 +++++++++- packages/mdc-chips/_mixins.scss | 5 +- packages/mdc-chips/chip-set/foundation.js | 54 +++++++++----- packages/mdc-chips/chip-set/index.js | 8 ++ packages/mdc-chips/chip/constants.js | 2 +- packages/mdc-chips/chip/foundation.js | 17 +++-- packages/mdc-chips/chip/index.js | 7 +- packages/mdc-chips/chip/mdc-chip.scss | 2 +- packages/mdc-toolbar/mdc-toolbar.scss | 2 +- .../mdc-chips/mdc-chip-set.foundation.test.js | 74 ++++++++++++++----- test/unit/mdc-chips/mdc-chip-set.test.js | 19 +++++ .../mdc-chips/mdc-chip.foundation.test.js | 20 +++-- test/unit/mdc-chips/mdc-chip.test.js | 6 +- 14 files changed, 196 insertions(+), 70 deletions(-) diff --git a/demos/chips.html b/demos/chips.html index c23f4108031..154b9dea3b7 100644 --- a/demos/chips.html +++ b/demos/chips.html @@ -78,7 +78,7 @@

Choice Chips

Small
-
+
Medium
@@ -94,7 +94,7 @@

Choice Chips

Filter Chips

No leading icon

-
+
@@ -103,7 +103,7 @@

No leading icon

Tops
cancel
-
+
@@ -134,8 +134,8 @@

No leading icon


With leading icon

-
- face +
+ face
diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index 97499d0de00..d63335d644c 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -90,7 +90,7 @@ Filter chips are a variant of chips which allow multiple selection from a set of
``` -> _NOTE_: To use a leading icon in a filter chip, put the `mdc-chip__icon--leading` element _before_ the `mdc-chip__checkmark` element: +To use a leading icon in a filter chip, put the `mdc-chip__icon--leading` element _before_ the `mdc-chip__checkmark` element: ```html
@@ -105,6 +105,31 @@ Filter chips are a variant of chips which allow multiple selection from a set of
``` +#### Pre-selected + +To display a pre-selected chip, add the class `mdc-chip--selected` to the root chip element. + +```html +
+
Add to calendar
+
+``` + +To pre-select filter chips that have a leading icon, also add the class `mdc-chip__icon--leading-hidden` to the `mdc-chip__icon--leading` element. This will ensure that the checkmark displaces the leading icon. + +```html +
+ face +
+ + + +
+
Filterable content
+
+``` + ### CSS Classes CSS Class | Description @@ -113,9 +138,11 @@ CSS Class | Description `mdc-chip-set--choice` | Optional. Indicates that the chips in the set are choice chips, which allow a single selection from a set of options. `mdc-chip-set--filter` | Optional. Indicates that the chips in the set are filter chips, which allow multiple selection from a set of options. `mdc-chip` | Mandatory. +`mdc-chip--selected` | Optional. Indicates that the chip is selected. `mdc-chip__text` | Mandatory. Indicates the text content of the chip. `mdc-chip__icon` | Optional. Indicates an icon in the chip. `mdc-chip__icon--leading` | Optional. Indicates a leading icon in the chip. +`mdc-chip__icon--leading-hidden` | Optional. Hides the leading icon in a filter chip when the chip is selected. `mdc-chip__icon--trailing` | Optional. Indicates a trailing icon in the chip. `mdc-chip__checkmark` | Optional. Indicates the checkmark in a filter chip. `mdc-chip__checkmark-svg` | Mandatory with the use of `mdc-chip__checkmark`. Indicates the checkmark SVG element in a filter chip. @@ -155,7 +182,7 @@ To use the `MDCChip` and `MDCChipSet` classes, [import](../../docs/importing-js. Method Signature | Description --- | --- `get foundation() => MDCChipFoundation` | Returns the foundation -`toggleSelected() => void` | Proxies to the foundation's `toggleSelected` method +`isSelected() => boolean` | Proxies to the foundation's `isSelected` method Property | Value Type | Description --- | --- | --- @@ -202,7 +229,12 @@ Method Signature | Description Method Signature | Description --- | --- -`toggleSelected() => void` | Toggles the selected class on the chip element +`isSelected() => boolean` | Returns true if the chip is selected +`setSelected(selected: boolean) => void` | Sets the chip's selected state #### `MDCChipSetFoundation` -None yet, coming soon. + +Method Signature | Description +--- | --- +`select(chipFoundation: MDCChipFoundation) => void` | Selects the given chip +`deselect(chipFoundation: MDCChipFoundation) => void` | Deselects the given chip diff --git a/packages/mdc-chips/_mixins.scss b/packages/mdc-chips/_mixins.scss index d6313a0eeaf..dc7722c6d79 100644 --- a/packages/mdc-chips/_mixins.scss +++ b/packages/mdc-chips/_mixins.scss @@ -86,10 +86,7 @@ $horizontal-padding-value: max($mdc-chip-horizontal-padding - $width, 0); $vertical-padding-value: max($mdc-chip-vertical-padding - $width, 0); - padding-top: $vertical-padding-value; - padding-right: $horizontal-padding-value; - padding-bottom: $vertical-padding-value; - padding-left: $horizontal-padding-value; + padding: $vertical-padding-value $horizontal-padding-value; border-width: $width; } diff --git a/packages/mdc-chips/chip-set/foundation.js b/packages/mdc-chips/chip-set/foundation.js index b32b95c3275..4e5e9b0983b 100644 --- a/packages/mdc-chips/chip-set/foundation.js +++ b/packages/mdc-chips/chip-set/foundation.js @@ -74,31 +74,51 @@ class MDCChipSetFoundation extends MDCFoundation { MDCChipFoundation.strings.INTERACTION_EVENT, this.chipInteractionHandler_); } + /** + * Selects the given chip. Deselects all other chips if the chip set is of the choice variant. + * @param {!MDCChipFoundation} chipFoundation + */ + select(chipFoundation) { + if (this.adapter_.hasClass(cssClasses.CHOICE)) { + this.deselectAll_(); + } + chipFoundation.setSelected(true); + this.selectedChips_.push(chipFoundation); + } + + /** + * Deselects the given chip. + * @param {!MDCChipFoundation} chipFoundation + */ + deselect(chipFoundation) { + const index = this.selectedChips_.indexOf(chipFoundation); + if (index >= 0) { + this.selectedChips_.splice(index, 1); + } + chipFoundation.setSelected(false); + } + + /** Deselects all selected chips. */ + deselectAll_() { + this.selectedChips_.forEach((chipFoundation) => { + chipFoundation.setSelected(false); + }); + this.selectedChips_.length = 0; + } + /** * Handles a chip interaction event - * @param {!Object} evt + * @param {!Event} evt * @private */ handleChipInteraction_(evt) { const chipFoundation = evt.detail.chip.foundation; - if (this.adapter_.hasClass(cssClasses.CHOICE)) { - if (this.selectedChips_.length === 0) { - this.selectedChips_[0] = chipFoundation; - } else if (this.selectedChips_[0] !== chipFoundation) { - this.selectedChips_[0].toggleSelected(); - this.selectedChips_[0] = chipFoundation; - } else { - this.selectedChips_ = []; - } - chipFoundation.toggleSelected(); - } else if (this.adapter_.hasClass(cssClasses.FILTER)) { - const index = this.selectedChips_.indexOf(chipFoundation); - if (index >= 0) { - this.selectedChips_.splice(index, 1); + if (this.adapter_.hasClass(cssClasses.CHOICE) || this.adapter_.hasClass(cssClasses.FILTER)) { + if (chipFoundation.isSelected()) { + this.deselect(chipFoundation); } else { - this.selectedChips_.push(chipFoundation); + this.select(chipFoundation); } - chipFoundation.toggleSelected(); } } } diff --git a/packages/mdc-chips/chip-set/index.js b/packages/mdc-chips/chip-set/index.js index 4ff5946df39..98e9d0f7b3e 100644 --- a/packages/mdc-chips/chip-set/index.js +++ b/packages/mdc-chips/chip-set/index.js @@ -58,6 +58,14 @@ class MDCChipSet extends MDCComponent { }); } + initialSyncWithDOM() { + this.chips.forEach((chip) => { + if (chip.isSelected()) { + this.foundation_.select(chip.foundation); + } + }); + } + /** * @return {!MDCChipSetFoundation} */ diff --git a/packages/mdc-chips/chip/constants.js b/packages/mdc-chips/chip/constants.js index 02c008c6299..2751ebad979 100644 --- a/packages/mdc-chips/chip/constants.js +++ b/packages/mdc-chips/chip/constants.js @@ -27,7 +27,7 @@ const strings = { /** @enum {string} */ const cssClasses = { CHECKMARK: 'mdc-chip__checkmark', - HIDDEN_LEADING_ICON: 'mdc-chip__icon--hidden-leading', + HIDDEN_LEADING_ICON: 'mdc-chip__icon--leading-hidden', LEADING_ICON: 'mdc-chip__icon--leading', SELECTED: 'mdc-chip--selected', }; diff --git a/packages/mdc-chips/chip/foundation.js b/packages/mdc-chips/chip/foundation.js index 3eeffd5c450..a3729cc584c 100644 --- a/packages/mdc-chips/chip/foundation.js +++ b/packages/mdc-chips/chip/foundation.js @@ -92,13 +92,20 @@ class MDCChipFoundation extends MDCFoundation { } /** - * Toggles the selected class on the chip element. + * @return {boolean} */ - toggleSelected() { - if (this.adapter_.hasClass(cssClasses.SELECTED)) { - this.adapter_.removeClass(cssClasses.SELECTED); - } else { + isSelected() { + return this.adapter_.hasClass(cssClasses.SELECTED); + } + + /** + * @param {boolean} selected + */ + setSelected(selected) { + if (selected) { this.adapter_.addClass(cssClasses.SELECTED); + } else { + this.adapter_.removeClass(cssClasses.SELECTED); } } diff --git a/packages/mdc-chips/chip/index.js b/packages/mdc-chips/chip/index.js index df425a90298..ddde612ad0a 100644 --- a/packages/mdc-chips/chip/index.js +++ b/packages/mdc-chips/chip/index.js @@ -75,10 +75,11 @@ class MDCChip extends MDCComponent { } /** - * Toggles selected state of the chip. + * Returns true if the chip is selected. + * @return {boolean} */ - toggleSelected() { - this.foundation_.toggleSelected(); + isSelected() { + return this.foundation_.isSelected(); } /** diff --git a/packages/mdc-chips/chip/mdc-chip.scss b/packages/mdc-chips/chip/mdc-chip.scss index 22c7415e8f8..31ede9c5e36 100644 --- a/packages/mdc-chips/chip/mdc-chip.scss +++ b/packages/mdc-chips/chip/mdc-chip.scss @@ -141,7 +141,7 @@ } } -.mdc-chip__icon--hidden-leading.mdc-chip__icon--leading { +.mdc-chip__icon--leading-hidden.mdc-chip__icon--leading { width: 0; // This ensures that the leading icon doesn't fade in while the checkmark is fading out. diff --git a/packages/mdc-toolbar/mdc-toolbar.scss b/packages/mdc-toolbar/mdc-toolbar.scss index 5319db472c1..34081c0b343 100644 --- a/packages/mdc-toolbar/mdc-toolbar.scss +++ b/packages/mdc-toolbar/mdc-toolbar.scss @@ -67,7 +67,7 @@ @media (max-width: $mdc-toolbar-mobile-landscape-width-breakpoint) and (orientation: landscape) { - padding: 0 0; + padding: 0; } @media (max-width: $mdc-toolbar-mobile-breakpoint) { 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 9e9d141a129..6ca516c37c6 100644 --- a/test/unit/mdc-chips/mdc-chip-set.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.foundation.test.js @@ -43,12 +43,14 @@ const setupTest = () => { const foundation = new MDCChipSetFoundation(mockAdapter); const chipA = td.object({ foundation: { - toggleSelected: () => {}, + isSelected: () => {}, + setSelected: () => {}, }, }); const chipB = td.object({ foundation: { - toggleSelected: () => {}, + isSelected: () => {}, + setSelected: () => {}, }, }); return {foundation, mockAdapter, chipA, chipB}; @@ -68,8 +70,8 @@ test('#destroy removes event listeners', () => { td.verify(mockAdapter.deregisterInteractionHandler('MDCChip:interaction', td.matchers.isA(Function))); }); -test('on custom MDCChip:interaction event toggles selected state with single selection on choice chips', () => { - const {foundation, mockAdapter, chipA, chipB} = setupTest(); +test('in choice chips, on custom MDCChip:interaction event selects chip if no chips are selected', () => { + const {foundation, mockAdapter, chipA} = setupTest(); let chipInteractionHandler; td.when(mockAdapter.registerInteractionHandler('MDCChip:interaction', td.matchers.isA(Function))) .thenDo((evtType, handler) => { @@ -77,7 +79,9 @@ test('on custom MDCChip:interaction event toggles selected state with single sel }); td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true); + td.when(chipA.foundation.isSelected()).thenReturn(false); assert.equal(foundation.selectedChips_.length, 0); + foundation.init(); chipInteractionHandler({ @@ -85,28 +89,37 @@ test('on custom MDCChip:interaction event toggles selected state with single sel chip: chipA, }, }); - td.verify(chipA.foundation.toggleSelected()); + td.verify(chipA.foundation.setSelected(true)); assert.equal(foundation.selectedChips_.length, 1); +}); - chipInteractionHandler({ - detail: { - chip: chipB, - }, - }); - td.verify(chipA.foundation.toggleSelected()); - td.verify(chipB.foundation.toggleSelected()); +test('in choice chips, on custom MDCChip:interaction event deselects chip if another chip is selected', () => { + const {foundation, mockAdapter, chipA, chipB} = setupTest(); + let chipInteractionHandler; + td.when(mockAdapter.registerInteractionHandler('MDCChip:interaction', td.matchers.isA(Function))) + .thenDo((evtType, handler) => { + chipInteractionHandler = handler; + }); + td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true); + + foundation.select(chipB.foundation); + td.when(chipA.foundation.isSelected()).thenReturn(false); + td.when(chipB.foundation.isSelected()).thenReturn(true); assert.equal(foundation.selectedChips_.length, 1); + foundation.init(); + chipInteractionHandler({ detail: { - chip: chipB, + chip: chipA, }, }); - td.verify(chipB.foundation.toggleSelected()); - assert.equal(foundation.selectedChips_.length, 0); + td.verify(chipA.foundation.setSelected(true)); + td.verify(chipB.foundation.setSelected(false)); + assert.equal(foundation.selectedChips_.length, 1); }); -test('on custom MDCChip:interaction event toggles selected state with multi-selection on filter chips', () => { +test('in filter chips, on custom MDCChip:interaction event selects multiple chips', () => { const {foundation, mockAdapter, chipA, chipB} = setupTest(); let chipInteractionHandler; td.when(mockAdapter.registerInteractionHandler('MDCChip:interaction', td.matchers.isA(Function))) @@ -115,7 +128,10 @@ test('on custom MDCChip:interaction event toggles selected state with multi-sele }); td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true); + td.when(chipA.foundation.isSelected()).thenReturn(false); + td.when(chipB.foundation.isSelected()).thenReturn(false); assert.equal(foundation.selectedChips_.length, 0); + foundation.init(); chipInteractionHandler({ @@ -123,7 +139,7 @@ test('on custom MDCChip:interaction event toggles selected state with multi-sele chip: chipA, }, }); - td.verify(chipA.foundation.toggleSelected()); + td.verify(chipA.foundation.setSelected(true)); assert.equal(foundation.selectedChips_.length, 1); chipInteractionHandler({ @@ -131,15 +147,33 @@ test('on custom MDCChip:interaction event toggles selected state with multi-sele chip: chipB, }, }); - td.verify(chipB.foundation.toggleSelected()); + td.verify(chipB.foundation.setSelected(true)); + assert.equal(foundation.selectedChips_.length, 2); +}); + +test('in filter chips, on custom MDCChip:interaction event deselects selected chips', () => { + const {foundation, mockAdapter, chipA, chipB} = setupTest(); + let chipInteractionHandler; + td.when(mockAdapter.registerInteractionHandler('MDCChip:interaction', td.matchers.isA(Function))) + .thenDo((evtType, handler) => { + chipInteractionHandler = handler; + }); + td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true); + + foundation.select(chipA.foundation); + foundation.select(chipB.foundation); + td.when(chipA.foundation.isSelected()).thenReturn(true); + td.when(chipB.foundation.isSelected()).thenReturn(true); assert.equal(foundation.selectedChips_.length, 2); + foundation.init(); + chipInteractionHandler({ detail: { chip: chipB, }, }); - td.verify(chipB.foundation.toggleSelected()); + td.verify(chipB.foundation.setSelected(false)); assert.equal(foundation.selectedChips_.length, 1); chipInteractionHandler({ @@ -147,6 +181,6 @@ test('on custom MDCChip:interaction event toggles selected state with multi-sele chip: chipA, }, }); - td.verify(chipA.foundation.toggleSelected()); + td.verify(chipA.foundation.setSelected(false)); assert.equal(foundation.selectedChips_.length, 0); }); diff --git a/test/unit/mdc-chips/mdc-chip-set.test.js b/test/unit/mdc-chips/mdc-chip-set.test.js index 32d831efacf..f125cdbd209 100644 --- a/test/unit/mdc-chips/mdc-chip-set.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.test.js @@ -44,6 +44,7 @@ test('attachTo returns an MDCChipSet instance', () => { class FakeChip { constructor() { this.destroy = td.func('.destroy'); + this.isSelected = td.func('.isSelected'); } } @@ -65,6 +66,24 @@ test('#destroy cleans up child chip components', () => { td.verify(component.chips[2].destroy()); }); +class FakeSelectedChip { + constructor() { + this.foundation = td.object({ + setSelected: () => {}, + }); + this.destroy = td.func('.destroy'); + this.isSelected = () => true; + } +} + +test('#initialSyncWithDOM sets selects chips with mdc-chip--selected class', () => { + const root = getFixture(); + const component = new MDCChipSet(root, undefined, (el) => new FakeSelectedChip(el)); + td.verify(component.foundation_.select(component.chips[0].foundation)); + td.verify(component.foundation_.select(component.chips[1].foundation)); + td.verify(component.foundation_.select(component.chips[2].foundation)); +}); + function setupTest() { const root = getFixture(); const component = new MDCChipSet(root); diff --git a/test/unit/mdc-chips/mdc-chip.foundation.test.js b/test/unit/mdc-chips/mdc-chip.foundation.test.js index 51773aa5120..c2e7dd454ef 100644 --- a/test/unit/mdc-chips/mdc-chip.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip.foundation.test.js @@ -72,19 +72,27 @@ test('#destroy removes event listeners', () => { td.verify(mockAdapter.deregisterTrailingIconInteractionHandler('mousedown', td.matchers.isA(Function))); }); -test('#toggleSelected adds mdc-chip--selected class if the class does not exist', () => { +test('#isSelected returns true if mdc-chip--selected class is present', () => { + const {foundation, mockAdapter} = setupTest(); + td.when(mockAdapter.hasClass(cssClasses.SELECTED)).thenReturn(true); + assert.isTrue(foundation.isSelected()); +}); + +test('#isSelected returns false if mdc-chip--selected class is not present', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.hasClass(cssClasses.SELECTED)).thenReturn(false); + assert.isFalse(foundation.isSelected()); +}); - foundation.toggleSelected(); +test('#setSelected adds mdc-chip--selected class if true', () => { + const {foundation, mockAdapter} = setupTest(); + foundation.setSelected(true); td.verify(mockAdapter.addClass(cssClasses.SELECTED)); }); -test('#toggleSelected removes mdc-chip--selected class if the class exists', () => { +test('#setSelected removes mdc-chip--selected class if false', () => { const {foundation, mockAdapter} = setupTest(); - td.when(mockAdapter.hasClass(cssClasses.SELECTED)).thenReturn(true); - - foundation.toggleSelected(); + foundation.setSelected(false); td.verify(mockAdapter.removeClass(cssClasses.SELECTED)); }); diff --git a/test/unit/mdc-chips/mdc-chip.test.js b/test/unit/mdc-chips/mdc-chip.test.js index d039b3bd2de..eb51affeb8f 100644 --- a/test/unit/mdc-chips/mdc-chip.test.js +++ b/test/unit/mdc-chips/mdc-chip.test.js @@ -186,8 +186,8 @@ function setupMockFoundationTest(root = getFixture()) { return {root, component, mockFoundation}; } -test('#toggleSelected proxies to foundation', () => { +test('#isSelected proxies to foundation', () => { const {component, mockFoundation} = setupMockFoundationTest(); - component.toggleSelected(); - td.verify(mockFoundation.toggleSelected()); + component.isSelected(); + td.verify(mockFoundation.isSelected()); });