Skip to content

Commit

Permalink
fix(cdk/collections): SelectionModel does not always respect the comp…
Browse files Browse the repository at this point in the history
…areWith function (#26447)

Fixed bug in SelectionModel where compareWith function was not consistently respected in deselect method.

Fixes #25878

(cherry picked from commit 50ec808)
  • Loading branch information
FrenchTechLead authored and crisbeto committed Jan 27, 2023
1 parent f6ff503 commit d912416
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
26 changes: 17 additions & 9 deletions src/cdk/collections/selection-model.ts
Expand Up @@ -131,15 +131,7 @@ export class SelectionModel<T> {
* Determines whether a value is selected.
*/
isSelected(value: T): boolean {
if (this.compareWith) {
for (const otherValue of this._selection) {
if (this.compareWith(otherValue, value)) {
return true;
}
}
return false;
}
return this._selection.has(value);
return this._selection.has(this._getConcreteValue(value));
}

/**
Expand Down Expand Up @@ -191,6 +183,7 @@ export class SelectionModel<T> {

/** Selects a value. */
private _markSelected(value: T) {
value = this._getConcreteValue(value);
if (!this.isSelected(value)) {
if (!this._multiple) {
this._unmarkAll();
Expand All @@ -208,6 +201,7 @@ export class SelectionModel<T> {

/** Deselects a value. */
private _unmarkSelected(value: T) {
value = this._getConcreteValue(value);
if (this.isSelected(value)) {
this._selection.delete(value);

Expand Down Expand Up @@ -238,6 +232,20 @@ export class SelectionModel<T> {
private _hasQueuedChanges() {
return !!(this._deselectedToEmit.length || this._selectedToEmit.length);
}

/** Returns a value that is comparable to inputValue by applying compareWith function, returns the same inputValue otherwise. */
private _getConcreteValue(inputValue: T): T {
if (!this.compareWith) {
return inputValue;
} else {
for (let selectedValue of this._selection) {
if (this.compareWith!(inputValue, selectedValue)) {
return selectedValue;
}
}
return inputValue;
}
}
}

/**
Expand Down
20 changes: 20 additions & 0 deletions src/cdk/collections/selection.spec.ts
Expand Up @@ -279,4 +279,24 @@ describe('SelectionModel', () => {
let singleSelectionModel = new SelectionModel();
expect(singleSelectionModel.isMultipleSelection()).toBe(false);
});

it('should deselect value if comparable to another one', () => {
type Item = {key: number; value: string};
const v1: Item = {key: 1, value: 'blue'};
const v2: Item = {key: 1, value: 'green'};
const compareFun = (x: Item, y: Item) => x.key === y.key;
const model = new SelectionModel<Item>(false, [v1], false, compareFun);
model.deselect(v2);
expect(model.selected.length).toBe(0);
});

it('should not deselect value if not comparable to another one', () => {
type Item = {key: number; value: string};
const v1: Item = {key: 1, value: 'blue'};
const v2: Item = {key: 2, value: 'apple'};
const compareFun = (x: Item, y: Item) => x.key === y.key;
const model = new SelectionModel<Item>(false, [v1], false, compareFun);
model.deselect(v2);
expect(model.selected.length).toBe(1);
});
});

0 comments on commit d912416

Please sign in to comment.