Skip to content

Commit

Permalink
fix(chips): Ignore selection events in chip set (#4878)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickrodee committed Jul 31, 2019
1 parent bfed33d commit 94c6a00
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 72 deletions.
8 changes: 4 additions & 4 deletions packages/mdc-chips/README.md
Expand Up @@ -365,7 +365,7 @@ Method Signature | Description
`removeClassFromLeadingIcon(className: string) => void` | Removes a class from the leading icon element
`eventTargetHasClass(target: EventTarget, className: string) => boolean` | Returns true if target has className, false otherwise
`notifyInteraction() => void` | Notifies the Chip Set that the chip has been interacted with\*
`notifySelection(selected) => void` | Notifies the Chip Set that the chip has been selected or deselected\*\*
`notifySelection(selected: boolean, chipSetShouldIgnore: boolean) => void` | Notifies the Chip Set that the chip has been selected or deselected\*\*. When `chipSetShouldIgnore` is `true`, the chip set does not process the event.
`notifyTrailingIconInteraction() => void` | Notifies the Chip Set that the chip's trailing icon has been interacted with\*
`notifyRemoval() => void` | Notifies the Chip Set that the chip will be removed\*\*\*
`getComputedStyleValue(propertyName: string) => string` | Returns the computed property value of the given style property on the root element
Expand Down Expand Up @@ -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, shouldNotifyClients: boolean) => void` | Calls `MDCChip#setSelectedFromChipSet(selected)` on the chip at the given `index`. Will emit a selection event if called with `shouldNotifyClients` set to `true`. The emitted selection event will be ignored by the `MDCChipSetFoundation`.
`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 All @@ -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, shouldNotifyClients: boolean) => void` | Sets the chip's selected state (called from the chip set) to the `selected` param. Will emit a selection event if called with `shouldNotifyClients` set to `true`. The emitted selection event will be ignored by the `MDCChipSetFoundation`.
`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.
Expand Down Expand Up @@ -438,7 +438,7 @@ Method Signature | Description
`getSelectedChipIds() => ReadonlyArray<string>` | Returns an array of the IDs of all selected chips
`select(chipId: string) => void` | Selects the chip with the given id
`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
`handleChipSelection(chipId: string, selected: boolean, chipSetShouldIgnore: boolean) => void` | Handles a custom `MDCChip:selection` event on the root element. When `chipSetShouldIgnore` is true, the chip set does not process the event.
`handleChipRemoval(chipId: string) => void` | Handles a custom `MDCChip:removal` event on the root element
`handleChipNavigation(chipId: string, key: string) => void` | Handles a custom `MDCChip:navigation` event on the root element

Expand Down
2 changes: 1 addition & 1 deletion packages/mdc-chips/chip-set/adapter.ts
Expand Up @@ -43,7 +43,7 @@ export interface MDCChipSetAdapter {
/**
* Sets the selected state of the chip at the given index.
*/
selectChipAtIndex(index: number, selected: boolean): void;
selectChipAtIndex(index: number, isSelected: boolean, shouldNotifyClients: boolean): void;

/**
* Returns the index of the chip at the given ID.
Expand Down
8 changes: 5 additions & 3 deletions packages/mdc-chips/chip-set/component.ts
Expand Up @@ -73,7 +73,9 @@ export class MDCChipSet extends MDCComponent<MDCChipSetFoundation> {
});

this.handleChipInteraction_ = (evt) => this.foundation_.handleChipInteraction(evt.detail.chipId);
this.handleChipSelection_ = (evt) => this.foundation_.handleChipSelection(evt.detail.chipId, evt.detail.selected);
this.handleChipSelection_ = (evt) => {
this.foundation_.handleChipSelection(evt.detail.chipId, evt.detail.selected, evt.detail.shouldIgnore);
};
this.handleChipRemoval_ = (evt) => this.foundation_.handleChipRemoval(evt.detail.chipId);
this.handleChipNavigation_ = (evt) => this.foundation_.handleChipNavigation(
evt.detail.chipId, evt.detail.key, evt.detail.source);
Expand Down Expand Up @@ -130,9 +132,9 @@ export class MDCChipSet extends MDCComponent<MDCChipSetFoundation> {
removeFocusFromChipAtIndex: (index) => {
this.chips_[index].removeFocus();
},
selectChipAtIndex: (index, selected) => {
selectChipAtIndex: (index, selected, shouldNotifyClients) => {
if (index >= 0 && index < this.chips_.length) {
this.chips_[index].setSelectedFromChipSet(selected);
this.chips_[index].setSelectedFromChipSet(selected, shouldNotifyClients);
}
},
};
Expand Down
54 changes: 34 additions & 20 deletions packages/mdc-chips/chip-set/foundation.ts
Expand Up @@ -67,21 +67,10 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {

/**
* Selects the chip with the given id. Deselects all other chips if the chip set is of the choice variant.
* Does not notify clients of the updated selection state.
*/
select(chipId: string) {
if (this.selectedChipIds_.indexOf(chipId) >= 0) {
return;
}

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_.selectChipAtIndex(previouslySelectedIndex, false);
}
this.selectedChipIds_.push(chipId);
const index = this.adapter_.getIndexOfChipById(chipId);
this.adapter_.selectChipAtIndex(index, true);
this.select_(chipId, false);
}

/**
Expand All @@ -98,12 +87,17 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
/**
* Handles a chip selection event, used to handle discrepancy when selection state is set directly on the Chip.
*/
handleChipSelection(chipId: string, selected: boolean) {
handleChipSelection(chipId: string, selected: boolean, shouldIgnore: boolean) {
// Early exit if we should ignore the event
if (shouldIgnore) {
return;
}

const chipIsSelected = this.selectedChipIds_.indexOf(chipId) >= 0;
if (selected && !chipIsSelected) {
this.select(chipId);
} else if (!selected && chipIsSelected) {
this.deselect_(chipId);
this.deselectAndNotifyClients_(chipId);
}
}

Expand All @@ -112,7 +106,7 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
*/
handleChipRemoval(chipId: string) {
const index = this.adapter_.getIndexOfChipById(chipId);
this.deselect_(chipId);
this.deselectAndNotifyClients_(chipId);
this.adapter_.removeChipAtIndex(index);
const maxIndex = this.adapter_.getChipListCount() - 1;
const nextIndex = Math.min(index, maxIndex);
Expand Down Expand Up @@ -189,12 +183,12 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
/**
* Deselects the chip with the given id.
*/
private deselect_(chipId: string) {
private deselectAndNotifyClients_(chipId: string) {
const index = this.selectedChipIds_.indexOf(chipId);
if (index >= 0) {
this.selectedChipIds_.splice(index, 1);
const chipIndex = this.adapter_.getIndexOfChipById(chipId);
this.adapter_.selectChipAtIndex(chipIndex, false);
this.adapter_.selectChipAtIndex(chipIndex, /** isSelected */ false, /** shouldNotifyClients */ true);
}
}

Expand All @@ -203,9 +197,9 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
*/
private toggleSelect_(chipId: string) {
if (this.selectedChipIds_.indexOf(chipId) >= 0) {
this.deselect_(chipId);
this.deselectAndNotifyClients_(chipId);
} else {
this.select(chipId);
this.selectAndNotifyClients_(chipId);
}
}

Expand All @@ -217,6 +211,26 @@ export class MDCChipSetFoundation extends MDCFoundation<MDCChipSetAdapter> {
}
}
}

private selectAndNotifyClients_(chipId: string) {
this.select_(chipId, true);
}

private select_(chipId: string, shouldNotifyClients: boolean) {
if (this.selectedChipIds_.indexOf(chipId) >= 0) {
return;
}

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

// tslint:disable-next-line:no-default-export Needed for backward compatibility with MDC Web v0.44.0 and earlier.
Expand Down
4 changes: 2 additions & 2 deletions packages/mdc-chips/chip/adapter.ts
Expand Up @@ -70,7 +70,7 @@ export interface MDCChipAdapter {
/**
* Emits a custom "MDCChip:selection" event denoting the chip has been selected or deselected.
*/
notifySelection(selected: boolean): void;
notifySelection(selected: boolean, chipSetShouldIgnore: boolean): void;

/**
* Emits a custom "MDCChip:trailingIconInteraction" event denoting the trailing icon has been
Expand Down Expand Up @@ -129,7 +129,7 @@ export interface MDCChipAdapter {
hasTrailingAction(): boolean;

/**
* Sets the the attribute on the trailing action if it exists.
* Sets the attribute on the trailing action if it exists.
*/
setTrailingActionAttr(attr: string, value: string): void;

Expand Down
8 changes: 4 additions & 4 deletions packages/mdc-chips/chip/component.ts
Expand Up @@ -192,8 +192,8 @@ export class MDCChip extends MDCComponent<MDCChipFoundation> implements MDCRippl
this.emit<MDCChipRemovalEventDetail>(
strings.REMOVAL_EVENT, {chipId: this.id, root: this.root_}, true /* shouldBubble */);
},
notifySelection: (selected) => this.emit<MDCChipSelectionEventDetail>(
strings.SELECTION_EVENT, {chipId: this.id, selected}, true /* shouldBubble */),
notifySelection: (selected, shouldIgnore) => this.emit<MDCChipSelectionEventDetail>(
strings.SELECTION_EVENT, {chipId: this.id, selected, shouldIgnore}, true /* shouldBubble */),
notifyTrailingIconInteraction: () => this.emit<MDCChipInteractionEventDetail>(
strings.TRAILING_ICON_INTERACTION_EVENT, {chipId: this.id}, true /* shouldBubble */),
removeClass: (className) => this.root_.classList.remove(className),
Expand All @@ -217,8 +217,8 @@ export class MDCChip extends MDCComponent<MDCChipFoundation> implements MDCRippl
return new MDCChipFoundation(adapter);
}

setSelectedFromChipSet(selected: boolean) {
this.foundation_.setSelectedFromChipSet(selected);
setSelectedFromChipSet(selected: boolean, shouldNotifyClients: boolean) {
this.foundation_.setSelectedFromChipSet(selected, shouldNotifyClients);
}

focusPrimaryAction() {
Expand Down
22 changes: 15 additions & 7 deletions packages/mdc-chips/chip/foundation.ts
Expand Up @@ -84,11 +84,15 @@ export class MDCChipFoundation extends MDCFoundation<MDCChipAdapter> {
}

setSelected(selected: boolean) {
this.setSelected_(selected, true);
this.setSelected_(selected);
this.notifySelection_(selected);
}

setSelectedFromChipSet(selected: boolean) {
this.setSelected_(selected, false);
setSelectedFromChipSet(selected: boolean, shouldNotifyClients: boolean) {
this.setSelected_(selected);
if (shouldNotifyClients) {
this.notifyIgnoredSelection_(selected);
}
}

getShouldRemoveOnTrailingIconClick() {
Expand Down Expand Up @@ -325,18 +329,22 @@ export class MDCChipFoundation extends MDCFoundation<MDCChipAdapter> {
return isDeletable && (evt.key === strings.BACKSPACE_KEY || evt.key === strings.DELETE_KEY);
}

private setSelected_(selected: boolean, shouldNotify: boolean) {
private 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');
}
}

if (shouldNotify) {
this.adapter_.notifySelection(selected);
}
private notifySelection_(selected: boolean) {
this.adapter_.notifySelection(selected, false);
}

private notifyIgnoredSelection_(selected: boolean) {
this.adapter_.notifySelection(selected, true);
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/mdc-chips/chip/types.ts
Expand Up @@ -29,6 +29,7 @@ export interface MDCChipInteractionEventDetail {

export interface MDCChipSelectionEventDetail extends MDCChipInteractionEventDetail {
selected: boolean;
shouldIgnore: boolean;
}

export interface MDCChipRemovalEventDetail extends MDCChipInteractionEventDetail {
Expand Down

0 comments on commit 94c6a00

Please sign in to comment.