Skip to content

fix(forms): fix disabled attribute handling - #18575

Closed
dzonatan wants to merge 1 commit into
angular:masterfrom
dzonatan:fix-form-control-disabled-instantiation
Closed

fix(forms): fix disabled attribute handling#18575
dzonatan wants to merge 1 commit into
angular:masterfrom
dzonatan:fix-form-control-disabled-instantiation

Conversation

@dzonatan

@dzonatan dzonatan commented Aug 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Update the disabled attribute of DOM element when setting a new FormControl instance either directly or via setControl().

Fixes #17806

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

myGroup.setControl('ctrl', new FormControl({ value: 'Control is disabled', disabled: true }));
// Input dom element has disabled attribute. Correct.

myGroup.setControl('ctrl', new FormControl({ value: 'Control should be enabled', disabled: false }));
// Input dom element still has disabled attribute. WRONG! Should not have.

Full issue with plunker demo: #17806

What is the new behavior?

myGroup.setControl('ctrl', new FormControl({ value: 'Control is disabled', disabled: true }));
// Input dom element has disabled attribute. Correct.

myGroup.setControl('ctrl', new FormControl({ value: 'Control should be enabled', disabled: false }));
// Input dom element does not have disabled attribute anymore. Correct.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

@dzonatan
fix(forms): fix disabled attribute handling
bfadd8b 
Update the disabled attribute when setting a new FormControl instance either directly or via setControl().

Fixes angular#17806
@dzonatan
dzonatan force-pushed the fix-form-control-disabled-instantiation branch from 59f6d7f to bfadd8b Compare August 7, 2017 22:04
@vicb vicb added action: review The PR is still awaiting reviews from at least one requested reviewer area: forms labels Aug 8, 2017
@dzonatan dzonatan mentioned this pull request Aug 16, 2017
Closed
@dzonatan

dzonatan commented Aug 25, 2017

Copy link
Copy Markdown
Contributor Author

@kara is there anything I can help to speed up this PR?
So strange that not so many fall into this issue. 😕

@dzonatan

dzonatan commented Oct 2, 2017

Copy link
Copy Markdown
Contributor Author

Image of Yaktocat

@dzonatan dzonatan mentioned this pull request Nov 23, 2017
Closed
kara
kara approved these changes Dec 20, 2017
View reviewed changes

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 20, 2017
@alxhub alxhub added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 20, 2017
@alxhub alxhub removed the action: merge The PR is ready for merge by the caretaker label Dec 20, 2017
@alxhub

alxhub commented Dec 20, 2017

Copy link
Copy Markdown
Member

Hey @kara,

This looks like it might break a test in g3, can you look into it?

@dzonatan

dzonatan commented Dec 20, 2017

Copy link
Copy Markdown
Contributor Author

What is g3? 🙊

@trotyl

trotyl commented Dec 21, 2017

Copy link
Copy Markdown
Contributor

@dzonatan Some naive translation: this change would break some internal apps in Google, they'll look into that and confirm what the problem is. If indeed something need to be change in this PR (rather than problem of those apps themselves), they'll bring feedback here.

@rvirji-va

rvirji-va commented Feb 2, 2018 •
edited
Loading

Copy link
Copy Markdown

Any updates on this? I'm having to bind to [disabled] on my form control in the frontend as a workaround, but I'd like to make this work as intended instead

@dzonatan @kara

@rvirji-va

rvirji-va commented Feb 7, 2018

Copy link
Copy Markdown

@alxhub any updates? Is this still breaking g3?

@LisaTatum

LisaTatum commented Mar 8, 2018

Copy link
Copy Markdown

It is ok that there are quality gates this pull request has to pass, but that it takes SIX MONTHS and it is not yet merged and this in an agile world... is really disappointing and not understandable.

I am also waiting for that feature!

@LisaTatum

LisaTatum commented Mar 18, 2018

Copy link
Copy Markdown

Dear @kara , any news on this? I would like to close our internal ticket :-)

@dzonatan

dzonatan commented Apr 20, 2018

Copy link
Copy Markdown
Contributor Author

@alxhub maybe you can give any update on this? ..

@banjankri

banjankri commented May 30, 2018

Copy link
Copy Markdown

@alxhub pinging again on this one as another month passed. We are patiently waiting for this issue to be fixed too ....

@dzonatan dzonatan mentioned this pull request Jun 4, 2018
Closed
@dzonatan

dzonatan commented Aug 8, 2018

Copy link
Copy Markdown
Contributor Author

Happy Anniversary

@banjankri

banjankri commented Sep 10, 2018

Copy link
Copy Markdown

what sort of cleanup is necessary to have this PR merged in @alxhub ? Or any other actions that need to be taken? Willing to jump on it to have that merged.

@jasonaden jasonaden added this to the needsTriage milestone Jan 29, 2019
@kara kara removed their assignment Sep 9, 2019
@dzonatan

dzonatan commented Sep 13, 2019

Copy link
Copy Markdown
Contributor Author

two years and counting.. still relevant :)

@dzonatan dzonatan mentioned this pull request Dec 4, 2019
Closed
@AndrewKushnir AndrewKushnir mentioned this pull request Jun 9, 2020
Closed
9 tasks
@AndrewKushnir

AndrewKushnir commented Nov 17, 2020

Copy link
Copy Markdown
Contributor

Hi, thanks for creating this PR! I'm doing some triage and it looks like this PR is very similar to #35311, so I will close this one in favor of #35311. Thank you.

@dzonatan

dzonatan commented Nov 17, 2020

Copy link
Copy Markdown
Contributor Author

image

@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Dec 19, 2020

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@kara kara kara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: forms cla: yes target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

FormControl does not reflect the new disabled value