disabled state is not emitted, blocking external listeners to react to changes of the form's status

[maxime1992]

Context

A classic way of showing that the form is invalid and cannot be saved is by disabling the "send" button.

As we deal with a lot of real time data we're using NgxAutomaticRootFormComponent quite a lot and therefore, on these kind of forms, we don't have a save button.

To show the user some global feedback on the form we've created a component that takes the formgroup as input and listen to both statusChanges and valueChanges.

Issue

This is not working as if we look into the code of ngx-sub-form:

ngx-sub-form/projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts

Lines 417 to 421 in b8890b8

	if (shouldDisable) {
	this.formGroup.disable({ emitEvent: false });
	} else {
	this.formGroup.enable({ emitEvent: false });
	}

We're not emitting the disable event.

Why was is done that way?

When calling disable or enable methods, the formGroup emits all the values again. Example:

  fg = new FormGroup({
    a: new FormControl(),
  });

  constructor() {
    this.fg.valueChanges.subscribe(x => console.log('VALUE CHANGE'));

    this.fg.disable();
    this.fg.enable();

    this.fg.patchValue({ a: 1 });
  }

Gives us:

VALUE CHANGE
VALUE CHANGE
VALUE CHANGE

After all, maybe this is expected as it's Angular's behavior anyway but I'm just trying to think if there's any downside updating the code above from

if (shouldDisable) {
  this.formGroup.disable({ emitEvent: false });
} else {
  this.formGroup.enable({ emitEvent: false });
}

to

if (shouldDisable) {
  this.formGroup.disable();
} else {
  this.formGroup.enable();
}

Can anyone think of a down side? Or confirm that it's probably a good idea?

[maxime1992]

Just had a random thought:

If we do this before broadcasting the value up we probably want to check that the form is not disabled

[tyroneneill]

hey @maxime1992

Thank you for the amazing library!

Looking through the formGroup I can see retrieving the disabled value is delegated onto the FormControl.

The FormControl has the API registerOnDisabledChange(fn: (isDisabled: boolean) => void): void, could we not hook into this to be notified of changes?

It looks to be triggered regardless of the mentioned emitEvent value defined:

[maxime1992]

Just another thought on that:

If we turn emitEvent to true, with #103 we'd broadcast up an empty object which is likely to be an error.

Also, if the RootForm is disabled we should probably never broadcast any update.

[maxime1992]

Discussed offline with @zakhenry and Ty:

• We should follow what angular does by default: emit the value when the form is enabled/disabled
• It's easy to not trigger downstreams if needed by just using distinctUntilChanged

To do:

• Remove the emitEvent: false
• Do not broadcast the value up for a RootForm if it's disabled

[maxime1992]

🎉 This issue has been resolved in version 5.2.0-feat-rewrite.1 🎉

The release is available on:

• npm package (@feat-rewrite dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[zakhenry]

This has now been reverted in the rewrite, unfortunately the rewrite implementation was causing forms to appear to be changed by the user, causing emit side effects. It will need some deeper thought to resolve this properly

[github-actions]

🎉 This issue has been resolved in version 5.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀