This repository was archived by the owner on Apr 12, 2024. It is now read-only.
fix(select): do not update selected property of an option element on digest with no change event #8367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…render This reverts commit dc149de. That commit fixes a bug caused by Firefox updating `select.value` on hover. However, it causes other bugs with select including the issue described in angular#7715. This issue details how selects with a blank disabled option skip to the second option. We filed a bug with Firefox for the problematic behavior the reverted commit addresses https://bugzilla.mozilla.org/show_bug.cgi?id=1039047, and alternate Angular fixes are being investigated. The test introduced in this commit does test a valid case and is therefore not reverted. Closes angular#7715 angular#7855
An earlier commit dc149de caused an error where the first option of a select would be skipped over if it had a blank disabled value. These tests demonstrate that with that commit in place, blank disabled options are skipped in a select. When the commit is reverted, the correct behavior is seen that the blank disabled option is still selected in both selects marked with required and those that have optional choices. Relates to angular#7715
A regression angular#7855 was introduced by angular@dc149de This test ensures that reverting that commit fixes this regression. In the regression, changes to a bound model in ng-change were not propagated back to the view. Test for angular#7855
…digest with no change event The `render()` method was being invoked on every turn of the digest cycle, which was inadvertently updating the DOM even when a `change` event had not been triggered. This change only calls the `render()` method when `ctrl.$render()` is called, as part of the NgModelController` lifecycle and when the `modelValue` has significantly changed.
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
@IgorMinar, @jeffbcross, @ealtenho, @caitp - how does this look? |
|
LGTM. I added these commits back to #8221 since I was addressing the issue of splitting the revert commit and the addition of the tests. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8221