Add support for legend/axis text size#66
Conversation
…s.text' var as parent of 'axis.text.%s' but rel() still not working
…me defined text size is still not supported
|
Font size, the number, is able to be extracted from Maybe it's because the Also, |
…el() in it. Modify the logic of legend text size default to 16px
| getStyleValue(info1$html, '//g[@class="yaxis axis yaxis_1"]//g[@class="tick major"]//text', | ||
| "font-size") | ||
| expect_match(axis.x.style.value, "20px") | ||
| expect_match(axis.y.style.value, "12px") |
| #' @return size | ||
| axesTextSize <- function(xy.size, theme.pars){ | ||
| text.size <- if(is.null(theme.pars[["text"]]$size)){11}else{theme.pars[["text"]]$size} | ||
| axis.text.size <- if(is.null(theme.pars[["axis.text"]]$size)||theme.pars[["axis.text"]]$size==rel(0.8)){ |
There was a problem hiding this comment.
In the output, the axis.text default to 0.8, and printed out as 0.8 * because this line of code in theme-element.r
There was a problem hiding this comment.
This function should be modified to be more clear and less fragile.
| axis.text <- theme.pars[[s("axis.text.%s")]] | ||
| ## TODO: also look at axis.text! (and text?) | ||
| size <- axesTextSize(axis.text$size, theme.pars) | ||
| size <- calc_element(s("axis.text.%s"), theme.pars)$size |
There was a problem hiding this comment.
this looks like a step in the right direction, good job!
|
code looks good but can you please paste a screenshot of the test cases after rendering them with bigger text size? I want to make sure the web page looks reasonable / readable even after playing with the text size. (I know you have expect_ statements that check for the size of individual elements that you resized, but it is also importantant to check that the plot is still OK overall) |
|
For documenting these new features probably would be good to add to Plot-specific options under Chapter 6 Animint options, https://rcdata.nau.edu/genomic-ml/animint2-manual/Ch06-other.html#plot-options |
|
Got it, will write the document after it merges. |
|
looks great, thanks very much, please merge! |
| legend.text = element_text(size = rel(0.8)), | ||
| legend.text = element_text(size = rel(16/11)), | ||
| legend.text.align = NULL, | ||
| legend.title = element_text(hjust = 0), | ||
| legend.title = element_text(size = rel(16/11), hjust = 0), |
There was a problem hiding this comment.
hi @Faye-yufan did you know that these changes to default text sizes result in much larger defaults? Here are some comparisons of old (left) vs new (right)
was this intentional? (I did not notice these changes in my initial review of this PR)
These new defaults seem to be often too large for the context of the animint2 manual, so I am thinking of changing them back to the original values for the animint2 manual, and I wonder if we should change the defaults back as well, or keep these new bigger defaults? what do you think?
There was a problem hiding this comment.
Hi @tdhock thanks for the info! I did not think of this situation before.
For here, I believe it's because the plot is printed by the data generated by ggplot_build and did not render to html, so the text size gets bigger in ggplot.
I changed the default text sizes for the convenience of rendering html, since previously animint set legend text size to 16px and I just thought the change to rel(16/11) would pass 16 as default to html.
Yes, I think we should change the defaults back in ggplot default, and change our default text size in JS side.
There was a problem hiding this comment.
the top image was animint/html and the bottom was static/ggplot_build/png.
that is a good idea to change back the defaults in R code (so then static/ggplot_build/png looks the same), and also to change the defaults in JS (so the animint/html is more reasonably sized). Please create another PR for that.
2 participants
Enable
sizeoptions inaxis.text.x,axis.text.y,legend.textto be aligned with ggplot2 syntax. Try to enableaxis.textandtextas well, it may require more logic in compiler.closes #64