Pass index to nodeView

[janwirth]

I need to know the position of my branch among its siblings. It would be good if Context included a position : Int. Right now I have to build the indexes manually after every change.

[peterszerzo]

I can see how this is can be very helpful, thanks for the suggestion @FranzSkuffka! I have to see what the most sensible solution here could be, among the following options:

1. We could make the context.siblings list include the node itself at the right position, allowing you to calculate the position through a List.Extra.findIndex. This comes with a slight perf hit but keeps Context simple. Could run into conflicts with identical nodes, but this is an advanced use case and it seems like an acceptable rough edge.
2. Expose position: Int like you're suggesting. There's no perf hit because this information is in scope at the time nodeView is called. Major version bump is necessary.
3. Expose the entire tree traversal path to get to the node. No perf hit for the same reason as in point 2. above. Major version bump here as well.

Do you have thoughts on what is a good design here?

[janwirth]

I think

1. would change the API so a major version bump would be necessary anyways. It could cause off-by-one errors in those who consume the package. I was expecting the node itself to be included but I'm out of luck.
2. Completely would satisfy my use-case. are you sure you can not extend types without a version bump?
3. Not sure if it's needed.

I'm for 2 just because it's the simplest version.

[janwirth]

In my use case, my parent node holds a list of labels for the children. (It's a 'split' node.) The child node needs to render the label because I want to benefit from the layouting algorithm. In this implementation, the child node would get its label from the list of labels of the parent using its position.

[janwirth]

I'm also for 2. because it's a common use-case similar to indexedMap.

[peterszerzo]

You might be right that there is no need for a version bump, I'll take a look. Thanks for all the input!

[janwirth]

Thanks for the library :)

[peterszerzo]

@FranzSkuffka sorry for taking so long on this, could not find the time to work this out over the summer. If you still need the feature, I am happy with the position solution, which you can implement as follows:

• the position information you need is the last member of the path list in this function: https://github.com/peterszerzo/elm-arborist/blob/master/src/Arborist.elm#L931.
• adding position: Int to the context object should be the only thing left that you need to compile: https://github.com/peterszerzo/elm-arborist/blob/master/src/Arborist.elm#L1439.

Let me know if you have any other questions.

[janwirth]

I have solved the problem on our side already, I probably won't dedicate any time to resolving this anytime soon.

[peterszerzo]

Alright @FranzSkuffka, I'll see if there's anybody else who needs this in the near future and then I'll make a call on adding it.