Fix the rectangles size calculation aligning to bar element logic

kurkle / chartjs-chart-treemap

Chart.js module for creating treemap charts

MIT License

139 stars 34 forks source link

Fix the rectangles size calculation aligning to bar element logic #131

Closed stockiNail closed 2 years ago

stockiNail commented 2 years ago

FIX #126

This is PR is fixing definitely the rectangles size calculation leveraging on the copy of Chart.js BarElement code, where borderWidth (as object) and borderRadius are correctly managed.

This also simplifies some parts of the code and re-enables the borderRadius as scriptable options.

TODO

[x] waiting for #127 and #128

stockiNail commented 2 years ago

looks great!

Yes definitely.. I will go on. major version or can we think to add to next minor? I dont see breaking changes till now

kurkle commented 2 years ago

I think this can be included in a minor

kurkle commented 2 years ago

needs a rebase

stockiNail commented 2 years ago

rebased

kurkle commented 2 years ago

Borders look quite bad now:

Compared to previous:

stockiNail commented 2 years ago

Weird… now it should be aligned with bar element. Let me have a look tomorrow. Also weird that test cases ended well

stockiNail commented 2 years ago

It looks like the borderColor is not applied

stockiNail commented 2 years ago

There is a line that it should not be there too

kurkle commented 2 years ago

I think the border was previously drawn over background, hence different color. But the artifact are due to devicePixelRatio and canvas weirdness with clipping.

stockiNail commented 2 years ago

I think the border was previously drawn over background, hence different color.

Yes, this morning I thought it. Nevertheless it could be a breaking change, even if this logic sounds correct to me because it maintains the borderColor of the config... Apologize if I didn't recognize before.