formly-js / angular-formly

JavaScript powered forms for AngularJS
http://docs.angular-formly.com
MIT License
2.22k stars 405 forks source link

resetModel doesn't work for nested forms #528

Closed MCKRUZ closed 8 years ago

MCKRUZ commented 8 years ago

http://angular-formly.com/#/example/other/nested-formly-forms

if you hit the reset model button it doesn't reset the field that is inside the nested form. Can we get an update to get this working?

kentcdodds commented 8 years ago

cc @benoror. Could you look into this? It may be something we need to work out in formly.

benoror commented 8 years ago

Calling vm.options.resetModel() doesn't seem to propagate to nested formly-form's (However stuff like vm.form seem to be correctly inherited).

If I bind the options variable to the template now resetModel() resets the nested fields, although ignoring the outer scope fields, see example: http://jsbin.com/zaqeke/18/edit?html,js,output

benoror commented 8 years ago

@kentcdodds Do you think this can be fixed in angular-formly itself (maybe related to https://github.com/formly-js/angular-formly/issues/465) ?

Or it might be related to the way angular handles nested forms, something along the lines of https://github.com/angular/angular.js/pull/11023

Update: This seems to work for fieldGroup's. I'm going to write some tests

benoror commented 8 years ago

Ok I wrote a failing test for this issue:

expected { innerField: 'zab' } to deeply equal { innerField: 'bar' }

I suspect this is related to nested keys handling https://github.com/formly-js/angular-formly/issues/426, any hint where should I look at?

MCKRUZ commented 8 years ago

Not sure who you're asking but I think the issue is that in function resetModel()

Your looping over all the $scope.fields _angularFix2['default'].forEach($scope.fields, function (field)

The problem is that the "fields" object that your looping over isn't the same bound object that you bind here

"formly-form model="model[options.key]" fields="options.data.fields""

It's just an arbitrary object that looks at "fields", whereas for this perticular wrapper it's in options.data.fields, not options.fields.

benoror commented 8 years ago

@MCKRUZ you're right! :smile_cat:

Therefore instead of messing up with data a cleaner solution would be to use a wrapper around a _fieldGroup_, which is something already planned to implement in https://github.com/formly-js/angular-formly/issues/486 :tada: (although support for templateOptions should be also considered, in order to customize panel's title)

JSBin example: http://jsbin.com/zaqeke/20/edit?html,js,output

@kentcdodds What do you think? Using something already proven (like fieldGroup) sounds better that reinventing the wheel IMHO

kentcdodds commented 8 years ago

I definitely want to see #486 implemented. Happily accepting PRs :-)

benoror commented 8 years ago

I believe I managed to fix this, along with #486

do you mind checking out a working example at http://jsbin.com/zaqeke/21/edit?html,js,output

benoror commented 8 years ago

Closed via https://github.com/formly-js/angular-formly/pull/529