Rework legend spacing

[teunbrand]

This PR is a first step at refactoring #5283 to be less aggressive in terms of visual changes.
Inadvertently, it also fixes #5455.

Briefly, this PR detaches the spacing from keys to labels and from titles to guides from the legend.spacing.{x/y} setting. Instead, the legend text/title's margin argument controls this particular spacing. It technically introduces a visual change, but keeps the defaults close to what they are.

Some details; this idea was coined by Claus in #3587 (comment). The gist of this PR is that it looks if user declared an explicit margin in the theme or guide, and if they didn't, it sets a default margin that is visually equivalent to current defaults. Since it now omits legend.spacing.{x/y} between key and label, this means legend.spacing.{x/y} is solely dedicated to the spacing between key-label pairs. In contrast to #5283, this wouldn't need to introduce a new text spacing concept, as it is captured in the margins.

There are two visual changes in the tests:

• One related to fixing Bug: rotated text in guide_colourbar() #5455.
• One related to guide-to-guide spacing, which I don't quite understand at the moment.

The reprex from #5455 with this PR:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot(mpg, aes(displ, hwy)) +
  geom_point(aes(colour = cty)) +
  scale_colour_continuous(
    breaks = c(10, 20, 30),
    labels = c("A", "B\nB", "C\nC\nC")
  ) +
  theme(
    legend.text = element_text(angle = 270, vjust = 1)
  )

^{Created on 2023-10-05 with reprex v2.0.2}

[clauswilke]

I think this is a great idea! If you can do this without major regressions that's even better.

[teunbrand]

I think this is a great idea!

Well it was originally yours to begin with!
If you have some gnarly situations in mind, I'd be happy to explore if this works. Most visual tests are fine though, but they don't deviate from the defaults too much.

[clauswilke]

I don't have any specific tests in mind, I'm just thinking about the many ways I've worked around the previous behavior and those workarounds may now produce weird results. But I don't have any specific example at hand. It's mostly stuff I did when I wrote my book, which is now already almost five years ago.

[thomasp85]

I like this route, but we need to figure out the cause for the guide spacing regression and find a solution

[teunbrand]

The guide-to-guide spacing thing prompts the question though whether there should a separate theme element to control key-to-key spacing versus guide-to-guide spacing.

[clauswilke]

The guide-to-guide spacing thing prompts the question though whether there should a separate theme element to control key-to-key spacing versus guide-to-guide spacing.

Probably. I've always felt the customization options for legends were insufficient and the same values were inappropriately being reused over and over.

[teunbrand]

I'm leaning towards adding key.spacing{.x/.y} arguments in guide_legend() to control the spacing between keys. That way, legend.spacing{.x/.y} can be purely for spacing between guides. The other guides are implemented as single keys, so that setting would be irrelevant for them.

[teunbrand]

I considered implementing the key.spacing in another PR, but I felt it was so closely related, it might best fit in this PR.

So now we can set the spacing between keys using the key.spacing arguments in guide_legend(). No other guides use this spacing, so it wouldn't make too much sense to make this a theme() setting. Default behaviour of the key.spacing matches current behaviour, with the exception that byrow has no influence on the spacing (as it shouldn't).

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2
#> Warning: package 'testthat' was built under R version 4.3.1

ggplot(mpg, aes(displ, hwy)) +
  geom_point(aes(colour = class)) +
  guides(colour = guide_legend(
    ncol = 2, key.spacing.y = 1, key.spacing.x = 2
  ))

^{Created on 2023-11-03 with reprex v2.0.2}

An additional benefit is that legend.spacing is now 'pure' in that it only controls the spacing between legends and not within legends.

This figure might be a better explainer of what this PR does:

[teunbrand]

Now also fixes the bin spacing issue:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot(mpg, aes(displ, hwy)) +
  geom_point(aes(size = cty)) +
  scale_size_continuous(
    breaks = c(10, 20, 30),
    labels = c("A", "B\nB", "C\nC\nC"),
    guide  = "bins"
  ) +
  theme(
    legend.text = element_text(angle = 270, vjust = 1)
  )

^{Created on 2023-11-08 with reprex v2.0.2}

[thomasp85]

Apart from the small comment LGTM

[teunbrand]

Thanks for the review Thomas!