fix(forms): Make radio buttons respect [attr.disabled]
#48864
Closed
+87
−1
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
dylhunn
added
action: review
1 task
dylhunn
removed
the
action: review
dylhunn
added
the
action: review
dylhunn
removed
the
action: review
|
Thanks @AndrewKushnir and sorry for the delay. Back to you on this. |
dylhunn
added
the
action: review
AndrewKushnir
approved these changes
Feb 2, 2023
AndrewKushnir
removed
the
action: review
`setDisabledState` is supposed to be called whenever the disabled state of a control changes, including upon control creation. However, a longstanding bug caused the method to not fire when an *enabled* control was attached. This bug was fixed in v15.
This had a side effect: previously, it was possible to instantiate a reactive form control with `[attr.disabled]=true`, even though the the corresponding control was enabled in the model. (Note that the similar-looking property binding version `[disabled]=true` was always rejected, though.) This resulted in a mismatch between the model and the DOM. Now, because `setDisabledState` is always called, the value in the DOM will be immediately overwritten with the "correct" enabled value.
Users should instead disable the control directly in their model. (There are many ways to do this, such as using the `{value: 'foo', disabled: true}` constructor format, or immediately calling `FooControl.disable()` in `ngOnInit`.)
If this incompatibility is too breaking, you may also opt out using `FormsModule.withConfig` or `ReactiveFormsModule.withConfig` at the time you import it, via the `callSetDisabledState` option.
However, there is an exceptional case: radio buttons. Because Reactive Forms models the entire group of radio buttons as a single `FormControl`, there is no way to control the disabled state for individual radios, so they can no longer be configured as disabled.
In this PR, we have special cased radio buttons to ignore their first call to `setDisabledState` when in `callSetDisabledState: 'always'` mode. This preserves the old behavior.
pullapprove
bot
requested review from
AndrewKushnir,
atscott and
jessicajaniuk
|
merge-assistance: the Public API change is to prevent a spurious documentation symbol for |
dylhunn
added
the
merge: caretaker note
jessicajaniuk
approved these changes
Feb 6, 2023
|
This PR was merged into the repository by commit 5968561. |
pkozlowski-opensource
pushed a commit
that referenced
this pull request
Feb 10, 2023
`setDisabledState` is supposed to be called whenever the disabled state of a control changes, including upon control creation. However, a longstanding bug caused the method to not fire when an *enabled* control was attached. This bug was fixed in v15.
This had a side effect: previously, it was possible to instantiate a reactive form control with `[attr.disabled]=true`, even though the the corresponding control was enabled in the model. (Note that the similar-looking property binding version `[disabled]=true` was always rejected, though.) This resulted in a mismatch between the model and the DOM. Now, because `setDisabledState` is always called, the value in the DOM will be immediately overwritten with the "correct" enabled value.
Users should instead disable the control directly in their model. (There are many ways to do this, such as using the `{value: 'foo', disabled: true}` constructor format, or immediately calling `FooControl.disable()` in `ngOnInit`.)
If this incompatibility is too breaking, you may also opt out using `FormsModule.withConfig` or `ReactiveFormsModule.withConfig` at the time you import it, via the `callSetDisabledState` option.
However, there is an exceptional case: radio buttons. Because Reactive Forms models the entire group of radio buttons as a single `FormControl`, there is no way to control the disabled state for individual radios, so they can no longer be configured as disabled.
In this PR, we have special cased radio buttons to ignore their first call to `setDisabledState` when in `callSetDisabledState: 'always'` mode. This preserves the old behavior.
PR Close #48864
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Feb 15, 2023
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.1.4/15.1.5) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.1.4/15.1.5) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.1.4/15.1.5) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.1.4/15.1.5) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.1.4/15.1.5) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.1.4/15.1.5) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.1.4/15.1.5) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`15.1.4` -> `15.1.5`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.1.4/15.1.5) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v15.1.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#​1515-2023-02-15) [Compare Source](angular/angular@15.1.4...15.1.5) ##### forms | Commit | Type | Description | | -- | -- | -- | | [5f2a3edcf2](angular/angular@5f2a3ed) | fix | Make radio buttons respect `[attr.disabled]` ([#​48864](angular/angular#48864)) | #### Special Thanks AleksanderBodurri, Alvaro Junqueira, Dylan Hunn, Joey Perrott, Matthieu Riegler, PaloMiklo and Paul Gschwendtner <!-- CHANGELOG SPLIT MARKER --> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMzguMiIsInVwZGF0ZWRJblZlciI6IjM0LjEzOC4yIn0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1786 Reviewed-by: 6543 <6543@obermui.de> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
|
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. |
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.
setDisabledStateis supposed to be called whenever the disabled state of a control changes, including upon control creation. However, a longstanding bug caused the method to not fire when an enabled control was attached. This bug was fixed in v15.This had a side effect: previously, it was possible to instantiate a reactive form control with
[attr.disabled]=true, even though the the corresponding control was enabled in the model. (Note that the similar-looking property binding version[disabled]=truewas always rejected, though.) This resulted in a mismatch between the model and the DOM. Now, becausesetDisabledStateis always called, the value in the DOM will be immediately overwritten with the "correct" enabled value.Users should instead disable the control directly in their model. (There are many ways to do this, such as using the
{value: 'foo', disabled: true}constructor format, or immediately callingFooControl.disable()inngOnInit.)If this incompatibility is too breaking, you may also opt out using
FormsModule.withConfigorReactiveFormsModule.withConfigat the time you import it, via thecallSetDisabledStateoption.However, there is an exceptional case: radio buttons. Because Reactive Forms models the entire group of radio buttons as a single
FormControl, there is no way to control the disabled state for individual radios, so they can no longer be configured as disabled.In this PR, we have special cased radio buttons to ignore their first call to
setDisabledStatewhen incallSetDisabledState: 'always'mode. This preserves the old behavior.Related to #48350.