rfortherestofus / omni

RMarkdown template, ggplot2 theme, and table function for OMNI Institute
https://rfortherestofus.github.io/omni/
Other
21 stars 5 forks source link

46 ggplot2 theme title and subtitle size #48

Closed LibbyHeeren closed 11 months ago

LibbyHeeren commented 11 months ago

Hi, Thomas! I updated a few things in the theme.R file so that things match the OMNI guidelines: issue #46

Data labels (from geom_text()) still look too large by default (should be same size as axis titles), and they are not Calibri by default. Unsure if that's something we can set in a theme or if the user (but that's in mm). In the meantime, I will show in the documentation how to change the font and size in geom_text() so that it's correct: family = "Calibri", size = 3.5, color = "black" or "white".

> GeomText$default_aes
Aesthetic mapping: 
* `colour`     -> "black"
* `size`       -> 3.88
* `angle`      -> 0
* `hjust`      -> 0.5
* `vjust`      -> 0.5
* `alpha`      -> NA
* `family`     -> ""
* `fontface`   -> 1
* `lineheight` -> 1.2
LibbyHeeren commented 11 months ago

Even with extra plot margin added, text is still pretty close to the edges. Margin values of at least 14 (5mm) or 28 (10mm) might be better.

dgkeyes commented 11 months ago

@tvroylandt you should feel free to comment on the text size issues that @LibbyHeeren raises.

On the issue about geom_text() defaults, I notice that the Urban Institute has a function that sets all geom defaults. Maybe we should do the same so that it's even easier for OMNI folks to use consistent fonts etc when making plots. We could also use a function like set_omni_defaults() within theme_omni() so that every time they run theme_omni() it also sets, for example, geom_text() to use Calibri at the right size, etc. Thoughts @tvroylandt?

LibbyHeeren commented 11 months ago

@dgkeyes My only concern with using update_geom_defaults() is that it sets it for the R session and persists until the user resets it (it modifies the global state). That may be what we want (sort of?), but I could see it causing some confusion if people run the function and don't realize it's set until they restart their R session, detach and reattach ggplot2, or reset the defaults explicitly back to what they were. Might not be a good idea to put anything that modifies a global inside a function 🤔

# Before running anything, this is what we get as our default geom_text defaults
> GeomText$default_aes
Aesthetic mapping: 
* `colour`     -> "black"
* `size`       -> 3.88
* `angle`      -> 0
* `hjust`      -> 0.5
* `vjust`      -> 0.5
* `alpha`      -> NA
* `family`     -> ""
* `fontface`   -> 1
* `lineheight` -> 1.2

# With update_geom_defaults() calls inside theme_omni(),  we run plot code using the theme, and then re-check the geom defaults
> GeomText$default_aes
Aesthetic mapping: 
* `colour`     -> "black"
* `size`       -> 3.5
* `angle`      -> 0
* `hjust`      -> 0.5
* `vjust`      -> 0.5
* `alpha`      -> NA
* `family`     -> "Calibri"
* `fontface`   -> 1
* `lineheight` -> 1.2
tvroylandt commented 11 months ago

@LibbyHeeren @dgkeyes About the defaults, SDCOE also have the same changes, but with a function to reset it back to defaults. I'm not specifically for or against it, it's useful when you are dealing with a single theme and difficult when you are consultants like us that use many differents settings.

I will add you to the repo Libby so that you can adapt things from there.

dgkeyes commented 11 months ago

Thanks, folks. @LibbyHeeren that is a good point about modifying global state. However, given that these are relatively inexperienced users, I think the solution @tvroylandt proposes (changing defaults, but also offering a function to change them back) makes most sense. @tvroylandt I'm creating an issue for you to add this function.