Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vertical space between legend keys changed after switching theme? #3180

Closed
tungmilan opened this issue Mar 7, 2019 · 13 comments · Fixed by #3414
Closed

Vertical space between legend keys changed after switching theme? #3180

tungmilan opened this issue Mar 7, 2019 · 13 comments · Fixed by #3414
Labels
bug themes 💃 tidy-dev-day 🤓

Comments

@tungmilan
Copy link

@tungmilan tungmilan commented Mar 7, 2019

Hello,

I noticed there was difference in the vertical space between legend keys when switching between themes e.g. from theme_grey to theme_bw (see the reprex below). I wonder what was the cause of that?

Thanks!

### ggplot2 3.1.0.9000 (2019-03-06)
library(ggplot2)

theme_set(theme_grey(base_size = 14))
p1 <- ggplot(mtcars) +
  aes(fill = factor(cyl), x = cyl) +
  geom_bar() +
  theme(
    legend.key = element_rect(size = 6),
    legend.key.size = unit(1, "cm")
  )
print(p1)

theme_set(theme_bw(base_size = 14))
p2 <- ggplot(mtcars) +
  aes(fill = factor(cyl), x = cyl) +
  geom_bar() +
  theme(
    legend.key = element_rect(size = 6),
    legend.key.size = unit(1, "cm")
  )
print(p2)

@ptoche
Copy link

@ptoche ptoche commented Mar 8, 2019

each theme defines/redefines various properties. You can see the defaults here

Possibly the reason for what you see is here: theme_bw is based on theme_gray. The following is defined by theme_gray:

legend.key = element_rect(fill = "grey95", colour = "white")

and redefined by theme_bw:

legend.key = element_rect(fill = "white", colour = NA),

That, I'm guessing, explains the extra white color between legend keys.

To test that, use theme_update() with the legend.key redefined and see if you can match the plots.

P.S. I haven't looked at the source code, but setting colour = NA possibly removes some padding. Looks like it would be innocuous to change that to colour = white and, if everyone agrees, the legend would look "better" with that extra white space in between keys. Or not?

@ptoche
Copy link

@ptoche ptoche commented Mar 15, 2019

It's down to the difference between colour = "white" and colour = NA:

library(reprex)
library(ggplot2)
theme_set(theme_bw(base_size = 14))
theme_update(legend.key = element_rect(size = 6, fill = "white", colour = NA), legend.key.size = unit(1, "cm"))
ggplot(mtcars) +
    aes(fill = factor(cyl), x = cyl) +
    geom_bar() 

Created on 2019-03-15 by the reprex package (v0.2.1)

library(reprex)
library(ggplot2)
theme_set(theme_bw(base_size = 14))
theme_update(legend.key = element_rect(size = 6, fill = "white", colour = "white"), legend.key.size = unit(1, "cm"))
ggplot(mtcars) +
    aes(fill = factor(cyl), x = cyl) +
    geom_bar() 

Created on 2019-03-15 by the reprex package (v0.2.1)

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Mar 15, 2019

Setting the color to black makes it clearer what happens. The spacing doesn't change, but there's an additional border that is drawn and that covers some of the key parts. Maybe we should remove this border for theme_bw()?

library(ggplot2)
theme_set(theme_bw(base_size = 14))
theme_update(legend.key = element_rect(size = 6, fill = "white", colour = "black"), legend.key.size = unit(1, "cm"))
ggplot(mtcars) +
  aes(fill = factor(cyl), x = cyl) +
  geom_bar() 

Created on 2019-03-15 by the reprex package (v0.2.1)

@tungmilan
Copy link
Author

@tungmilan tungmilan commented Mar 15, 2019

Thanks @ptoche & @clauswilke ! That makes sense. So is there a way to make the size of those legend boxes the same? The blue key box is noticeably larger than the others.

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Mar 15, 2019

I would recommend to just set colour = NA in the theme element. We should probably make that fix in the ggplot2 code base also.

library(ggplot2)
theme_set(theme_bw(base_size = 14))
theme_update(legend.key = element_rect(size = 6, fill = "white", colour = NA), legend.key.size = unit(1, "cm"))
ggplot(mtcars) +
  aes(fill = factor(cyl), x = cyl) +
  geom_bar() 

Created on 2019-03-15 by the reprex package (v0.2.1)

@tungmilan
Copy link
Author

@tungmilan tungmilan commented Mar 15, 2019

Thanks @clauswilke ! Now I understand. Would be great if this is fixed int the future release

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

@clauswilke

Maybe we should remove this border for theme_bw()?

I would recommend to just set colour = NA in the theme element. We should probably make that fix in the ggplot2 code base also.

Sorry, I'm confused... Are you talking about theme_gray()? theme_bw() doesn't have borders, and theme_gray() does.

library(ggplot2)

theme_bw()$legend.key$colour
#> [1] NA
theme_gray()$legend.key$colour
#> [1] "white"

Created on 2019-03-18 by the reprex package (v0.2.1.9000)

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Mar 18, 2019

I may have mixed them up. The borders seem to be causing trouble (uneven glyph size), so it may be better to remove them and to create spacing between the keys in a different manner.

@tungmilan
Copy link
Author

@tungmilan tungmilan commented Mar 18, 2019

@clauswilke & @yutannihilation : is there any reason why ggplot doesn't have an option to manipulate vertical distance between legend key boxes? Thanks!

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Mar 18, 2019

There's no good principled reason. It's just that the way legend drawing is currently implemented, adding variable vertical spacing between legend key boxes will not be trivial to implement.

@ptoche
Copy link

@ptoche ptoche commented Mar 27, 2019

Looking at it again, setting colour = NA looks like the correct thing to do: Had you noticed how the blue square in the legend appears taller than the other two squares? (I hadn't the first time around) It seems that the covering line (the black line in Claus's example) is not vertically centered, thus creating the undesired effect. Must be fixed.

@paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Jun 17, 2019

If I'm reading the consensus correctly, I think we can close this particular issue by ensuring that legend.key is an element_rect() with colour = NA in all of the built-in themes.

I think this only occurs once, in the theme_grey() definition (on which most other themes inherit).

legend.key = element_rect(fill = "grey95", colour = "white"),

This will probably cause a lot of dopplegangers to change, which would also have to be validated as part of the PR (just describing this as I'm about to tag it for tidy developer day).

@paleolimbot paleolimbot added the tidy-dev-day 🤓 label Jun 17, 2019
paleolimbot pushed a commit that referenced this issue Jul 8, 2019
* Changed theme_grey() setting for legend key so that it creates no border (NA) rather than drawing a white one.

* update visual test cases for theme_gray() updates
@lock
Copy link

@lock lock bot commented Jan 4, 2020

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Jan 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug themes 💃 tidy-dev-day 🤓
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants