MatthewBJane / ThemePark

Fun ggplot themes for popular culture
http://matthewbjane.com/ThemePark/
MIT License
198 stars 13 forks source link

legend.key fill #29

Open ASKurz opened 1 year ago

ASKurz commented 1 year ago

I think you're gonna want to adjust the fill setting for the legend.key for theme_barbie(). Here's an example of what currently happens:

library(tidyverse)
library(ThemePark)

mtcars %>% 
  ggplot(aes(mpg, wt)) +
  geom_point(aes(shape = factor(cyl))) +
  theme_barbie()
Screenshot 2023-08-13 at 7 01 00 PM

See the default gray background in the legend key? Here are two possible fixes:

mtcars %>% 
  ggplot(aes(mpg, wt)) +
  geom_point(aes(shape = factor(cyl))) +
  theme_barbie(barbie_font = FALSE)  +
  theme(legend.key = element_rect(fill = "white"))

mtcars %>% 
  ggplot(aes(mpg, wt)) +
  geom_point(aes(shape = factor(cyl))) +
  theme_barbie(barbie_font = FALSE)  +
  theme(legend.key = element_rect(fill = barbie_theme_colors["panel"]))
Screenshot 2023-08-13 at 7 02 17 PM Screenshot 2023-08-13 at 7 02 30 PM
ASKurz commented 1 year ago

Here's a more radical option:

mtcars %>% 
  ggplot(aes(mpg, wt)) +
  geom_point(aes(shape = factor(cyl))) +
  theme_barbie()  +
  theme(legend.background = element_rect(fill = "white"),
        legend.key = element_rect(fill = barbie_theme_colors["panel"]))
Screenshot 2023-08-13 at 7 21 27 PM
MatthewBJane commented 1 year ago

I actually very much like the "radical" one. I will add this. This one is a bit tedious so I will probably do this tomorrow. I will follow up on this issue once it is done. Thanks for finding these flaws, Solomon!

ASKurz commented 1 year ago

Glad to be of service.

ASKurz commented 1 year ago

Update: To get it right, I think it might have to be this:

mtcars %>% 
  ggplot(aes(mpg, wt)) +
  geom_point(aes(shape = factor(cyl))) +
  theme_barbie()  +
  theme(legend.background = element_rect(fill = "white"),
        # note we're adjusting both fill and color settings
        legend.key = element_rect(fill = barbie_theme_colors["panel"],
                                  color =  barbie_theme_colors["panel"]))
MatthewBJane commented 1 year ago

totally forgot to come back to this... @christopherkenny or @lukepilling, is this worth implementing?

lcpilling commented 1 year ago

Seems like yes for Barbie. Would we need to check the other themes to make sure it didn't look "bad"? Or just implement the solution throughout?

MatthewBJane commented 1 year ago

Idk that seems like a huge pain. Probably implementing it in Barbie would be fine for now. Unless you or Chris want to try to implement it across the board. I don't think it is necessary for now though

christopherkenny commented 8 months ago

FYI, we could clean this up if we have specific choices, but ggplot2 version 3.5.0 has this as a breaking change (which fixes the ugly greys)/

We can see how that looks below: themes_test.pdf

Code to generate ``` devtools::load_all() library(ggplot2) library(patchwork) list_fn <- function(prefix) { fns <- utils::lsf.str("package:ThemePark") fns[startsWith(fns, prefix)] } p <- lapply(list_fn('theme_'), function(fn) { theme <- getFromNamespace(fn, 'ThemePark') mtcars |> ggplot(aes(mpg, wt)) + geom_point(aes(shape = factor(cyl))) + theme() + labs(title = fn) }) |> wrap_plots() ggsave(plot = p, filename = 'themes_test.pdf', width = 48, height = 36) ```
christopherkenny commented 8 months ago

And here's a png version, since it doesn't show as pdf. themes_test