kurkle / chartjs-chart-treemap

Chart.js module for creating treemap charts
MIT License
139 stars 34 forks source link

Remove tree option using only data one #137

Open stockiNail opened 2 years ago

stockiNail commented 2 years ago

Fro @kurkle https://github.com/kurkle/chartjs-chart-treemap/pull/121#issuecomment-1258534332

Remove tree option to pass the data to the chart but only data option, like all other charts.

kurkle commented 2 years ago

This would probably be best done together with #122, by moving buildData to parse and using the parsed data in updateElement.

stockiNail commented 2 years ago

I agree. I thought several times to that (I did some tests for me to understand how parse method it's working). There are a couple of "issues" that we have to share:

  1. we don't solve the issue about the unresolved options of the element (like borderWidth and captions) because there are needed to calculated the space and then the values of the element.
  2. we will do a double cycle (one in the parse data and the other in updateElements) for the real calculation of the element size.

Furthermore, if the parse method will only parse the data, normalizing those, probably we need to maintain the hierarchy, needed in updateElements (for instance the parent or children of parsed data, not the original data). I didn't check it (I was busy in last days).

stockiNail commented 2 years ago

@kurkle to address better the options topic, do you think we could implement a configure method where the controller options have got a different scopes hierarchy? Now, they are using 'datasets.treemap', '', adding 'datasets.treemap', 'elements.treemap', '' we could access to the element options resolving to the element config (like captions, spacing, rtl, borderWidth, needed in buildData phase).