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 25, 2023
1 parent 834e865 commit c1b1823
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
24 changes: 21 additions & 3 deletions src/cdk/collections/selection-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,16 @@ export class SelectionModel<T> {

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

if (this.compareWith) {
comparableValue = this._getComparableValue(value) as T;
this._selection.delete(comparableValue);
} else {
this._selection.delete(value);
}
if (this._emitChanges) {
this._deselectedToEmit.push(value);
this._deselectedToEmit.push(comparableValue || value);
}
}
}
Expand All @@ -238,6 +243,19 @@ 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 null if none.*/
private _getComparableValue(inputValue: T): T | null {
let comparableValue: T | null = null;
if (this.compareWith) {
this._selection.forEach(selectedValue => {
if (this.compareWith?.call(undefined, inputValue, selectedValue)) {
comparableValue = selectedValue;
}
});
}
return comparableValue;
}
}

/**
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 T = {key: number; value: string};
const v1: T = {key: 1, value: 'blue'};
const v2: T = {key: 1, value: 'green'};
const compareFun = (x: T, y: T) => x.key === y.key;
const model = new SelectionModel<T>(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 T = {key: number; value: string};
const v1: T = {key: 1, value: 'blue'};
const v2: T = {key: 2, value: 'apple'};
const compareFun = (x: T, y: T) => x.key === y.key;
const model = new SelectionModel<T>(false, [v1], false, compareFun);
model.deselect(v2);
expect(model.selected.length).toBe(1);
});
});

0 comments on commit c1b1823

Please sign in to comment.