fix: unnecessary space on the left side

[il3ven]

Fixes #33

Solution:

The idea is to change the offsetX variable to the width of the yLabel instead of using a fixed value. This width is not in absolute pixel but relative to the given viewBox.

The function getWidth is based on the assumption that the font-size is kind of the height of the character and the width is proportional to the height. I couldn't find any spec for this but based on my tests I think it true at least for our use case.

I have tried inputting different values and the getWidth function holds. (See Tim's comment)

Note

Due to this line we cannot specify the fontSize in em, cm or px. 1.5px will throw an error.
https://github.com/il3ven/svg-line-chart/blob/813b3d02812a968975d0eab28a6b006af0c2e146/src/index.mjs#L405

Do we need to normalize 1.5px to 1.5 instead of throwing an error?

[TimDaub]

I don't think isNan exists: https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/isNaN

[il3ven]

Yeah, I meant isNaN.

…

On Sun, 7 Nov 2021, 15:49 Tim Daubenschütz, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/index.mjs <#34 (comment)> : > @@ -398,3 +400,11 @@ export function generateLabelRange(min, max, numLabels) { return labels; } + +function getWidth(fontSize, dataPoints) { + const characterHeight = Number(fontSize) // This height is relative to viewbox and not in px + if(isNan(characterHeight)) throw new Error('Invalid fontSize') I don't think isNan exists: https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/isNaN — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#34 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBDAI2RSMS6MDCEBDAIYHDUKZHCRANCNFSM5HP54LPQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[TimDaub]

I took a few screenshots to document the behavior under different conditions

I think it's a good approach. I'm happy if we continue with it.

[TimDaub]

Declare magic constants as variables.

I tend to prefer e.g calling the padding you added in the return by what it is const padding = 2 and then later in the return use it.