-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(cdk-experimental/ui-patterns): add grid selection behavior #31320
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
base: main
Are you sure you want to change the base?
Conversation
feat(cdk-experimental/ui-patterns): grid selection behavior
|
||
/** Deselects all items in the grid. */ | ||
deselectAll() { | ||
for (const row of this.inputs.cells()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An issue I had with the similar approach in list selection was if there's a pre-selected value in this.inputs.value
, but the corresponding item/cell is lazily loaded(collapsed) and does not appear in the this.inputs.(items|cells)
, then the deselectAll can't deselect that value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm how did you work around this? If we take the current approach then the value doesn't get properly cleared, but if we make it so that deselectAll
removes values that don't correspond to an existing cell/item we may be changing the state of disabled cells/items
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I figured removing everything like this.inputs.value.set([]);
is not a perfect solution for keeping the disabled items selected. I don't have a workaround so it's still bugged for tree.
One potential solution could be restricting a selection value to be a complex type. E.g.
interface Selectable<V> {
value: V;
disabled: boolean;
}
this also allows items to have the same value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having to pass in complex types for selection won't work. It's probably best to go with deselecting any values that are not found in the items/cells. I made the change and added a unit test for it
No description provided.