angular: RC6 Forms: disabled attribute cannot be set dynamically anymore

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 ]

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 48
  • Comments: 74 (1 by maintainers)

Most upvoted comments

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

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

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

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?

@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

@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

@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!

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

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

@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" />

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" />

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.

Disabled, should definitely be allow to accept a function. Disabled state will often change and it is a mess to have helper methods and watchers just to check a condition.

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

Erm… maybe I’m a bit off here, but I was able to get the disabled feature working with [attr.disabled]=“isDisabled”

The problem could be solved the ‘reactive’ way, if the FormControl constructor accepted a function for the disabled property of the first parameter. Currently it only accepts a boolean value. E.g.

  form = new FormGroup({
    'first': new FormControl(),
    'second': new FormControl({value: '', disabled: formValue => formValue.first === 'something'})
  })

I’ve been handling this scenario by tying into the ngDoCheck Lifecycle hook.

ngDoCheck() {
    if (this.childControl.disabled != this.isDisabled) {
        this.isDisabled ? this.childControl.disable() : this.childControl.enable();
    }
}

You can solve it with a Directive. A directive named Canbedisabled and a poperty “blocked”, for example. Write a setter for blocked and set it to nativelement.disabled property.

Code example:

@Directive({
  selector : ["canbedisabled"]
})
export class Canbedisabled{
  
    constructor(private el: ElementRef) {
       
    }
  
  @Input()
  set blocked(blocked : boolean){
    this.el.nativeElement.disabled = blocked;
  }
}

 <input formControlName="first" canbedisabled [blocked]="isDisabled">

That kind of restrictive approach take down the advantage of using Angular Reactive forms since disable and enable an element is a pretty much a basic feature, specially if you are building a dynamic form.

So if you need to build a complex dynamic form, and need total control on that form, right now it’s better use the template-driven approach, even though it can means more work, such as validation, down the road this approach will be bather then end up with a complex code full of workarounds to solve basic things like enable and disable a form element.

Btw, there is nothing that says you can not use both approaches, reactive and template-driven, in the same application, just be aware of the advantages and limitations of each one.

https://angular.io/docs/ts/latest/guide/reactive-forms.html

@kemsky (kemsky commented on Sep 22, 2016),

This actually seems like the best approach, IMO. I first realized Microsoft had a hand in the Angular 2 project when I saw how much Boilerplate gets implemented into projects when they use ReactiveForms. For some things, I’m thrilled about Microsoft’s influence on Angular2, but this is one area where I’ll be rejecting the MS-Boilerplating – assuming its their hand in it.

There’s a correct way to do things, and there’s a right way. If you’re throttling the Engineer’s productivity so much without much benefit – not to mention adding more Coupling in the system – then that’s not the “right way”. Why don’t we just make it so your application blows up if an input is not wrapped in a form?

Why ReactiveForms??? What benefits do you really get? Seems like validation (etc) is better done using a Visitor Pattern, Decorator, Mediator, Service or something else anyway.

That said – before I throw the ugly baby out with the bathwater – could you gives us some insight to any caveats you’ve encountered on the ‘no-reactive-forms-boilerplate’ bandwagon?

Much Appreciated!

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.

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

@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"

Agree with the many others…disabled NEEDS to be dynamically controlled, otherwise it’s fairly useless

You can use form.getRawValue()

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.

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();

this.myForm.controls[‘id’].disable();

does not work for me

EDIT

I figured out how to do it without using a directive, see my SO link below.

I just saw this warning in the console, so I am trying to solve this the RxJS way as per the comment from @pmulac - subscribe to ValueChanges.

In my case I need to enable a control that is disabled on init.

SO question here if anyone can shed some light on how to solve this, thanks.

@pmulac while @PTC-JoshuaMatthews directive can be definitely useful, I’d just like to point out that in our project in the end for many single controls (e.g. input, checkbox) that had to be disabled, we switched to binding on [readonly] in order to actually disable that input. If validation was somehow involved, e.g. a readonly field had to be ignored under some condition, we made custom validation that took condition into account. On the other hand, if it was a matter of disabling whole forms, it looks to me that binding on [disabled] still works good, with no warning. Just our 2¢

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

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