Component | Donut: Automatic label placement

[rokotyan]

No description provided.

[curran]

LGTM!

[rokotyan]

@curran Left some comments to the code

[curran]

Will do the following, as suggested in #5 (comment)

• Leverage getCSSVariableValueInPixels to get font size based on --vis-donut-central-label-font-size and vis-donut-central-sub-label-font-size
• Leverage estimateTextSize to approximate text width and height for alignment

[curran]

Maybe this other PR would be a better first step:

• Component | Donut: Half Donut Label Placement #9

Then later, on top of those changes, we could add an option to automatically calculate the offsets based on measuring/estimating text size. What do you think?

[curran]

Converted this to draft PR because:

• This other PR is cleaner: Component | Donut: Half Donut Label Placement #9
• We can layer on top of chose changes some additional changes that focus on measuring/approximating text size, making the changes in this PR obsolete (there would be no longer any need for, say halfDonutLabelOffsetY, since it would be automatically computed)

[curran]

Current status:

• managed to use estimateTextSize for centralLabel
• solved left/right cases

Next steps:

• solve top/bottom case
• account for both centralLabel and centralSubLabel in all calculations (Math.max for width)

[curran]

I did the text measuring, but it only accounts for the main part, not the parts that stick up or down

[curran]

Researching this more to get the right terminology...

The text size estimation appears to only account for the height as the distance between the baseline and the cap height. It does not account for the descender or ascender, resulting in those things getting cut off.

I wonder what the best solution here would be...

Perhaps we could bring back the idea of halfDonutLabelOffsetY as an additional offset to be applied in addition to the measured text size?

Or maybe we can modify the text height estimation to account for the descenders and ascenders? I'm not sure what that would look like. I don't think it's as straightforward as multiplying the size by some constant, because I would guess that various fonts have different values for the ratio of the cap height (which we know) to ascender height and descender height (both of which we don't know).

[curran]

Noting down the code that measured the x-height here for future reference, since we are squashing the commits and can't see the commit history:

      const fastEstimatesMode = true // Fast but inaccurate
      const fontWidthToHeightRatio = 0.52
      const centralLabelFontSize = getCSSVariableValueInPixels('var(--vis-donut-central-label-font-size)', this.element)
      const centralLabelSize = estimateTextSize(this.centralLabel, centralLabelFontSize, 0, fastEstimatesMode, fontWidthToHeightRatio)
      const centralSubLabelFontSize = getCSSVariableValueInPixels('var(--vis-donut-central-sub-label-font-size)', this.element)
      const centralSubLabelSize = estimateTextSize(this.centralSubLabel, centralSubLabelFontSize, 0, fastEstimatesMode, fontWidthToHeightRatio)

[curran]

Using getBoundingClientRect achieves the desired result.

[rokotyan]

@curran Nice!

You can keep working in your donut-label-offsets branch #9 overriding the history.

Feel free the modify the estimateTextSize function (it's current logic is pretty simple), or maybe just add some extra padding to take into account the imprecise nature of that function. It you think that won't solve the problem, you can use getBoundingClientRect. The downside of getBoundingClientRect is that it triggers a force reflow which is not good for performance.

Also please don't forget about the alignment

[curran]

Got it. Thanks for the feedback!

I made this mockup to clarify what you mean - that the sub-label should be aligned with the central label, and those should both be aligned with the edge of the half donut. I'm pretty sure this is what you mean.

Also, would we want to introduce some sort of "padding" option to apply in addition to the automatic offsets that do exact alignment? I could imagine cases where users would want the automatic alignment, but then would also want to add a tiny gap so the text is not exactly aligned.

[curran]

Closing in favor of #9