Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The disabled attribute does not work on a top level form which is an array #84

Closed
maxime1992 opened this issue Aug 6, 2019 · 10 comments
Assignees
Labels
effort-2: hours Will only take a few hours to fix/create flag: good for first contribution Good for a first PR on the repo released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: bug/fix This is a bug or at least needs a fix workaround-1: obvious Obvious workaround

Comments

@maxime1992
Copy link
Contributor

maxime1992 commented Aug 6, 2019

Trying to get the top level form disabled in the following case (where currentStateRotors is an array):

<app-rotors-form
  *ngIf="(currentStateRotors$ | async) as currentStateRotors"
  [rotors]="currentStateRotors"
  [disabled]="true"
></app-rotors-form>

The disabled property is not taken into account.

It should loop over the array and disable all the controls from the array.

@maxime1992 maxime1992 added scope: lib Anything related to the library itself type: bug/fix This is a bug or at least needs a fix workaround: none No workaround flag: good for first contribution Good for a first PR on the repo labels Aug 6, 2019
maxime1992 added a commit that referenced this issue Aug 15, 2019
@maxime1992
Copy link
Contributor Author

☝️ Wrote a test for that and couldn't repro 🤔

@maxime1992
Copy link
Contributor Author

maxime1992 commented Nov 14, 2019

I was able to repro and understand what's happening.

I've tried to write a fix to see if it would be that and it worked. PR on the way but missing tests for now.

EDIT: about the "workaround: obvious" --> instead of passing [disabled]="true" we can wait a tick before setting it to true...

@maxime1992 maxime1992 added effort-1: minutes Will only take a few minutes to fix/create state: has PR A PR is available for that issue workaround-1: obvious Obvious workaround and removed state: needs repro This issue needs a repro workaround: none No workaround labels Nov 14, 2019
@maxime1992 maxime1992 self-assigned this Nov 14, 2019
@areijngoudt
Copy link

We've noticed the same behaviour when FormArray's are in SubForms. When initially [disable]="true", normal FormControls get disabled but FormControls that are part of FormArrays don't get disabled.

@maxime1992
Copy link
Contributor Author

After a few hours of investigation and banging my head on a wall, I found out really really weird issues with Angular and disabled forms:

angular/angular#31506
angular/angular#22556

To explain a bit the weird behavior I did encounter here:

  • ngx-sub-form creates an internal FormGroup for every sub component (or control value accessor under the hood)
  • whenever the writeValue hook is called by angular, we set the value of the form to the new one:

this.formGroup.setValue(transformedValue, {
emitEvent: false,
});

  • In some cases when the form is disabled and its value changes (not always...) the form is being set to enabled
  • console logging this.formGroup.disabled just before and right after the setValue above always show the same value (which is what we'd expect) aven though it's apparently not true 🤷‍♂️
  • saving if the form is disabled or not before the setValue and disable it after if it was already disabled... fixes it 😱 ...
const fgDisabled: boolean = this.formGroup.disabled;

this.formGroup.setValue(transformedValue, {
  emitEvent: false,
});

if (fgDisabled) {
  this.formGroup.disable();
}

✔️

I do not understand, I'm not sure how to repro and therefore can't write a test but I've made a comment explaining that in the code so it doesn't go away 👍

Fix coming soon

@maxime1992 maxime1992 added effort-2: hours Will only take a few hours to fix/create and removed effort-1: minutes Will only take a few minutes to fix/create labels Dec 18, 2019
@maxime1992
Copy link
Contributor Author

🎉 This issue has been resolved in version 3.0.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@maxime1992
Copy link
Contributor Author

@areijngoudt you can upgrade to the new version 👆

Let us know how it goes and if it fixes the issue on your side too 😸

maxime1992 added a commit that referenced this issue Dec 18, 2019
maxime1992 added a commit that referenced this issue Dec 18, 2019
@maxime1992
Copy link
Contributor Author

🎉 This issue has been resolved in version 3.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@maxime1992
Copy link
Contributor Author

actually I dealt with 2 errors at the same time and the first one might not be the one you needed.

You should rather grab latest (3.0.6) instead of 3.0.5 👍

@areijngoudt
Copy link

@maxime1992 works like a charm, thanks!

I still have one question: when the form is set to disabled (using the [disabled] input property), the formGroup becomes dirty (while the form values have not changed). Is there a way to prevent this?

@maxime1992
Copy link
Contributor Author

works like a charm

Glad to hear that 👍 !

when the form is set to disabled (using the [disabled] input property), the formGroup becomes dirty

Sounds like a bug @areijngoudt can you create a new issue please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-2: hours Will only take a few hours to fix/create flag: good for first contribution Good for a first PR on the repo released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: bug/fix This is a bug or at least needs a fix workaround-1: obvious Obvious workaround
Projects
None yet
Development

No branches or pull requests

2 participants