Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Restoring of old selection does not use compareWith #215

Closed
probert94 opened this issue Mar 4, 2020 · 4 comments · Fixed by #216
Closed

[BUG] Restoring of old selection does not use compareWith #215

probert94 opened this issue Mar 4, 2020 · 4 comments · Fixed by #216
Labels
bug Something isn't working
Milestone

Comments

@probert94
Copy link
Contributor

Describe the bug
In mat-select-search.component.ts#L552, the previously selected values are restored, if they have been removed by the new filter.
However, the used logic does not use the compareWith-function of the mat-select and so different instances of the same object cause problems.
In my case, if I search for a value I make a request to the server, resulting in new instances of the objects. If I deselect an already selected object, it won't be removed. Additionally, selected objects, which match the current search term, are added again, resulting in two instances being part of the value.

To Reproduce
Here is a stackblitz based on ngx-mat-select-search-example

  1. Open the select and select the first 2 banks (Bank A (Switzerland) and Bank B (Switzerland) ).
  2. Search for Switzerland
  3. Deselect Bank A (Switzerland)
  4. In the Selected Banks Bank A (Switzerland) is still selected, Bank B (Switzerland) is selected twice.

Expected behavior
The mat-select-search should use the provided compareWith-Function to check if two objects are the same.

Additional Information
It should be enough to change mat-select-search#L553 from

if (values.indexOf(previousValue) === -1 && optionValues.indexOf(previousValue) === -1)

to

if (values.findIndex(v => this.matSelect.compareWith(v, previousValue)) === -1 && optionValues.findIndex(v => this.matSelect.compareWith(v, previousValue)) === -1)
@macjohnny
Copy link
Member

thanks for reporting and the detailed analysis!

macjohnny added a commit that referenced this issue Mar 4, 2020
…ple` (#216)

* #215: fix selection of different instances of same object when using `multiple`

* Apply suggestions from code review

Co-authored-by: Esteban Gehring <esteban.gehring@gmail.com>
@macjohnny macjohnny added this to the 2.1.2 milestone Mar 4, 2020
@macjohnny
Copy link
Member

@Springrbua thanks for your fix. it was released with version 2.1.2

@saithis
Copy link

saithis commented Mar 4, 2020

@Springrbua Thanks a lot! We had the same problem yesterday and I wanted to report it now, just to see that you already fixed it :) You are awesome.

@probert94
Copy link
Contributor Author

@macjohnny than you for the quick response and the new release :)
@saithis I am glad that I could contribute to this project :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants