fix(list): selection list always firing change event for selectAll and deselectAll #11029
Conversation
googlebot
added
the
cla: yes
src/lib/list/selection-list.ts
Outdated
| * Sets the selected state on all of the options | ||
| * and emits an event if anything changed. | ||
| */ | ||
| private _toggleAll(isSelected: boolean) { |
There was a problem hiding this comment.
How about _setAllOptionsSelected? To me "toggle" means specifically flipping from what it was before.
src/lib/list/selection-list.ts
Outdated
| * and emits an event if anything changed. | ||
| */ | ||
| private _toggleAll(isSelected: boolean) { | ||
| let hasChanged = false; |
There was a problem hiding this comment.
Add a comment like
// Keep track of whether anything changed because we only want to emit
// a change event if something actually changed.
…d deselectAll Currently selection list will always fire its model change event when invoking `selectAll` and `deselectAll`, even if the state of any of the options hasn't changed. These changes ensure that it only gets fired when something has changed.
crisbeto
force-pushed
the
selection-list-toggle-all-events
branch
from
49ca718 to
f1a54f1
Compare
|
Addressed the feedback @jelbourn. |
jelbourn
added
pr: lgtm
action: merge
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently selection list will always fire its model change event when invoking
selectAllanddeselectAll, even if the state of any of the options hasn't changed. These changes ensure that it only gets fired when something has changed.