search

json-schema-form / angular-schema-form

Generate forms from a JSON schema, with AngularJS!
https://json-schema-form.github.io/angular-schema-form
MIT License
2.47k stars 653 forks source link

[Issue 769]: Fix retrieval of the correct Object during $destroy on elements within an Array #770

Closed doomie999 closed 7 years ago

doomie999 commented 8 years ago

Description

During a $destroy (e.g. when conditions evaluate to false), form.key is used to access the appropriate part of the Model in order to apply the DestroyStrategy.

This does not work for Elements within an Array, because form.key looks like this: ['array', '', 'property'] In other words, it does not have a reference to the ArrayIndex of the Element (within which contains the property to be destroyed).

My fix was to make a copy of the form.key (in order to avoid breaking anything else that depends on its original format), and then update it so that it looks like this: ['array', '5', 'property'] Where '5' here is just an example of the ArrayIndex.

This "updated" form.key is then used in the sfSelect to correctly access the appropriate part of the Model to be destroyed.

Fixes Related issues

@json-schema-form/angular-schema-form-lead

Anthropic commented 8 years ago

@doomie999 thanks for the PR, does this still work if there is an array within an array?

doomie999 commented 8 years ago

@Anthropic Unfortunately, this fix doesn't work correctly on Arrays nested within Arrays.

I used scope.$index as the ArrayIndex of the Element in the Array. Thus, when a $destroy is triggered on a nested Array, the scope.$index picks up the ArrayIndex in the nested Array, and my fix incorrectly sets that as the ArrayIndex of the parent Array as well.

Having said that, I was actually able to access the ArrayIndex of the parent Array by using scope.$parent.$parent.$parent.$index, but this didn't feel very elegant to me.

Would you be able to offer any advice on how to access the ArrayIndex of the parent Array?

Anthropic commented 8 years ago

@doomie999 take a look at #742 it adds arrayIndeces which would give you the sub array path. I just merged it into the webpack-babel branch, if you re-target that branch it will be available to you. The changes may not be super clear on how to test it though, I am working on that at the moment and this weekend.

doomie999 commented 7 years ago

@Anthropic I took a look at #742 and the webpack-babel branch (took a while because the change in build script from gulp to npm threw me off for a bit!)

Pardon my inexperience, but I'm not quite sure how to go about using the fix for #742 in this issue. #742 creates the arrayIndices (via the same chaining of $parent as I mentioned above), but this is done in the builder, and the arrayIndices are not available in sfField (where the $destroy is done).

I could repeat the same logic written in #742 inside sfField, but that's not quite DRY. Any thoughts/suggestions?

Anthropic commented 7 years ago

@joelwkent any ideas on the best way to get your arrayIndices into sfField for a destroy?

joelwkent commented 7 years ago

Hi guys, apologies I had not seen this question until @Anthropic gave me a nudge on Gitter.

I wonder if we can move the arrayIndices code to a service that could be used by this PR and conditions, onChange etc... I'm not 100% sure how to do this but I'll have a look over the code and report back shortly.

Anthropic commented 7 years ago

@doomie999 I have committed some changes towards a solution, still have more work to do, but I think I have a way to make the details available soon-ish.

doomie999 commented 7 years ago

@Anthropic Looking forward to your sharing of the details!

I was exploring making a change to the bootstrap-decorator's array.html. Specifically, I was trying to use ng-init in the ng-repeat to persist $index to the scope via some (new) function in newArray.js.

Sadly, I didn't quite manage to get it to work, though I would be interested if you have any thoughts on this approach?

Anthropic commented 7 years ago

@doomie999 the code I added passes the index and array indices to the scope by adding a controller above the added items and then importing that data using require ?^^sfKeyController into sfField. I have added the behaviour to array and tabarray.

Anthropic commented 7 years ago

@doomie999 FYI the latest version in the ASF webpack branch seems to work better now. You can see from the debug output that the key is available and should work with destroy now.

Anthropic commented 7 years ago

As I mentioned in #769 I believe I have this working in alpha.4 at the latest, so I will close this. Turned out that I changed the ng-repeat to use $$hashKey which seems to have done the trick. Thank you so much for your help on this @doomie999 I really appreciate it, you saved me a lot of time trying out variations I would have tried myself, so thank you!