kurkle / chartjs-plugin-gradient

Easy gradients for Chart.js
MIT License
37 stars 6 forks source link

Enable gradient styles on the legend items #29

Closed stockiNail closed 2 years ago

stockiNail commented 2 years ago

Fix #28

This PR is enabling to set fillStyle and strokeStyle on the legend items related to datasets where the gradient has been set.

It is applying only backgroundColor and borderColor, if set.

It doesn't go in exception on CHART.JS version 2.x but it can not enable the styles on legend items because the legend size calculation is done in a different time of chart lifecycle.

TODO

stockiNail commented 2 years ago

Impressive!

I had some comments and I think some more tests with scales not starting from zero. I think we need to cover all these scale cases: 0-positive (covered) negative-0 positive-positive negative-positive negative-negative

Yes, I put it in Draft because there a couple of things where I have to pay attention:

1. change the calculation of percentage using getPixelStop (unique way everywhere) 2. understand if the afterDraw hook is the right one to change the legend. I think it could be a problem if the animation is disabled 3. destroy the state when the chart is destroying

This afternoon I didn't have time to go on. I'll do asap.

EDIT: hopefully I have time tomorrow. Sorry :(

stockiNail commented 2 years ago

I think we need to cover all these scale cases: 0-positive (covered) negative-0 positive-positive negative-positive negative-negative

Done. Maybe I could add the same tests on radial linear (done only cartesian linear).

EDIT: added the additional test cases to the radial linear axis.

stockiNail commented 2 years ago

@kurkle 41 files.... I know you don't like big PR. Sorry for that.

EDIT: 42!

stockiNail commented 2 years ago

I'm not sure about the state handling, but that is internals and can be changed at any time.

OK. I can review later. I tought about the structure, using map. And also an another issue was that even if the dataset is hidden, the state should be present for the legend.