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

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

Fixes angular#25878
  • Loading branch information
FrenchTechLead committed Jan 27, 2023
1 parent c4414ad commit cf6a8dd
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 cf6a8dd

Please sign in to comment.