search

awendland / angular-json-tree

Angular directive for displaying a JSON object in an expandable tree structure
Other
79 stars 41 forks source link

fix ngRepeat:dupes error #13

Closed CrimsonGlory closed 5 years ago

CrimsonGlory commented 8 years ago

used $index+subval instead of $index to force the tree to update when a value changes.

awendland commented 8 years ago

Hey @CrimsonGlory! Thanks for submitting this PR 😄

Would you mind detailing some of the reasoning behind this change as well as what this change impacts, what benefits it brings, and what potential gotchas may result from it?

I'm looking forward to merging it in. Thanks, again.

CrimsonGlory commented 8 years ago

With the following json already works ok on master branch:

{
  "id": "0001",
  "some_array": [1,2,3]
}

But this currently fails:

{
  "id": "0001",
  "some_array": [1,2,3,3]
}

That ng-dupes error can be fixed by adding track by $index but this make the first item in the json tree not render correctly. track by ($index+subvalue) fixes this but I experienced some rare bugs I wasn't able to isolate. This patch uses track by ($index+subkey+subvalue) which works correctly on all cases I tried. but I still don't fully get the track by stuff.

awendland commented 8 years ago

That ng-dupes error definitely seems to be a problem with the current version.

One question, one request:

Question: why can't we just use $index for tracking? Won't it ensure that duplicates get shown because they are different entries in the backing values array/keys array?

Request: would you mind adding some test cases to the karma test suite? If you're having difficulty with the test setup, would you mind posting q couple of the JSON strings that you're testing with and describing what currently fails and how it should work?