arrayIndices to allow conditions to access the array index in nested arrays

[joelwkent]

When using nested arrays it is not possible to have a condition that targets specific elements in the nested array. ArrayIndex only gives you access to the inner most array index. I raised this as issue #596

Therefore I have added an additional property available in conditions that is similar to arrayIndex called arrayIndices which is an array of the array indices. For a 3 deep nested array it equates to the AngularJS:

arrayIndices = [$parent.$parent.$index, $parent.$index, $index]

It can be used in a condition like this:

"condition": "model.grandParent[arrayIndices[0]].parent[arrayIndices[1]].child[arrayIndices[2]].age > 18

I have not altered arrayIndex to ensure it continues to work as is.

I have implemented this in the src/services/decorators.js by filtering the integers out of the form.key array.

I have also implemented it in the src/services/builder.js . Here I filter the empty strings out of the args.form.key array to establish the array depth, then used this to build the array of [$index][$parent.$parent.$index][$parent.$parent.$parent.$parent.$index]...

I have added a test to \tests\directives\schema-form-tests.js that uses the builder.js version. The test uses a 3 deep array of the structure array.object.array.object.object.array.object (so arrays in arrays, and objects in objects). The test form contains conditions targeting elements all levels of the structure.

I have checked my code conforms to the Google code style (jscs) as suggested in the contribution guidelines.

I have not yet added anything to the docs as I wanted to hear any feedback and see if people approve the term arrayIndices (as appose to arrayIndexes or another term). If this change is approved I'm happy to add to the docs.

Please let me know if you have any questions or feedback. Thanks.

[joelwkent]

Note about the Travis CI failing (test failing)
I extended the tests to cover the case of array.object.object.array. Whilst adding this test I noticed a bug in the new builder were the model is not bound correctly for properties in nested arrays. I found an existing ticket that exactly matches the issue I am seeing #590 . If I make the change suggested in #590 and set line 143 of builder.js from

state.keyRedaction = state.keyRedaction || 0;
to
state.keyRedaction = 0;

Then nested arrays display correctly with the new builder, models map and my test passes. Also changing this line does not break any existing tests.

[Anthropic]

@joelwkent Well it seems like it should be ok to add that.

I had a quick look at onClick and I think if you can add the below to your PR with arrayIndeces included it should cover onClick, then just need onChange updated too, but that doesn't have to be a part of your PR in fact it probably belongs in #374.

In submit.html:
ng-click="buttonClick($event,form,{{$index}})"> not sure if this should go a level deeper and make sure arrayIndex and arrayIndeces are available to the decorator templates, I imagine it should.

In sfField:

scope.buttonClick = function($event, form, index) {
  if (angular.isFunction(form.onClick)) {
    form.onClick($event, form);
  } else if (angular.isString(form.onClick)) {
    if (sfSchema) {
      //evaluating in scope outside of sfSchemas isolated scope
      sfSchema.evalInParentScope(form.onClick, {'$event': $event, form: form, arrayIndex: index});
    } else {
      scope.$eval(form.onClick, {'$event': $event, form: form, arrayIndex: index});
    }
  }
};

[joelwkent]

@Anthropic , I'll build an example and tests to include arrayIndex and arrayIndices in the onClick function. I'm happy to add it to the onChange as well (can do it in a separate commit in this PR so that in can be removed if someone else does it).

Regarding your code snippet, you pass $index to buttonClick to use for the arrayIndex value which is a good approach but would require an update to the decorators which is no big deal. But, to construct arrayIndices I will need to filter the form.key so I could just get arrayIndex from the form key at not extra cost and save changing the decorators.

I don't like the idea of constructing arrayIndices potentially 3 times in the condition, onChange and onClick functions but I think we do this for now, wrap it in tests, then we can refactor it later (possibly into the core as suggested by nicklasb on Gitter) and we'll have the tests to confirm it is still working as expected.

Thoughts?

[Burgov]

rats, I really need this feature. Any news?

[Burgov]

I've tested it and it works in our project. Would like to see this PR finished and merged. Anything I can do to help? It's been stale since Apr 13..?

[joelwkent]

Hi @Burgov, thanks for your interest in this PR!

ArrayIndices for conditions is complete. I was concerned that the scope for 3+ nested arrays was incorrect so I have added a 4th nested array to the tests but the tests passed so there are no issues. I will push the changes to the test today.

I've also added arrayIndices to the onClick and onChnage functions but I have not yet pushed these changes (and the associated tests) to this PR. I'm happy to do so or raise as a separate PR.

I spoke to @Anthropic yesterday regarding when this PR will be merged and he said it would most likely be after the integration of the new ASF core which is currently in it's final stages. It may be possible for the arrayIndices functionality to be released with the new core but I'd have to check with @Anthropic .

[cjrazdar]

Hi @joelwkent. Thank you for your work on this solution.

I have arrayIndices working for the condition sections of my form, but I haven't yet been able to get it working for the onClick functions. Would you be able to push your changes to this PR?

[joelwkent]

Hi @cjrazdar, thanks for reaching out. I'll be able to push the onClick changes mid next week. I'm currently away from home. The onClick array indices were complicated. Unlike conditions, the scope nesting varied with the depth of the array.

[cjrazdar]

Thanks @joelwkent! I'm looking forward to it.

[joelwkent]

Hi @cjrazdar , I have pushed the changes to make arrayIndex and arrayIndices available to onClick functions to this PR (commit ac6cee4). I have not completed the tests yet for arrayIndices in onClick functions but I can push them at a later date.

[cjrazdar]

Looks good @joelwkent! Thanks, I really appreciate it.

[cjrazdar]

Actually, for some reason, I still cannot use arrayIndices in the onClick function.

Here is how I used it:
"onClick": "sayNo(model.TestCond[arrayIndices[0]].TestAnalysisType)"

For reference, the sayNo function is passed an argument and then it writes that argument to the console. Also, I've accessed this same data for the condition on this button. The following statement works:
"condition": "model.TestCond[arrayIndices[0]].TestAnalysisType"

@joelwkent, could you provide me some insight into why the onClick function cannot use arrayIndices in my code?

[joelwkent]

Hi @cjrazdar, the onClick function is passed the following parameters:

onClick: function(event, form)

and in this PR I have now added arrayIndex and arrayIndices to make:

onClick: function(event, form, arrayIndex, arrayIndices)

So if you wanted to do an inline onClick function you could put the following in your form definition JSON:

onClick: function(event, form, arrayIndex, arrayIndices){ console.log('Function: Array Index: ' + arrayIndex + ', array indices: ' + arrayIndices.toString()); }

Or to call an existing function you could use:

onClick: "onClickTestFnc(event, form, arrayIndex, arrayIndices)"

and define the function separately like:

$scope.onClickTestFnc = function(event, form, arrayIndex, arrayIndices) {
      console.log('Controller Function onClickTestFnc: Array Index: ' + arrayIndex + ', array indices: ' + arrayIndices.toString());
};

So to do what you want to do you could use:

"onClick": "sayNo(event, form, arrayIndex, arrayIndices)"

then your sayNo function would look like:

$scope.sayNo = function(event, form, arrayIndex, arrayIndices) {
      console.log($scope.model.TestCond[arrayIndices[0]].TestAnalysisType);
};

This is the way I have been using onClick and works for me but others may have different approaches. Hope this helps.

[Anthropic]

@joelwkent and I have discussed the option of using the following signature:
function(event, form, { array: {index: 1, indices: [0,3] }}, locals) {
By providing an object rather than separate parameters it future proofs the addition of elements as they may arise.
locals is a place holder for when we add a scope for managing conditional state.

[lopsided]

Any update on this? I could really do with this feature!

[Anthropic]

@lopsided have you tried @joelwkent 's implementation as it stands now?
Just wondering if it works for you :)

Joel can you make another PR to target the webpack branches and I will just accept them and we can iron it out and make changes there?

[lopsided]

@Anthropic I gave it a go today but I had lots of problems, I don't think any of them were related to this PR but probably mostly to do with changes in the develop branch. Either way, haven't got it to work yet. Will try again tomorrow.

[lopsided]

I copied the changes I can see in this PR over the top of the 0.8.13 file and still can't get it to work.

I have made a plunkr here: https://plnkr.co/edit/TOgHtfTJSBCVU7EVSC4z?p=preview

Hopefully it is clear enough from this what I am trying to achieve. Is the idea that this PR should make this work? Or am I in the wrong place...?!

[lopsided]

OK I've made some progress - I was missing the model. prefix to the condition, adding this makes it work :) Although it still doesn't work for the conditional wrapper, only for the fields individually. I can live with that. (Updated the plunkr to show this)

[joelwkent]

Hi @lopsided , thank you very much for testing out this PR! I took a look at your plunker and I think you just need to change 'type': 'conditional' to 'type': 'section' (conditional is now depreciated) and give the section a key :

{
  'type': 'section',
  'key': 'contract_items[].details[].start_date',
  'condition': 'model.contract_items[arrayIndices[0]].details[arrayIndices[1]].enabled',
  'items': [{
      'key': 'contract_items[].details[].start_date',
      'dateFormat': 'dd/MM/yyyy',
    }, {
      'key': 'contract_items[].details[].end_date',
      'dateFormat': 'dd/MM/yyyy',
    }, ]
},

Thanks again for trying it out and letting us know that it works for you. As @Anthropic suggested I have started to port this PR to the webpack branch and I will raise a new PR shortly with arrayIndices in the webpack branch.

[lopsided]

Hi @joelwkent no problem! I changed conditional to section (thanks for pointing out the deprecation) but it still doesn't work - see updated plunkr. Did you mean to add a key for it? According to the docs it doesn't need one.

[lopsided]

@joelwkent sorry, hate to ask, but do you have an eta for this? I'm going to deploy to prod soon and would love to not be using my hacked branch! Thanks :)

[joelwkent]

Hi @lopsided I have raised a new PR which targets the webpack branch. As @Anthropic mentioned this new PR should be able to be merged sooner as it aligns with the current development direction. The new PR #742 is just for arrayIndices in conditions, I will raise additional PRs for arrayIndices in onClick and onChange shortly.

I am going to close this PR now as it is replaced by #742