fix(selection-list): remove selected option from model value on destroy #9106
Conversation
googlebot
added
the
cla: yes
crisbeto
changed the title
crisbeto
force-pushed
the
selection-list-selected-destroy
branch
from
bbb11a6 to
473890b
Compare
src/lib/list/selection-list.ts
Outdated
| // to avoid changed after checked errors. | ||
| Promise.resolve().then(() => { | ||
| this._setSelected(false); | ||
| this.selectionList._reportValueChange(); |
There was a problem hiding this comment.
How about moving that into a single line and just doing this.selected = false?
There was a problem hiding this comment.
That feels a little side-effect-ey since we're basically using a property as a method.
There was a problem hiding this comment.
Why is that? I think the setter's main intention is to update the internal state and reflect it within in the model. Otherwise the setter wouldn't be needed at all.
Promise.resolve().then(() => this.selected = false);There was a problem hiding this comment.
True, although I think of it mostly if you're setting it from the outside. That being said it does avoid having to duplicate some of the logic. I've switched it.
Fixes selected options that are removed from the DOM not being removed from the model. Relates to angular#9104.
crisbeto
force-pushed
the
selection-list-selected-destroy
branch
from
473890b to
4fdb546
Compare
devversion
added
pr: lgtm
action: merge
…oy (angular#9106) Fixes selected options that are removed from the DOM not being removed from the model. Relates to angular#9104.
…oy (angular#9106) Fixes selected options that are removed from the DOM not being removed from the model. Relates to angular#9104.
|
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. |
Fixes selected options that are removed from the DOM not being removed from the model.
Relates to #9104.