fix(cdk-experimental/ui-patterns): add missing event handlers #30786
Conversation
wagnermaciel
commented
Apr 2, 2025
•
wagnermaciel
commented
- Add missing event handlers for alternative multiselection strategy
wagnermaciel
requested review from
crisbeto and
mmalerba
and removed request for
a team
| @@ -60,7 +60,14 @@ export class ListSelection<T extends ListSelectionItem<V>, V> { | |||
|
|
|||
| // TODO: Need to discuss when to drop this. | |||
| this._anchor(); | |||
| this.inputs.value.update(values => values.concat(item.value())); | |||
|
|
|||
| const orderedValues = []; | |||
There was a problem hiding this comment.
Are we sure this is the behavior we want? It means someone can't build a "pick your top 3" listbox that actually ranks the first, second, third place choices
There was a problem hiding this comment.
Hmm, interesting. I'm not sure how important it is to maintain the order of the source options. I believe it was Kristiyan who proposed this change. @crisbeto could you weigh in here?
There was a problem hiding this comment.
The context here is that mat-select used to keep the selection order and we had to introduce the sorting, because users found it confusing that going into a multi-select and checking a bunch of stuff could lead to different results each time. There might also be some implications for the data the app submits to the server.
FWIW in mat-select we also have an input that allows users to customize the sorting which we could introduce here.
There was a problem hiding this comment.
I think I'll leave it unsorted for now and wait for feedback from the community on this one. It's easier to add this feature in the future than it would be to remove it. I'm also not convinced we should add a hook for sorting since signals makes it trivial for devs to implement this by just using computed
There was a problem hiding this comment.
Yeah I think it depends on what you're implementing, for a standard <select multiple> there's no meaning to the order, so it makes sense to order them, but it feels like something that should be up to the developer based on the use case.
wagnermaciel
added
the
target: major
wagnermaciel
force-pushed
the
listbox-selection
branch
from
603ebd1 to
be499e6
Compare
* Add missing event handlers for alternative multiselection strategy
wagnermaciel
force-pushed
the
listbox-selection
branch
from
f042258 to
8c1e1b4
Compare
wagnermaciel
changed the title
…r#30786) * Add missing event handlers for alternative multiselection strategy
