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.

Sign up for GitHub

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

Jump to bottom

In multiselect, (selectionChange) event of the control does not contain all the elements but just the newly selected #387

Closed
macjohnny opened this issue Jul 14, 2022 · 13 comments · Fixed by #388
Closed

In multiselect, (selectionChange) event of the control does not contain all the elements but just the newly selected #387

macjohnny opened this issue Jul 14, 2022 · 13 comments · Fixed by #388

Comments

@macjohnny
Copy link
Member

Hey, this fix just broke my behavior! By modifying a copy of values but not the original one the (selectionChange) event of the control does not contain all the elements but just the newly selected.

@qstiegler Can you show me your usecase where version 4.1.1 threw an exception for you so I can maybe fix this?

Originally posted by @angelaki in #376 (comment)
@macjohnny
Copy link
Member Author

@angelaki sorry for the issue

so the FormControl has the correct set of selexted values but (selectionChange) does not emit the correct set?

@angelaki
Copy link
Contributor

angelaki commented Jul 14, 2022 •
edited
Loading

@macjohnny yes, correct! Tbh this wasn't even an intended behavior of my initial implementation but modifying the original source passed the correct event value. Now the form is correct but the event broken.

@macjohnny
Copy link
Member Author

Hmm i wonder why, because in

ngx-mat-select-search/src/app/mat-select-search/mat-select-search.component.ts

Line 632 in 092aaf9

this.matSelect._onChange(updatedSelectedValues);
we set the correct values, but maybe because those options are not present while filtering, they get deselected (which is exactly the purpose of this workaround method, which to compensate for that)
Any suggestion how to fix that?

@angelaki
Copy link
Contributor

angelaki commented Jul 14, 2022 via email

@macjohnny macjohnny changed the title Hey, this fix just broke my behavior! By modifying a copy of values but not the original one the (selectionChange) event of the control does not contain all the elements but just the newly selected. In multiselect, (selectionChange) event of the control does not contain all the elements but just the newly selected Jul 14, 2022
@angelaki
Copy link
Contributor

angelaki commented Jul 14, 2022 •
edited
Loading

Ok, wow. Let's say: it's getting complicated. I made a fork to show the current misbehavior: https://github.com/angelaki/ngx-mat-select-search?organization=angelaki&organization=angelaki. The earlier version wasn't perfect at all, because the handler just manipulates the event value. It was more luck that it was the first handler called - but it somehow always is? I already tried toggling he search with *ngIf. The controls internal handler always seams to be called first. Lucky me ...

Never the less, we are already in the valueChanges cycle and I have no idea how to stop it without some super ugly private accesses etc. Imho the earlier solution was the best we can get. Why was it changed at all? How is it possible that @qstiegler array was readonly? Maybe we can find a solution for this. But anyway, I guess it's staying a bit quirky :/

@qstiegler
Copy link
Contributor

Hey guys, I'm sorry to hear about the negative side effect of the fix.

The initial error occurred when you have one or more selected, then you search for another value where the previous selected values are not part of the options anymore. When you select now the value, the previous selected values get unselected. I hope this helps.

@macjohnny
Copy link
Member Author

Ok so for the time being, I suggest we revert #377 and then analyze the situation of the readonly array

@angelaki
Copy link
Contributor

Yeah, think so,too. @qstiegler can you create a minimal reproduction of your behavior? Your description sounds like this should always happen. This just isn't the case, as you can see in the live examples: https://stackblitz.com/github/bithost-gmbh/ngx-mat-select-search-example?file=src%2Fapp%2Fapp.component.html

@macjohnny macjohnny mentioned this issue Jul 15, 2022
Merged
@qstiegler
Copy link
Contributor

qstiegler commented Jul 15, 2022 •
edited
Loading

@angelaki Puh...it took me a while today to create a working minimal reproduction but now I figured it out.

The problem is the combination with the NGXS store. When you push values to the store, they get immutable. This means that the next time when you select another value and you're adding the previousSelectedValues, it tries to manipulate the same value which is marked as immutable.

Here is the link to reproduce: https://stackblitz.com/edit/angular-ivy-5dffxg

  1. Select Option 1
  2. Search for 2
  3. Select Option 2
  4. The console output shows the error Cannot add property 1, object is not extensible and only the Option 2 is selected

I think in general if you do manipulations of the form value object, it always should be immutable, that's why I thought my solution would be good. I hope we find another one which fixes both.

As a short term solution I can create a copy of the array before I send it to the store but this is pretty prone to failure.

I hope this will help you guys!

@qstiegler
Copy link
Contributor

Is there a reason why you closed it? I really hope you'll come up with an immutable solution.

@angelaki
Copy link
Contributor

angelaki commented Jul 19, 2022 via email

@macjohnny
Copy link
Member Author

@qstiegler I reopened #376

@macjohnny
Copy link
Member Author

@angelaki this was released in 4.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@qstiegler @macjohnny @angelaki