Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
feat(chips): Use index for all chip operations (#4869)
Browse files Browse the repository at this point in the history 
Change interface for MDCChipSetAdapter#removeChip to MDCChipSetAdapter#removeChipAtIndex. Update comments for MDCChipSetAdapter.

BREAKING CHANGE: MDCChipSetAdapter#removeChip has been replaced with MDCChipSetAdapter#removeChipAtIndex. MDCChipSetAdapter#setSelected has been replaced with MDCChipSetAdapter#selectChipAtIndex
  • Loading branch information
@patrickrodee
patrickrodee authored Jul 2, 2019
1 parent 42065fe commit 07078bb
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 47 deletions.
4 changes: 2 additions & 2 deletions packages/mdc-chips/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ Method Signature | Description
Method Signature | Description
--- | ---
`hasClass(className: string) => boolean` | Returns whether the chip set element has the given class
`removeChip(chipId: string) => void` | Removes the chip with the given id from the chip set
`setSelected(chipId: string, selected: boolean) => void` | Sets the selected state of the chip with the given id
`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`
`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`
Expand Down
18 changes: 14 additions & 4 deletions packages/mdc-chips/chip-set/adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,33 @@ export interface MDCChipSetAdapter {
hasClass(className: string): boolean;

/**
* Removes the chip with the given id from the chip set.
* Removes the chip with the given index from the chip set.
* Make sure to remove it from the chip list, too.
*/
removeChip(chipId: string): void;
removeChipAtIndex(index: number): void;

/**
* Sets the selected state of the chip with the given id.
* Sets the selected state of the chip at the given index.
*/
setSelected(chipId: string, selected: boolean): void;
selectChipAtIndex(index: number, selected: boolean): void;

/**
* Returns the index of the chip at the given ID.
* @param chipId the unique ID of the chip
* @return the numerical index of the chip with the matching id or -1.
*/
getIndexOfChipById(chipId: string): number;

/**
* Calls Chip#focusPrimaryAction() on the chip at the given index.
* @param index the index of the chip
*/
focusChipPrimaryActionAtIndex(index: number): void;

/**
* Calls Chip#focusTrailingAction() on the chip at the given index.
* @param index the index of the chip
*/
focusChipTrailingActionAtIndex(index: number): void;

/**
Expand Down
10 changes: 4 additions & 6 deletions packages/mdc-chips/chip-set/component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,8 @@ export class MDCChipSet extends MDCComponent<MDCChipSetFoundation> {
},
hasClass: (className) => this.root_.classList.contains(className),
isRTL: () => window.getComputedStyle(this.root_).getPropertyValue('direction') === 'rtl',
removeChip: (chipId) => {
const index = this.findChipIndex_(chipId);
if (index >= 0) {
removeChipAtIndex: (index) => {
if (index >= 0 && index < this.chips_.length) {
this.chips_[index].destroy();
this.chips_[index].remove();
this.chips_.splice(index, 1);
Expand All @@ -131,9 +130,8 @@ export class MDCChipSet extends MDCComponent<MDCChipSetFoundation> {
removeFocusFromChipAtIndex: (index) => {
this.chips_[index].removeFocus();
},
setSelected: (chipId, selected) => {
const index = this.findChipIndex_(chipId);
if (index >= 0) {
selectChipAtIndex: (index, selected) => {
if (index >= 0 && index < this.chips_.length) {
this.chips_[index].selected = selected;
}
},
Expand Down
15 changes: 9 additions & 6 deletions packages/mdc-chips/chip-set/foundation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
getIndexOfChipById: () => -1,
hasClass: () => false,
isRTL: () => false,
removeChip: () => undefined,
removeChipAtIndex: () => undefined,
removeFocusFromChipAtIndex: () => undefined,
setSelected: () => undefined,
selectChipAtIndex: () => undefined,
};
}

Expand Down Expand Up @@ -75,11 +75,13 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {

if (this.adapter_.hasClass(cssClasses.CHOICE) && this.selectedChipIds_.length > 0) {
const previouslySelectedChip = this.selectedChipIds_[0];
const previouslySelectedIndex = this.adapter_.getIndexOfChipById(previouslySelectedChip);
this.selectedChipIds_.length = 0;
this.adapter_.setSelected(previouslySelectedChip, false);
this.adapter_.selectChipAtIndex(previouslySelectedIndex, false);
}
this.selectedChipIds_.push(chipId);
this.adapter_.setSelected(chipId, true);
const index = this.adapter_.getIndexOfChipById(chipId);
this.adapter_.selectChipAtIndex(index, true);
}

/**
Expand Down Expand Up @@ -111,7 +113,7 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
handleChipRemoval(chipId: string) {
const index = this.adapter_.getIndexOfChipById(chipId);
this.deselect_(chipId);
this.adapter_.removeChip(chipId);
this.adapter_.removeChipAtIndex(index);
const maxIndex = this.adapter_.getChipListCount() - 1;
const nextIndex = Math.min(index, maxIndex);
this.removeFocusFromChipsExcept_(nextIndex);
Expand Down Expand Up @@ -191,7 +193,8 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
const index = this.selectedChipIds_.indexOf(chipId);
if (index >= 0) {
this.selectedChipIds_.splice(index, 1);
this.adapter_.setSelected(chipId, false);
const chipIndex = this.adapter_.getIndexOfChipById(chipId);
this.adapter_.selectChipAtIndex(chipIndex, false);
}
}

Expand Down
47 changes: 24 additions & 23 deletions test/unit/mdc-chips/mdc-chip-set.foundation.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ test('exports cssClasses', () => {

test('defaultAdapter returns a complete adapter implementation', () => {
verifyDefaultAdapter(MDCChipSetFoundation, [
'hasClass', 'removeChip', 'setSelected',
'hasClass', 'removeChipAtIndex', 'selectChipAtIndex',
'focusChipPrimaryActionAtIndex', 'focusChipTrailingActionAtIndex',
'getIndexOfChipById', 'isRTL', 'getChipListCount',
'removeFocusFromChipAtIndex',
Expand All @@ -56,92 +56,92 @@ const setupTest = () => {
};

test('in choice chips, #select does nothing if chip is already selected', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true);
foundation.select('chipA');
foundation.select('chipA');
td.verify(mockAdapter.setSelected('chipA', true), {times: 1});
td.verify(mockAdapter.selectChipAtIndex(0, true), {times: 1});
assert.equal(foundation.getSelectedChipIds().length, 1);
});

test('in choice chips, #select selects chip if no chips are selected', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true);
assert.equal(foundation.getSelectedChipIds().length, 0);

foundation.select('chipA');
td.verify(mockAdapter.setSelected('chipA', true));
td.verify(mockAdapter.selectChipAtIndex(0, true));
assert.equal(foundation.getSelectedChipIds().length, 1);
});

test('in choice chips, #select deselects chip if another chip is selected', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true);
foundation.select('chipB');
assert.equal(foundation.getSelectedChipIds().length, 1);

foundation.select('chipA');
td.verify(mockAdapter.setSelected('chipB', false));
td.verify(mockAdapter.setSelected('chipA', true));
td.verify(mockAdapter.selectChipAtIndex(1, false));
td.verify(mockAdapter.selectChipAtIndex(0, true));
assert.equal(foundation.getSelectedChipIds().length, 1);
});

test('in filter chips, #select selects multiple chips', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true);
assert.equal(foundation.getSelectedChipIds().length, 0);

foundation.select('chipA');
td.verify(mockAdapter.setSelected('chipA', true));
td.verify(mockAdapter.selectChipAtIndex(0, true));
assert.equal(foundation.getSelectedChipIds().length, 1);

foundation.select('chipB');
td.verify(mockAdapter.setSelected('chipB', true));
td.verify(mockAdapter.selectChipAtIndex(1, true));
assert.equal(foundation.getSelectedChipIds().length, 2);
});

test('in filter chips, #select does nothing if chip is already selected', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(false);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true);
foundation.select('chipA');
foundation.select('chipA');
td.verify(mockAdapter.setSelected('chipA', true), {times: 1});
td.verify(mockAdapter.selectChipAtIndex(0, true), {times: 1});
assert.equal(foundation.getSelectedChipIds().length, 1);
});

test('in filter chips, #handleChipInteraction deselects chip if in selectedChipId', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true);
foundation.handleChipInteraction('chipA');
foundation.handleChipInteraction('chipB');
assert.equal(foundation.getSelectedChipIds().length, 2);

foundation.handleChipInteraction('chipB');
td.verify(mockAdapter.setSelected('chipB', false));
td.verify(mockAdapter.selectChipAtIndex(1, false));
assert.equal(foundation.getSelectedChipIds().length, 1);

foundation.handleChipInteraction('chipA');
td.verify(mockAdapter.setSelected('chipA', false));
td.verify(mockAdapter.selectChipAtIndex(0, false));
assert.equal(foundation.getSelectedChipIds().length, 0);
});

test('#handleChipInteraction selects chip if the chip set is a filter chip set', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(false);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(true);

foundation.handleChipInteraction('chipA');
td.verify(mockAdapter.setSelected('chipA', true));
td.verify(mockAdapter.selectChipAtIndex(0, true));
});

test('#handleChipInteraction selects chip if the chip set is a choice chip set', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(true);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(false);

foundation.handleChipInteraction('chipA');
td.verify(mockAdapter.setSelected('chipA', true));
td.verify(mockAdapter.selectChipAtIndex(0, true));
});

test('#handleChipInteraction removes focus from all chips except the selected one', () => {
Expand All @@ -156,12 +156,12 @@ test('#handleChipInteraction removes focus from all chips except the selected on
});

test('#handleChipInteraction does nothing if the chip set is neither choice nor filter', () => {
const {foundation, mockAdapter} = setupTest();
const {foundation, mockAdapter} = setupChipNavigationTest(['chipA', 'chipB', 'chipC']);
td.when(mockAdapter.hasClass(cssClasses.CHOICE)).thenReturn(false);
td.when(mockAdapter.hasClass(cssClasses.FILTER)).thenReturn(false);

foundation.handleChipInteraction('chipA');
td.verify(mockAdapter.setSelected('chipA', true), {times: 0});
td.verify(mockAdapter.selectChipAtIndex(0, true), {times: 0});
});

test('#handleChipSelection selects an unselected chip if selected is true', () => {
Expand Down Expand Up @@ -200,9 +200,10 @@ test('#handleChipSelection does nothing if selected is false and the chip is not

test('#handleChipRemoval removes chip', () => {
const {foundation, mockAdapter} = setupTest();
td.when(mockAdapter.getIndexOfChipById('chipA')).thenReturn(1);

foundation.handleChipRemoval('chipA');
td.verify(mockAdapter.removeChip('chipA'));
td.verify(mockAdapter.removeChipAtIndex(1));
});

test('#handleChipRemoval removes focus from all chips except the next one', () => {
Expand Down
12 changes: 6 additions & 6 deletions test/unit/mdc-chips/mdc-chip-set.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,25 +162,25 @@ test('#adapter.hasClass returns true if class is set on chip set element', () =>
assert.isTrue(component.getDefaultFoundation().adapter_.hasClass('foo'));
});

test('#adapter.removeChip removes the chip object from the chip set', () => {
test('#adapter.removeChipAtIndex removes the chip object from the chip set', () => {
const {component} = setupTest();
const chip = component.chips[0];
component.getDefaultFoundation().adapter_.removeChip(chip.id);
component.getDefaultFoundation().adapter_.removeChipAtIndex(0);
assert.equal(component.chips.length, 2);
td.verify(chip.destroy());
td.verify(chip.remove());
});

test('#adapter.removeChip does nothing if the given object is not in the chip set', () => {
test('#adapter.removeChipAtIndex does nothing if the given object is not in the chip set', () => {
const {component} = setupTest();
component.getDefaultFoundation().adapter_.removeChip('chip0');
component.getDefaultFoundation().adapter_.removeChipAtIndex(-1);
assert.equal(component.chips.length, 3);
});

test('#adapter.setSelected sets selected on chip object', () => {
test('#adapter.selectChipAtIndex sets selected on chip object', () => {
const {component} = setupTest();
const chip = component.chips[0];
component.getDefaultFoundation().adapter_.setSelected(chip.id, true);
component.getDefaultFoundation().adapter_.selectChipAtIndex(0, true);
assert.equal(chip.selected, true);
});

Expand Down

0 comments on commit 07078bb

Please sign in to comment.