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.

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

fix(forms): improve select performance and ensure that selected option matches the ngModel value #56459

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Jun 14, 2024

Rather than calling writeValue() over and over for individual option elements that are added/removed from the DOM, this change only writes to the value when it could actually be affected. This only happens when the currently selected item is changing (updated value or deleted) or if there is no currently a selected value and an option value changes (either via creation or a value update). We defer the update until after rendering is complete for two reasons: first, to avoid repeatedly calling writeValue on every option element until we find the selected one (could be the very last element). Second, to ensure that we perform the write after the DOM elements have been created (this doesn't happen until the end of change detection when animations are enabled).

This is needed to efficiently set the select value when adding/removing options. The previous approach resulted in exponentially more _compareValue calls than the number of option elements (issue #41330).

Finally, this PR fixes an issue with delayed element removal when using the animations module (in all browsers). Previously when a selected option was removed (so no option matched the ngModel anymore), Angular changed the select element value before actually removing the option from the DOM. Then when the option was finally removed from the DOM, the browser would change the select value to that of the first option, even though it didn't match the ngModel (issue #18430). Note that this is still somewhat of an application issue. The model value still needs to be updated to a valid value when the selected value is deleted or it will be out of sync with the DOM.

Fixes #41330, fixes #18430.

@ngbot ngbot bot added this to the Backlog milestone Jun 14, 2024
…n matches the ngModel value

Rather than calling `writeValue()` over and over for individual option elements that are
added/removed from the DOM, this change only writes to the value when it
could actually be affected. This only happens when the currently
selected item is changing (updated value or deleted) or if there is
_no_ currently a selected value and an option value changes (either via
creation or a value update). We defer the update until after rendering
is complete for two reasons: first, to avoid repeatedly calling
`writeValue` on every option element until we find the selected one
(could be the very last element). Second, to ensure that we perform the
write after the DOM elements have been created (this doesn't happen
until the end of change detection when animations are enabled).

This is needed to efficiently set the select value when adding/removing options. The
previous approach resulted in exponentially more `_compareValue` calls than the number
of option elements (issue angular#41330).

Finally, this PR fixes an issue with delayed element removal when using the animations
module (in all browsers). Previously when a selected option was removed (so no option
matched the ngModel anymore), Angular changed the select element value before actually
removing the option from the DOM. Then when the option was finally removed from the DOM,
the browser would change the select value to that of the first option, even though it
didn't match the ngModel (issue angular#18430). Note that this is still
somewhat of an application problem when using `ngModel`. The model value
still needs to be updated to a valid value when the selected value is
deleted or it will be out of sync with the DOM.

Fixes angular#41330, fixes angular#18430.

Co-authored-by: Theodore Brown <theodorejb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant