kurkle / chartjs-chart-treemap

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

Fix borders and spacing on high dpr displays #142

Closed kurkle closed 2 years ago

cloudflare-workers-and-pages[bot] commented 2 years ago

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3a4e999
Status: ✅  Deploy successful!
Preview URL: https://4e5603ee.chartjs-chart-treemap.pages.dev
Branch Preview URL: https://borders.chartjs-chart-treemap.pages.dev

View logs

github-actions[bot] commented 2 years ago

Size Change: +866 B (+4%)

Total Size: 23 kB

Filename Size Change
dist/chartjs-chart-treemap.esm.js 8.47 kB +330 B (+4%)
dist/chartjs-chart-treemap.js 8.71 kB +331 B (+4%)
dist/chartjs-chart-treemap.min.js 5.87 kB +205 B (+4%)

compressed-size-action

stockiNail commented 2 years ago

Therefore it was related to dpr?

kurkle commented 2 years ago

Therefore it was related to dpr?

yes, it is. this is still not good enough though.

stockiNail commented 2 years ago

Apologize but this evening I was busy. I was thinking to fallback to the previous logic drawing border over the background. Anyway I dont understand how it could work in chartjs for the bars

stockiNail commented 2 years ago

I have also to admit that I will never discover it was related to dpr…

kurkle commented 2 years ago

borderWidth: 0.5

DPR=1

image

DPR=2

image
kurkle commented 2 years ago

ok, fixed the borderWidth: 0.5, dpr=1 case image

image

Still got some issue with rtl

image

image

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

86.9% 86.9% Coverage
0.0% 0.0% Duplication

stockiNail commented 2 years ago

It looks like a very annoying issue... but FMI, what do you think it has generated this issue? Some updates? was always there?

kurkle commented 2 years ago

It looks like a very annoying issue... but FMI, what do you think it has generated this issue? Some updates? was always there?

Looks like it broke (for docs) in this commit: https://github.com/kurkle/chartjs-chart-treemap/commit/9deba20de5e9a528bf3830ec7935a9e728c09684

I guess it is some change in Chart.js from 3.7.0 to 3.9.1

EDIT: oops, I thought this was a discussion in issue #146

kurkle commented 2 years ago

It looks like a very annoying issue... but FMI, what do you think it has generated this issue? Some updates? was always there?

To answer in this scope, I think it was always there, but as the bordes were drawn on top of the background, it was not so obvious / annoying.

There is a similar issue in chart.js bar charts, and there is a inflateAmount option used to address it. Datalabels uses the same "rasterize" technique to keep fonts sharp (this should probably be applied to label drawing too)

stockiNail commented 2 years ago

EDIT: oops, I thought this was a discussion in issue #146

:D