fix legend entry text size with theme grey

[tdhock]

Hi @Faye-yufan I noticed an unexpected inconsistency while re-building the animint2 manual.
See screenshot below, notice how legend entry sizes are too small in the second plot. I added a test which says both should be 16px. Can you please investigate and fix?

[tdhock]

Also I noticed the "a" character which is drawn in the tiny svg to the left of each legend entry is too small to read, currently 3.88 -- can you please add a test which fails for the current code and then fix? (this is actually a separate issue which has existed for a long time, but shows up clearly in the test case that I created for this PR, so seems like a good opportunity to fix)

[tdhock]

Another inconsistency that is clear from this test case is that there are black axis lines drawn on the first plot but not on the second. For consistency with default ggplot2 appearance we could remove the black axis lines from the first? What do you think?

[Faye-yufan]

Appreciate the feedback.

• legend size bug for theme_grey() and other complete themes shall be a quick fix, I hadn't thought of this when coding Support for specifying legend/axis font size via theme() #64
• It's interesting to see the extremely small legend svg occurs, it takes the GeomText default_aes = aes(size = 3.88). I think it should be fixed on JS side.
• Agree to remove the black xline, maybe I'll resolve it in a new PR? Seems not much related to this one.

[tdhock]

thanks for this quick fix!
However I don't think the logic of checking for a complete theme is correct.
I added a new test case to illustrate.
If you use a complete theme and then overwrite the legend.text size, then that value of text size should be used, even though it is a complete theme. (current code takes the default value)
Can you please try a different fix that can handle this as well?

[Faye-yufan]

Thanks for the info!
I'm trying to figure out how to distinguish if the legend text size was input or by default.

[tdhock]

I don't think the logic should care whether or not the size was default. Also I think the default value of 16 should be specified in the theme, not in the getLegendList function. But we previously observed that changing the theme value to 16 results in static ggplot text which is too big.
The tricky part is that ggplot2 text size units are in points/pts (see ?gpar and ?element_text), which is not the same as animint2/svg text size units (px/pixels).
One solution may be to to try using "pt" instead of "px" in animint JS code. "pt" does appear to be a valid CSS unit, https://developer.mozilla.org/en-US/docs/Web/CSS/font-size#values can you please try that? In that case we probably want a default value smaller than 16 (since 16pt is probly a lot bigger than 16px).

[Faye-yufan]

Interesting, thanks a lot! Will do.

[Faye-yufan]

8.8pt(ggplot theme default) seems still smaller than 16px(animint default) on my screen.

And I start considering that px is still the better option after reading this article https://www.w3.org/Style/Examples/007/units.en.html since pt is the absolute units in CSS, it is not as good as px except for printed output, the legend entry text size might not be aligned with other elements of the plot.

The magic unit of CSS, the px, is a often a good unit to use, especially if the style requires alignment of text to images, or simply because anything that is 1px wide or a multiple of 1px is guaranteed to look sharp.

Indeed, it looked inappropriately large on a static plot if I modified the theme. How about magnifying the font size proportionally to px, based on ggplot theme output? something like this:

.style("font-size", function(d){ return d["text_size"] * 1.82 + "px"})

[Faye-yufan]

Now the rules of legend and axis text size are:

1. Default using ggplot2 theme_grey() text size, with unit pt in CSS styles
2. If use numeric/number to define the text size, the unit would be pt
3. If define with a character/string, it must end with px or pt

The default is different from what animint previous looked like.

[tdhock]

this is a good test

[tdhock]

This looks great, thanks for the fix. OK to merge now if you like, after you update DESCRIPTION/NEWS.
Also for a future PR: it is possible for the user to specify other text sizes in this way? (axis names, axis tick labels, facet titles, plot title)

[Faye-yufan]

Yes, it is possible and easier to specify other text since we had settled down the font size strategy. Thanks!