ngneat / forms-manager

🦄 The Foundation for Proper Form Management in Angular
https://www.netbasal.com
MIT License
518 stars 29 forks source link

Add dirty functionality #3

Closed NetanelBasal closed 4 years ago

NetanelBasal commented 4 years ago

I'm thinking about adding a new observable which notifies whether the form is dirty. Dirty means that the current value is different from the initial value.

This observable can be used to toggle the visibility of a save button:

isDirty$ = this.manager.selectIsDirty(formName);
<button *ngIf="isDirty$ | async">Save</button>

Or show a dialog when the user leaves the page.

We need to think whether it makes sense that it'll work in conjunction with the persist form feature.

Coly010 commented 4 years ago

Is there a big difference in how this would work versus just using selectDirty()? When the state is persisted currently, is it only the value that is stored, or the full object? If it's the full object, surely the native dirty boolean will also be stored, reading from the store again will show that it was originally dirty, which would make sense, especially in terms of "save as draft" use-cases?

NetanelBasal commented 4 years ago

selectDirty just read the form's dirty property. This property doesn't care if the user is back to the initial value.

We only store the value. There's no need to take a redundant space from local storage, as Angular will set these properties based on the form's value.

NetanelBasal commented 4 years ago

Is there a big difference in how this would work versus just using selectDirty()?

Yes, dirty means that the initial value is different from the current value. (deep-equal)

Coly010 commented 4 years ago

This property doesn't care if the user is back to the initial value.

So the key difference between selectIsDirty and selectDirty will be down to whether the user has gone back to the initial state of the form, after potentially modifying it?

To avoid confusion, I think a better method name may be needed.

To have this work with the persistence, I'm guessing that the initial value will need to be stored also?

NetanelBasal commented 4 years ago

Exactly!

NetanelBasal commented 4 years ago

I'm working on v2. I'm going to change the API to be more "Angularlish", for example:

manager.valueChanges();
manager.disableChanges();
etc...
NetanelBasal commented 4 years ago

Done 👍

Coly010 commented 4 years ago

Following the API changes, should this still be selectIsDirty or should this perhaps be something different, e.g isInitialValue.

NetanelBasal commented 4 years ago

I think it should be very clear:

initialValueChanges() => Observable<boolean>

Also it should be optional. There is no need to save the initial value if the user doesn't need this functionality.

NetanelBasal commented 4 years ago

@Coly010 are you taking this issue? If not, I will :)

Coly010 commented 4 years ago

Sorry I was feeling poorly the last few days, I can tackle this tonight unless you need it sooner

NetanelBasal commented 4 years ago

Sure. Take the time, and feel well!

Coly010 commented 4 years ago

@NetanelBasal I'm gonna need you to take this, I'm really not well and can't focus right, but if you're willing could you tag me or let me know when it's merged as I'd like to take a look at how you implement it, as even that is baffling me right now, and I'm not sure if I'm just not thinking straight or if it's truly beyond my ability.

NetanelBasal commented 4 years ago

Sure, I will take this.