From 29ac2c2ec22f062895ed1fc8fc4a52b3ccb4e5b8 Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 3 Jul 2019 10:37:10 -0700 Subject: [PATCH 1/4] feat(chips): Add setSelectedFromChipset method --- packages/mdc-chips/README.md | 4 ++- packages/mdc-chips/chip-set/component.ts | 2 +- packages/mdc-chips/chip/component.ts | 4 +++ packages/mdc-chips/chip/foundation.ts | 27 +++++++++++++------ test/unit/mdc-chips/mdc-chip-set.test.js | 5 ++-- .../mdc-chips/mdc-chip.foundation.test.js | 11 ++++++-- test/unit/mdc-chips/mdc-chip.test.js | 6 +++++ 7 files changed, 45 insertions(+), 14 deletions(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index d934c103f5b..103aedda61f 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -310,6 +310,7 @@ Method Signature | Description `focusPrimaryAction() => void` | Proxies to the foundation's `focusPrimaryAction` method `focusTrailingAction() => void` | Proxies to the foundation's `focusTrailingAction` method `removeFocus() => void` | Proxies to the foundation's `removeFocus` method +`setSelectedFromChipset(selected: boolean) => void` | Proxies to the foudnation's `setSelectedFromChipset` method (only called from the chip set) Property | Value Type | Description --- | --- | --- @@ -392,7 +393,7 @@ Method Signature | Description --- | --- `hasClass(className: string) => boolean` | Returns whether the chip set element has the given class `removeChipAtIndex(index: number) => void` | Removes the chip with the given `index` from the chip set -`selectChipAtIndex(index: string, selected: boolean) => void` | Sets the selected state of the chip at the given `index` +`selectChipAtIndex(index: string, selected: boolean) => void` | Calls `MDCChip#setSelectedFromChipset(selected)` on the chip at the given `index` `getIndexOfChipById(id: string) => number` | Returns the index of the chip with the matching `id` or -1 `focusChipPrimaryActionAtIndex(index: number) => void` | Calls `MDCChip#focusPrimaryAction()` on the chip at the given `index` `focusChipTrailingActionAtIndex(index: number) => void` | Calls `MDCChip#focusTrailingAction()` on the chip at the given `index` @@ -408,6 +409,7 @@ Method Signature | Description --- | --- `isSelected() => boolean` | Returns true if the chip is selected `setSelected(selected: boolean) => void` | Sets the chip's selected state +`setSelectedFromChipset(selected: boolean) => void` | Sets the chip's selected state (called from the chip set) `getShouldRemoveOnTrailingIconClick() => boolean` | Returns whether a trailing icon click should trigger exit/removal of the chip `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. diff --git a/packages/mdc-chips/chip-set/component.ts b/packages/mdc-chips/chip-set/component.ts index ea7b88ae047..6e013d965aa 100644 --- a/packages/mdc-chips/chip-set/component.ts +++ b/packages/mdc-chips/chip-set/component.ts @@ -132,7 +132,7 @@ export class MDCChipSet extends MDCComponent { }, selectChipAtIndex: (index, selected) => { if (index >= 0 && index < this.chips_.length) { - this.chips_[index].selected = selected; + this.chips_[index].setSelectedFromChipset(selected); } }, }; diff --git a/packages/mdc-chips/chip/component.ts b/packages/mdc-chips/chip/component.ts index c6aeaf1bed2..5a41c41e44b 100644 --- a/packages/mdc-chips/chip/component.ts +++ b/packages/mdc-chips/chip/component.ts @@ -217,6 +217,10 @@ export class MDCChip extends MDCComponent implements MDCRippl return new MDCChipFoundation(adapter); } + setSelectedFromChipset(selected: boolean) { + this.foundation_.setSelectedFromChipset(selected); + } + focusPrimaryAction() { this.foundation_.focusPrimaryAction(); } diff --git a/packages/mdc-chips/chip/foundation.ts b/packages/mdc-chips/chip/foundation.ts index d28399317df..c5d3a228ffc 100644 --- a/packages/mdc-chips/chip/foundation.ts +++ b/packages/mdc-chips/chip/foundation.ts @@ -84,14 +84,11 @@ export class MDCChipFoundation extends MDCFoundation { } setSelected(selected: boolean) { - if (selected) { - this.adapter_.addClass(cssClasses.SELECTED); - this.adapter_.setPrimaryActionAttr(strings.ARIA_CHECKED, 'true'); - } else { - this.adapter_.removeClass(cssClasses.SELECTED); - this.adapter_.setPrimaryActionAttr(strings.ARIA_CHECKED, 'false'); - } - this.adapter_.notifySelection(selected); + this.setSelected_(selected, true); + } + + setSelectedFromChipset(selected: boolean) { + this.setSelected_(selected, false); } getShouldRemoveOnTrailingIconClick() { @@ -327,6 +324,20 @@ export class MDCChipFoundation extends MDCFoundation { const isDeletable = this.adapter_.hasClass(cssClasses.DELETABLE); return isDeletable && (evt.key === strings.BACKSPACE_KEY || evt.key === strings.DELETE_KEY); } + + private setSelected_(selected: boolean, shouldNotify: boolean) { + if (selected) { + this.adapter_.addClass(cssClasses.SELECTED); + this.adapter_.setPrimaryActionAttr(strings.ARIA_CHECKED, 'true'); + } else { + this.adapter_.removeClass(cssClasses.SELECTED); + this.adapter_.setPrimaryActionAttr(strings.ARIA_CHECKED, 'false'); + } + + if (shouldNotify) { + this.adapter_.notifySelection(selected); + } + } } // 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-set.test.js b/test/unit/mdc-chips/mdc-chip-set.test.js index dc0467a5457..a102642309f 100644 --- a/test/unit/mdc-chips/mdc-chip-set.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.test.js @@ -57,6 +57,7 @@ class FakeChip { this.focusTrailingAction = td.func('.focusTrailingAction'); this.remove = td.func('.remove'); this.removeFocus = td.func('.removeFocus'); + this.setSelectedFromChipset = td.func('.setSelectedFromChipset'); this.selected = false; } } @@ -177,11 +178,11 @@ test('#adapter.removeChipAtIndex does nothing if the given object is not in the assert.equal(component.chips.length, 3); }); -test('#adapter.selectChipAtIndex sets selected on chip object', () => { +test('#adapter.selectChipAtIndex calls setSelectedFromChipset on chip object', () => { const {component} = setupTest(); const chip = component.chips[0]; component.getDefaultFoundation().adapter_.selectChipAtIndex(0, true); - assert.equal(chip.selected, true); + td.verify(chip.setSelectedFromChipset(true)); }); test('#adapter.getChipListCount returns the number of chips', () => { diff --git a/test/unit/mdc-chips/mdc-chip.foundation.test.js b/test/unit/mdc-chips/mdc-chip.foundation.test.js index 555ab61f39b..ccb49e013b9 100644 --- a/test/unit/mdc-chips/mdc-chip.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip.foundation.test.js @@ -92,18 +92,25 @@ test('#setSelected sets aria-checked="false" if false', () => { td.verify(mockAdapter.setPrimaryActionAttr(strings.ARIA_CHECKED, 'false')); }); -test('#setSelected removes calls adapter.notifySelection when selected is true', () => { +test('#setSelected notifies of selection when selected is true', () => { const {foundation, mockAdapter} = setupTest(); foundation.setSelected(true); td.verify(mockAdapter.notifySelection(true)); }); -test('#setSelected removes calls adapter.notifySelection when selected is false', () => { +test('#setSelected notifies of unselection when selected is false', () => { const {foundation, mockAdapter} = setupTest(); foundation.setSelected(false); td.verify(mockAdapter.notifySelection(false)); }); +test('#setSelectedFromChipset does not notify', () => { + const {foundation, mockAdapter} = setupTest(); + foundation.setSelectedFromChipset(false); + foundation.setSelectedFromChipset(true); + td.verify(mockAdapter.notifySelection(td.matchers.isA(Boolean)), {times: 0}); +}); + test('#getDimensions returns adapter.getRootBoundingClientRect when there is no checkmark bounding rect', () => { const {foundation, mockAdapter} = setupTest(); td.when(mockAdapter.getCheckmarkBoundingClientRect()).thenReturn(null); diff --git a/test/unit/mdc-chips/mdc-chip.test.js b/test/unit/mdc-chips/mdc-chip.test.js index efcb4b17558..8e19c4f9c58 100644 --- a/test/unit/mdc-chips/mdc-chip.test.js +++ b/test/unit/mdc-chips/mdc-chip.test.js @@ -420,6 +420,12 @@ test('#set shouldRemoveOnTrailingIconClick proxies to foundation', () => { td.verify(mockFoundation.setShouldRemoveOnTrailingIconClick(false)); }); +test('#setSelectedFromChipset proxies to the same foundation method', () => { + const {component, mockFoundation} = setupMockFoundationTest(); + component.setSelectedFromChipset(true); + td.verify(mockFoundation.setSelectedFromChipset(true)); +}); + test('#beginExit proxies to foundation', () => { const {component, mockFoundation} = setupMockFoundationTest(); component.beginExit(); From 0e1571777c4bc340e5ed917ef0084b4312baf5d9 Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 3 Jul 2019 10:38:36 -0700 Subject: [PATCH 2/4] WIP update readme --- packages/mdc-chips/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index 103aedda61f..f6031dc62f3 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -310,7 +310,7 @@ Method Signature | Description `focusPrimaryAction() => void` | Proxies to the foundation's `focusPrimaryAction` method `focusTrailingAction() => void` | Proxies to the foundation's `focusTrailingAction` method `removeFocus() => void` | Proxies to the foundation's `removeFocus` method -`setSelectedFromChipset(selected: boolean) => void` | Proxies to the foudnation's `setSelectedFromChipset` method (only called from the chip set) +`setSelectedFromChipset(selected: boolean) => void` | Proxies to the foundation's `setSelectedFromChipset` method (only called from the chip set) Property | Value Type | Description --- | --- | --- From dee380affdef792354690ef76d518526521c76aa Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 3 Jul 2019 10:55:03 -0700 Subject: [PATCH 3/4] WIP update from review --- packages/mdc-chips/README.md | 6 +++--- packages/mdc-chips/chip-set/component.ts | 2 +- packages/mdc-chips/chip/component.ts | 4 ++-- packages/mdc-chips/chip/foundation.ts | 2 +- test/unit/mdc-chips/mdc-chip-set.test.js | 2 +- test/unit/mdc-chips/mdc-chip.test.js | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/mdc-chips/README.md b/packages/mdc-chips/README.md index f6031dc62f3..0677ca2dfee 100644 --- a/packages/mdc-chips/README.md +++ b/packages/mdc-chips/README.md @@ -310,7 +310,7 @@ Method Signature | Description `focusPrimaryAction() => void` | Proxies to the foundation's `focusPrimaryAction` method `focusTrailingAction() => void` | Proxies to the foundation's `focusTrailingAction` method `removeFocus() => void` | Proxies to the foundation's `removeFocus` method -`setSelectedFromChipset(selected: boolean) => void` | Proxies to the foundation's `setSelectedFromChipset` method (only called from the chip set) +`setSelectedFromChipSet(selected: boolean) => void` | Proxies to the foundation's `setSelectedFromChipset` method (only called from the chip set) Property | Value Type | Description --- | --- | --- @@ -393,7 +393,7 @@ Method Signature | Description --- | --- `hasClass(className: string) => boolean` | Returns whether the chip set element has the given class `removeChipAtIndex(index: number) => void` | Removes the chip with the given `index` from the chip set -`selectChipAtIndex(index: string, selected: boolean) => void` | Calls `MDCChip#setSelectedFromChipset(selected)` on the chip at the given `index` +`selectChipAtIndex(index: string, selected: boolean) => void` | Calls `MDCChip#setSelectedFromChipSet(selected)` on the chip at the given `index` `getIndexOfChipById(id: string) => number` | Returns the index of the chip with the matching `id` or -1 `focusChipPrimaryActionAtIndex(index: number) => void` | Calls `MDCChip#focusPrimaryAction()` on the chip at the given `index` `focusChipTrailingActionAtIndex(index: number) => void` | Calls `MDCChip#focusTrailingAction()` on the chip at the given `index` @@ -409,7 +409,7 @@ Method Signature | Description --- | --- `isSelected() => boolean` | Returns true if the chip is selected `setSelected(selected: boolean) => void` | Sets the chip's selected state -`setSelectedFromChipset(selected: boolean) => void` | Sets the chip's selected state (called from the chip set) +`setSelectedFromChipSet(selected: boolean) => void` | Sets the chip's selected state (called from the chip set) `getShouldRemoveOnTrailingIconClick() => boolean` | Returns whether a trailing icon click should trigger exit/removal of the chip `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. diff --git a/packages/mdc-chips/chip-set/component.ts b/packages/mdc-chips/chip-set/component.ts index 6e013d965aa..cda345832c7 100644 --- a/packages/mdc-chips/chip-set/component.ts +++ b/packages/mdc-chips/chip-set/component.ts @@ -132,7 +132,7 @@ export class MDCChipSet extends MDCComponent { }, selectChipAtIndex: (index, selected) => { if (index >= 0 && index < this.chips_.length) { - this.chips_[index].setSelectedFromChipset(selected); + this.chips_[index].setSelectedFromChipSet(selected); } }, }; diff --git a/packages/mdc-chips/chip/component.ts b/packages/mdc-chips/chip/component.ts index 5a41c41e44b..6fa34be3a2a 100644 --- a/packages/mdc-chips/chip/component.ts +++ b/packages/mdc-chips/chip/component.ts @@ -217,8 +217,8 @@ export class MDCChip extends MDCComponent implements MDCRippl return new MDCChipFoundation(adapter); } - setSelectedFromChipset(selected: boolean) { - this.foundation_.setSelectedFromChipset(selected); + setSelectedFromChipSet(selected: boolean) { + this.foundation_.setSelectedFromChipSet(selected); } focusPrimaryAction() { diff --git a/packages/mdc-chips/chip/foundation.ts b/packages/mdc-chips/chip/foundation.ts index c5d3a228ffc..371260d891f 100644 --- a/packages/mdc-chips/chip/foundation.ts +++ b/packages/mdc-chips/chip/foundation.ts @@ -87,7 +87,7 @@ export class MDCChipFoundation extends MDCFoundation { this.setSelected_(selected, true); } - setSelectedFromChipset(selected: boolean) { + setSelectedFromChipSet(selected: boolean) { this.setSelected_(selected, false); } diff --git a/test/unit/mdc-chips/mdc-chip-set.test.js b/test/unit/mdc-chips/mdc-chip-set.test.js index a102642309f..65b558796c8 100644 --- a/test/unit/mdc-chips/mdc-chip-set.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.test.js @@ -182,7 +182,7 @@ test('#adapter.selectChipAtIndex calls setSelectedFromChipset on chip object', ( const {component} = setupTest(); const chip = component.chips[0]; component.getDefaultFoundation().adapter_.selectChipAtIndex(0, true); - td.verify(chip.setSelectedFromChipset(true)); + td.verify(chip.setSelectedFromChipSet(true)); }); test('#adapter.getChipListCount returns the number of chips', () => { diff --git a/test/unit/mdc-chips/mdc-chip.test.js b/test/unit/mdc-chips/mdc-chip.test.js index 8e19c4f9c58..b1ab9f87191 100644 --- a/test/unit/mdc-chips/mdc-chip.test.js +++ b/test/unit/mdc-chips/mdc-chip.test.js @@ -422,7 +422,7 @@ test('#set shouldRemoveOnTrailingIconClick proxies to foundation', () => { test('#setSelectedFromChipset proxies to the same foundation method', () => { const {component, mockFoundation} = setupMockFoundationTest(); - component.setSelectedFromChipset(true); + component.setSelectedFromChipSet(true); td.verify(mockFoundation.setSelectedFromChipset(true)); }); From d03e55de84b10c5dab43982fd246eb3d4e22c5fd Mon Sep 17 00:00:00 2001 From: Patrick RoDee Date: Wed, 3 Jul 2019 11:52:29 -0700 Subject: [PATCH 4/4] WIP update from review --- test/unit/mdc-chips/mdc-chip-set.test.js | 4 ++-- test/unit/mdc-chips/mdc-chip.foundation.test.js | 6 +++--- test/unit/mdc-chips/mdc-chip.test.js | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/unit/mdc-chips/mdc-chip-set.test.js b/test/unit/mdc-chips/mdc-chip-set.test.js index 65b558796c8..ffa11779711 100644 --- a/test/unit/mdc-chips/mdc-chip-set.test.js +++ b/test/unit/mdc-chips/mdc-chip-set.test.js @@ -57,7 +57,7 @@ class FakeChip { this.focusTrailingAction = td.func('.focusTrailingAction'); this.remove = td.func('.remove'); this.removeFocus = td.func('.removeFocus'); - this.setSelectedFromChipset = td.func('.setSelectedFromChipset'); + this.setSelectedFromChipSet = td.func('.setSelectedFromChipSet'); this.selected = false; } } @@ -178,7 +178,7 @@ test('#adapter.removeChipAtIndex does nothing if the given object is not in the assert.equal(component.chips.length, 3); }); -test('#adapter.selectChipAtIndex calls setSelectedFromChipset on chip object', () => { +test('#adapter.selectChipAtIndex calls setSelectedFromChipSet on chip object', () => { const {component} = setupTest(); const chip = component.chips[0]; component.getDefaultFoundation().adapter_.selectChipAtIndex(0, true); diff --git a/test/unit/mdc-chips/mdc-chip.foundation.test.js b/test/unit/mdc-chips/mdc-chip.foundation.test.js index ccb49e013b9..76dbf581f3f 100644 --- a/test/unit/mdc-chips/mdc-chip.foundation.test.js +++ b/test/unit/mdc-chips/mdc-chip.foundation.test.js @@ -104,10 +104,10 @@ test('#setSelected notifies of unselection when selected is false', () => { td.verify(mockAdapter.notifySelection(false)); }); -test('#setSelectedFromChipset does not notify', () => { +test('#setSelectedFromChipSet does not notify', () => { const {foundation, mockAdapter} = setupTest(); - foundation.setSelectedFromChipset(false); - foundation.setSelectedFromChipset(true); + foundation.setSelectedFromChipSet(false); + foundation.setSelectedFromChipSet(true); td.verify(mockAdapter.notifySelection(td.matchers.isA(Boolean)), {times: 0}); }); diff --git a/test/unit/mdc-chips/mdc-chip.test.js b/test/unit/mdc-chips/mdc-chip.test.js index b1ab9f87191..671b1768557 100644 --- a/test/unit/mdc-chips/mdc-chip.test.js +++ b/test/unit/mdc-chips/mdc-chip.test.js @@ -420,10 +420,10 @@ test('#set shouldRemoveOnTrailingIconClick proxies to foundation', () => { td.verify(mockFoundation.setShouldRemoveOnTrailingIconClick(false)); }); -test('#setSelectedFromChipset proxies to the same foundation method', () => { +test('#setSelectedFromChipSet proxies to the same foundation method', () => { const {component, mockFoundation} = setupMockFoundationTest(); component.setSelectedFromChipSet(true); - td.verify(mockFoundation.setSelectedFromChipset(true)); + td.verify(mockFoundation.setSelectedFromChipSet(true)); }); test('#beginExit proxies to foundation', () => {