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

Add strip.spacing theme element #5958

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Jun 27, 2024

This PR aims to fix #5935.

Briefly, a new theme element strip.spacing is used to set the distance from the panel to the strip unconditionally.

Previously, we could only use strip.switch.pad.{grid/wrap} to set this distance, but only if there was an axis in between the panel and the strip. In case the conditions for the strip.switch.pad.{grid/wrap} are met, then strip.spacing is additive to that spacing. The default spacing is rel(0) inheriting from the root theme(spacing) argument, so it doesn't add space by default.

However, it does add a gtable cell even when there is 0-spacing, which helps towards #5628 as the gtable shape no longer depends on the relative placement of axes and strips (the cell is added regardless).

To give a few examples. We weren't able to do this before:

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

p <- ggplot(mpg, aes(displ, hwy)) +
  geom_point()

p + facet_wrap(year ~ drv) +
  theme(strip.spacing = unit(1, "cm"))

Or this:

p + facet_grid(year ~ drv, switch = "both") +
  theme(strip.spacing = unit(1, "cm"))

Created on 2024-06-27 with reprex v2.1.0

@thomasp85
Copy link
Member

I am pretty sure this might have consequences for patchwork. Can I get you to look into this before we move forward?

@teunbrand
Copy link
Collaborator Author

teunbrand commented Aug 20, 2024

Hmm you're right that this doesn't work with patchwork. The PR essentially uses the same gtable cells as the strip.switch.{wrap/grid}.pad setting. I had thought that patchwork was able to deal with this and strip.position = "outside" but it does not (see thomasp85/patchwork#361). Let's press the pause buttom on this PR for now until patchwork has fielded a solution for this.

@teunbrand teunbrand marked this pull request as draft August 20, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strip.placement = "outside" and strip.switch.pad.wrap don't work together perfectly for facet_wrap.
2 participants