RC6 Forms: disabled attribute cannot be set dynamically anymore

[0cv]

I'm submitting a bug/feature request (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[x] feature request
[ ] support request => Please do not submit support request here, instead see /~https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior
Have a look at that, (demo: http://plnkr.co/edit/P25dYPC5ChRxpyxpL0Lj?p=preview):

@Component({
  selector: 'my-app',
  providers: [],
  template: `
    <div [formGroup]="form">
      <input formControlName="first" [disabled]="isDisabled">
    </div>
  `,
  directives: []
})
export class App {
  isDisabled = true
  form = new FormGroup({
    'first': new FormControl('Hello')
  })
}

There is a warning message asking to refactor the code, but more importantly, the input is unfortunately not disabled. Refactoring the code to what is suggested is not helping either, i.e. doing this will not work, (demo: http://plnkr.co/edit/Gf7FGR42UXkBh6e75cm2?p=preview):

@Component({
  selector: 'my-app',
  providers: [],
  template: `
    <div [formGroup]="form">
      <input formControlName="first">
    </div>
  `,
  directives: []
})
export class App {
  isDisabled = false
  form = new FormGroup({
    'first': new FormControl({value: 'hello', disabled: this.isDisabled})
  })

  constructor() {
    setTimeout(() {
      this.isDisabled = true  
    }, 10)
  }
}

Expected/desired behavior
After the callback of the setTimeout is executed, the input should be disabled, but it is not.

Reproduction of the problem
Yes, please look here: http://plnkr.co/edit/P25dYPC5ChRxpyxpL0Lj?p=preview

What is the motivation / use case for changing the behavior?
To have the same behaviour, or similar one than in RC5. At least, we shall have a possibility, even if it's a breaking change, to set dynamically the disabled attribute. As of now, it does not seem possible anymore.

Please tell us about your environment:

• Angular version: 2.0.0-rc.6
• Browser: [ Chrome 52 ]
• Language: [ TypeScript ]

[kara]

@Krisa If you want to set the disabled state programmatically, you'll need to call disable(). Changing this.isDisabled will never have an effect because it is passed by value.

export class App {
  isDisabled = false
  form = new FormGroup({
    'first': new FormControl({value: 'hello', disabled: false})
  })

  constructor() {
    setTimeout(() => {
      this.form.get('first').disable();
    }, 10)
  }
}

See example here: http://plnkr.co/edit/UXNiM8Pz73Go27nDRD7M?p=preview

Re the first example, we deliberately haven't hooked up the template-driven entry point to reactive forms. That's why we throw the warning. If we did hook it up, users would immediately run into 'changed after checked' errors because the validation of reactive forms occurs synchronously. We generally don't suggest mixing reactive and template-driven patterns for this reason.

We considered throwing an error instead, but we were concerned that if someone had a custom form component that used disabled as an input for something unrelated, their code would break. Perhaps we can make the warning message a bit clearer. Let me know if you think the language could be changed. In any case, closing this as it works as designed.

[0cv]

@kara - I was aware of the disable method, but it's not the same, at least not in my case.

Let me take a bit more complicated example than what we can see above. I have a form composed of roughly 70 to 80 fields split across several components. Some part of the form is disabled/enabled, shown/hidden based on either some attributes of the record (on load, or on save), or depending on the user viewing this record, or even from where the record is being called (i.e. different route). I'm playing with the disabled and ngIf attribute / directive (required is so far always static in my case) but all are defined by, I would call that rules, at the attribute/directive level. This approach is quite consistent for me, ngIf like disabled are all in the html template.

Now, if I have to refactor this code based on what you suggest, I will probably need a central function in my class, and call disable() or enable() for each field, based on the value of the form and from external property (user profile, route params, etc.). This function shall subscribe to valueChanges, and also be called when the component is loaded, or when the save method is called. Definitely doable, but it feels terribly awkward, at least compared to the current solution, don't you think?

Either way, I will have to live with what is implemented. I have so far downgraded the form module to 0.3.0, hoping for a PR resolving the situation above, but based on your comment, I fear this spec is not going to change, is it? Could you please (re)confirm one or the other?

Thanks
Chris

[udos86]

@kara Thanks for your great work so far! Please reconsider disabled property bindings in reactive forms as this is an essential feature to any dynamic forms approach. Otherwise it is not possible to directly change the enabled/disabled state of a control by simply setting a corresponding business object model property on the outside. It would need native ES6 Proxies to intercept the disabled-setter call of that object model in order to call disable() or enable() on the FormControl as well. I myself building ng2 Dynamic Forms would really appreciate it!

[capi]

If I call this.myForm.controls["code"].disable();, this.myForm.valid is always false:

this.myForm.patchValue({code: 17});
console.log(this.myForm.valid);
this.myForm.controls["code"].disable();
console.log(this.myForm.valid);

This outputs

true
false

Why is this the case? I want the control to be disabled, but the from itself to be still valid.

[0cv]

@capi Looks like a bug to my opinion, but it's also not related to the original issue I've reported, which is the ability to set the disabled attribute dynamically.

Please open a new issue for that to not mix up things

[maku]

@capi I have the same behaviour like you. For me this is wrong....

[capi]

@maku I filed #11379 for this, we should not mix the two issues, @Krisa is entirely right about that.

[unsafecode]

@udos86 @Krisa We are experiencing the same behavior, and it looks quite troublesome to solve: on one hand, we could code the dynamic enable/disable behavior by subscribing to valueChangesof FormControl, but it requires lot of code each time (and we have large forms with many fields).

On the other hand, I was thinking about a custom directive to replace disable, taking the FormControland an Observable, in order to keep the reactive approach in templates, but I need to experiment with it.

[BrainCrumbz]

Is there any update on this? I mean, now you have a view framework that lets you work with declarative, one-way binding code for a load of attributes and for whole content items, but then for disabled attribute you're stuck to imperative code?

[0cv]

@kara @pkozlowski-opensource
Positive or negative, it would be great to have a feedback, even very quick, on what's coming soon, to not let us in the dark, potentially refactoring unnecessarily our code.

Thank you

[kemsky]

@Krisa, i would recommend to avoid reactive forms. Reactive forms require a lot of coding if want interaction with other parts of template and offer very small benefits. You should be able to create configurable custom validator (or even validate your data model manually) and use it instead of required, also you could work with a set of forms instead of combining everything into one big form.

[unsafecode]

@Krisa On our project, we quick-solved this by using two custom utility methods:

import { FormGroup, FormControl, AbstractControl } from "@angular/forms";

export function includeInForm(form: FormGroup, controlName: string, includeChildren?: boolean) {
    const ctl = form.get(controlName);
    makeOptional(ctl, false, includeChildren);
}

export function excludeFromForm(form: FormGroup, controlName: string, excludeChildren?: boolean) {
    const ctl = form.get(controlName);
    makeOptional(ctl, true, excludeChildren);
}

function makeOptional(ctl: AbstractControl, isOptional: boolean, children?: boolean) {
    if (isOptional) {
        (<any>ctl).__validator = ctl.validator || (<any>ctl).__validator;
        ctl.clearValidators();
    } else {
        ctl.setValidators(ctl.validator || (<any>ctl).__validator);
    }

    if (children) {
        Object.keys((<FormGroup>ctl).controls).forEach((control) => {
            makeOptional(ctl.get(control), isOptional);
        });
    }

    ctl.updateValueAndValidity();
}

It might not be the most elegant - for sure - but it works and made the code work again with minimal changes.

[natcohen]

I'm also looking for a simpler and less verbose solution to disable using form variables!

[andrevmatos]

Why is this closed? There's still no clear way to declaratively disable a form control, nor workaround for it.

[natcohen]

Seems that the angular team doesnt want to communicate on this...max be thés are still discussing alternatives

[andrevmatos]

Ok, best workaround I could think of is to create a directive to do the dirty job, and "declaretively" test for a boolean condition and get a form/input/FormControl disabled without the need test for expressions that sometimes aren't observables where it's easy to know when something changed (i.e.: to depend on the change detection system).

You can even use it to disable other FormControl, or use another FormControl.value as a condition to disable itself.

@Directive({
  selector: '[disableFC][disableCond]'
})
export class DisableFCDirective {
  @Input() disableFC: FormControl;

  constructor() { }

  get disableCond(): boolean { // getter, not needed, but here only to completude
    return !!this.disableFC && this.disableFC.disabled;
  }

  @Input('disableCond') set disableCond(s: boolean) {
    if (!this.disableFC) return;
    else if (s) this.disableFC.disable();
    else this.disableFC.enable();
  }
}

And it can be used like:
<input type="text" [formControl]="textFC" [disableFC]="textFC" [disableCond]="anotherElement.value < 10" />

[raisindetre]

@andrevmatos
Hi there - have been grappling with this issue and tried to apply your suggested fix without success. Would you mind taking a look at this Plunk and clarifying where its going wrong?

[andrevmatos]

@raisindetre
Before all, you need to set formControlName on the selects for it to use the controls inside the formGroup, or formControl to set the control directly. After that, you can use my directive to enable or disable dinamically the control. I changed your example to demonstrate my directive disabling the first select based on a boolean variable (toggled by the button), and the second to disable it based oh the value of the first.

[raisindetre]

Ah, got you - thanks! I was almost there at one point but I was wrapping the formControlName directive in square brackets. Still trying to get the hang of model/reactive driven forms.

[PTC-JoshuaMatthews]

@andrevmatos

I updated your directive to what I would consider a bit cleaner of an approach, sharing the formControl input passed from the html element that is already a formConrol.

@Directive({
    selector: '[formControl][disableCond]'
})
export class DisableFormControlDirective {
    @Input() formControl: FormControl;

    constructor() { }

    get disableCond(): boolean { // getter, not needed, but here only to completude
        return !!this.formControl && this.formControl.disabled;
    }

    @Input('disableCond') set disableCond(s: boolean) {
        if (!this.formControl) return;
        else if (s) this.formControl.disable();
        else this.formControl.enable();
    }
}

This allows us to remove the duplication from the template

<input type="text" [formControl]="textFC" [disableCond]="anotherElement.value < 10" />

[PTC-JoshuaMatthews]

@kara
Although I would really appreciate it if the Angular team could update the reactive forms component to track the formControl disabled property correctly.

[BrainCrumbz]

@andrevmatos @PTC-JoshuaMatthews we've found also the version at this GH issue comment by @unsafecode to be quite interesting.

[pmulac]

I have been facing the same issue - there isn't a clean way to disable portions of a reactive/model-driven form. The use case is pretty common: Consider a group radio buttons that each display a child form when selected, and each subform that is not selected needs to be disabled in order to make the entire form valid.

I really like @PTC-JoshuaMatthews 's DisableFormControlDirective, but then I realized that it defeats the purpose of using reactive forms... In other words, the disabling becomes template-driven (to test it, you need to parse the template). As far as I can tell, the best way to stay model-driven/reactive is to subscribe to valueChanges on a control and then call disable on the corresponding subform (which is very verbose/ugly). I'm really surprised there isn't a better way to achieve this.

[billfranklin]

@kara why is this closed?

[ghost]

@billfranklin The correct way is to bind to attr.disabled. Check the solution that kekeh posted.

[hk-skit]

@kekeh 's solution is great but it does not work with custom form controls. I am trying to conditionally disable form fields in angular2-json-form-schema. I am planning to do it by using a directive the way @PTC-JoshuaMatthews has suggested. Is there any better way to achieve the same? I know I can implement ngDoCheck in custom controls but it won't be an efficient solution.

[vehansayvazi]

any news on this?

[mkotsollaris]

Not sure why this is closed even though no clean solution/approach was given ...

[ghost]

@mkotsollaris Did you read the reply with the most thumbs up?

[PTC-JoshuaMatthews]

I ended up abandoning this approach altogether in my applications. Instead of setting isDisabled property on my component and expecting the form to track that properties changes, I instead call this.form.controls[“mycontrol”].disable() in the ngAfterViewInit hook. If the status changes I explicitly call enable() to reactive it.

This approach works fine as I’m already responding to some event to enable/disable the control so no problem doing it directly through the form api as opposed to attempting to bind the disabled status to a property. I suspect my desire to bind it comes from my angular 1 days.

[vishalbiradar]

Is it good/correct way of using below code for disabling the forms ???

<input [attr.disabled]="disabled?'':null"/>

I have tried with the suggestion provided by angular,
But still it was throwing the warning, and the field isn't disabled.
In component

  form = new FormGroup({
    firstName: new FormControl({value: 'Vishal', disabled: true}, Validators.required),
    lastName: new FormControl('Drew', Validators.required)
  });

Then, in html

<input type="text" formControlName="firstName" 
       [value]="form.controls['firstName'].value"
       [disabled]="form.controls['firstName'].disabled">

[AndrewPa]

@Highspeed7 That works "partially" in that the element's disabled attribute value is bound to the expected value, but for some arcane HTML standard reason, the disabled attribute need only exist for it to disable the element, even if disabled="false".

@kekeh Added to this solution by binding the ternary isDisabled ? '' : null instead of just isDisabled. This is basically a hack to get Angular to clear the attribute entirely when the form is re-enabled.

So on the template, this would be the workaround:
[attr.disabled]="isDisabled ? '' : null"

[sgronblo]

What are the implications of setting [attr.disabled] vs [disabled]?

[dudewad]

@sgronblo afaik there aren't any. I was under the impression that [disabled] is just shorthand for [attr.disabled] but I could be wrong.

[tinyweasel]

This worked:

<input [attr.disabled]="disabled?'':null"/>

private disabled: boolean = false;

// disable input box
disableInput(): void {
   this.disabled = true;
}

// enable input box
enableInput(): void {
   this.disabled = false;
}

It worked for me even by just having this.disabled = false; etc, wherever I needed it. The functions weren't necessary.

[Sangarlal]

this.myForm.controls['id'].disable();

[danzrou]

Is there a more proper solution for this case instead of binding to the disabled attribute (which is kept after the form is re-enabled) ?

[PTC-JoshuaMatthews]

@danzrou Does the answer directly above you not work for you? Just disable the form control that the input is tied to and the input will be dealt with automatically.

[dudewad]

@danzrou yes, fwiw, afaik that is the defacto way to enable/disable form controls:
someFormGroup.controls['controlName'].disable()
and
someFormGroup.controls['controlName'].enable()

Keep in mind that reactive forms are not the standard form set. They are intended to be used programmatically, not through the dom, which is why (I suspect) they don't allow the bindings anymore.

[danzrou]

My current situation is that I am passing a form control to a child component and the control is changing dynamically according to user selection. If I pass a control which has disabled=true, the input element is indeed disabled. Then I pass a a new control which has disabled=false (and I made sure it is not disabled) and yet the disabled attribute remains on the input field. This still happens in Angular 7.2.3

[PTC-JoshuaMatthews]

@danzrou can you elaborate on how are you passing the form control into the child component? My guess is that the child component isn’t seeing the new form control.

[danzrou]

@PTC-JoshuaMatthews it passes it using standard input. Some more details:

• Each time user clicks on some element, new form controls are generated for this request therefore the reference for them is not kept
• This means a new control instance is being pass each time (not the same one with property changes)

[Sangarlal]

Is there a more proper solution for this case instead of binding to the disabled attribute (which is kept after the form is re-enabled) ?

this.myForm.controls['id'].enable();

[PTC-JoshuaMatthews]

@danzrou I could see why that wouldn’t work, you’re swapping out the form control but not changing disabled state while it is bound to the input. Form controls aren’t really meant to be swapped out like that, there’s no event taking place for it to react to.

If you’re sure that you have to swap form controls, I would recommend using an input setter to detect when the form control changes and re-initializing the input when this happens.

https://stackoverflow.com/questions/36653678/angular2-input-to-a-property-with-get-set

As for how to reinitialize it a dirty solution would be by hiding it with an ngIf and then showing it in a timeout directly after. A cleaner solution might just be to call formControl.setDisabled(formControl.disabled). Just as long as you’re doing this while the form control is bound to the input. Might have to get creative with a timeout

[sgentile]

please provide a flag in angular config to disable this warning

[dudewad]

@sgentile I think the entire point of the warning is that you're using reactive forms, and therefore should not be using template-driven-form syntax to control form state. This could cause a lot of confusion when a developer sees reactive forms, tries to program in reactive forms, and unknowingly ends up fighting a template-driven form convention because it was implemented in conflict. The solution is to set/unset the disabled state in code just like you're handing the rest of your reactive form state (myForm.controls['someControl'].disable())

[sgentile]

Quite a few assumptions here. For example, I'm using Primeng and I need to pass disabled to their control and this causes the console warning. I think it's fine to show console warnings to developers, however they should be able to decide if they want them. Make a setting in the angular.json file It's over extending imo.

…

On Sat, Aug 31, 2019, 8:06 PM Layton Miller ***@***.***> wrote: @sgentile </~https://github.com/sgentile> I think the entire point of the warning is that you're using reactive forms, and therefore should not be using template-driven-form syntax to control form state. This could cause a lot of confusion when a developer sees reactive forms, tries to program in reactive forms, and unknowingly ends up fighting a template-driven form convention because it was implemented in conflict. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11271?email_source=notifications&email_token=AABAKNGBGNDK3TLWNHCXVJLQHMBPTA5CNFSM4CON24SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5TXM7Y#issuecomment-526874239>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/AABAKNE74ZR3UZBVBEQQXHDQHMBPTANCNFSM4CON24SA> .

[sgentile]

Furthermore when I did attempt to use disable it didn't work

…

On Sat, Aug 31, 2019, 8:06 PM Layton Miller ***@***.***> wrote: @sgentile </~https://github.com/sgentile> I think the entire point of the warning is that you're using reactive forms, and therefore should not be using template-driven-form syntax to control form state. This could cause a lot of confusion when a developer sees reactive forms, tries to program in reactive forms, and unknowingly ends up fighting a template-driven form convention because it was implemented in conflict. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11271?email_source=notifications&email_token=AABAKNGBGNDK3TLWNHCXVJLQHMBPTA5CNFSM4CON24SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5TXM7Y#issuecomment-526874239>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/AABAKNE74ZR3UZBVBEQQXHDQHMBPTANCNFSM4CON24SA> .

[dudewad]

@sgentile Yeah sorry - wasn't attempting to assume but without more info you sound like most developers who complain about a feature w/o a justification. Framework and language devs have to make the most generic choices possible and a lot of times that gets overlooked. I think your case is a valid point. Ideally you'd submit a feature request for this since this is a closed ticket that is attempting to report a bug.

[sgentile]

You have motivated me to see if I can get a ViewChild reference to the control and see if I can set it programatically

…

On Sat, Aug 31, 2019, 8:18 PM Layton Miller ***@***.***> wrote: @sgentile </~https://github.com/sgentile> Yeah sorry - wasn't attempting to assume but without more info you sound like most developers who complain about a feature w/o a justification. Framework and language devs have to make the most generic choices possible and a lot of times that gets overlooked. I think your case is a valid point. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11271?email_source=notifications&email_token=AABAKNEXR32GSL3U5ZS6TZLQHMC5LA5CNFSM4CON24SKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5TXSJI#issuecomment-526874917>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/AABAKND5MXVOV5BYO4R6DS3QHMC5LANCNFSM4CON24SA> .

[angular-automatic-lock-bot]

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.}