search

ONSvisual / Simple-charts

Simple responsive charts
17 stars 12 forks source link

Aspect Ratio Bug #54

Open giacomobg opened 4 years ago

giacomobg commented 4 years ago

I noticed in the Line template that we take the width and calculate the height with an aspect ratio, but then subtract the height of the margins.

This means as far as I can see we are including the margin in the aspect ratio, which is a mistake since the margin does not scale.

I think this extends to other charts. This might explain some oddly thick or thin bars at different sizes that I noticed in the past in the stacked bar horizontal.

henryjameslau commented 4 years ago

Agree, should not include margin top or bottom var height = Math.ceil((chart_width * dvc.optional.aspectRatio[size][1]) / dvc.optional.aspectRatio[size][0])

henryjameslau commented 3 years ago

Also applies to clustered bar chart horizontal. Height should be set through aspect ratio or barHeight and the svg height should be the height + margin.top and margin.bottom