theme docs - strip.text.x.* family

[Polkas]

closes #5054

• The new (>=3.3.2 version) strip.text.x.top, strip.text.x.bottom, strip.text.y.left, strip.text.y.right arguments are documented in the ggplot2::theme now.
• I added the additional example for that new feature.
• There was a surprising small difference in the one testthat snap file, so I had to update it (please check out the svg file diff).

[yutannihilation]

Thanks for catching! Looks good, but could you exclude the update on the snapshot? I think it's unnecessary.

[Polkas]

Please check out that after revert to the old snap svg file the test are failing, for most systems.
I will revert the revert:)

[yutannihilation]

Hmm, so does it mean this change actually affects the behaviour of ggplot2? Sorry, then the things are a bit complex than I expected. We have to figure out what's happening here...

[Polkas]

I will investigate it. Summary for current situation and the source of the test error.

ggplot2/tests/testthat/test-facet-strips.r

Lines 164 to 168 in fc191a0

	test_that("y strip labels are rotated when strips are switched", {
	switched <- p + facet_grid(am ~ cyl, switch = "both")
	expect_doppelganger("switched facet strips", switched)
	})

The difference in svg is the same in two places translate(46.59,403.41) -> translate(40.54,403.41) for the text tag.

[Polkas]

So I fixed another problem as new theme arguments was expected but not taken into account.

ggplot2/R/facet-grid-.r

Line 321 in fc191a0

strips <- render_strips(col_vars, row_vars, params$labeller, theme)

from that place we ended in ggplot2:::build_strip and there we look for theme arguments which not exists in theme before, but of course now they are.

ggplot2/R/labeller.r

Lines 503 to 534 in 63125db

	if (horizontal) {
	grobs_top <- lapply(labels_vec, element_render, theme = theme,
	element = "strip.text.x.top", margin_x = TRUE,
	margin_y = TRUE)
	grobs_top <- assemble_strips(matrix(grobs_top, ncol = ncol, nrow = nrow),
	theme, horizontal, clip = clip)
	grobs_bottom <- lapply(labels_vec, element_render, theme = theme,
	element = "strip.text.x.bottom", margin_x = TRUE,
	margin_y = TRUE)
	grobs_bottom <- assemble_strips(matrix(grobs_bottom, ncol = ncol, nrow = nrow),
	theme, horizontal, clip = clip)
	list(
	top = grobs_top,
	bottom = grobs_bottom
	)
	} else {
	grobs_left <- lapply(labels_vec, element_render, theme = theme,
	element = "strip.text.y.left", margin_x = TRUE,
	margin_y = TRUE)
	grobs_left <- assemble_strips(matrix(grobs_left, ncol = ncol, nrow = nrow),
	theme, horizontal, clip = clip)
	grobs_right <- lapply(unlist(labels[, rev(seq_len(ncol(labels))), drop = FALSE], use.names = FALSE),
	element_render, theme = theme,
	element = "strip.text.y.right", margin_x = TRUE,
	margin_y = TRUE)
	grobs_right <- assemble_strips(matrix(grobs_right, ncol = ncol, nrow = nrow),
	theme, horizontal, clip = clip)
	list(

and later could not be used e.g. here

ggplot2/R/facet-grid-.r

Lines 394 to 440 in fc191a0

	if (switch_x) {
	if (!is.null(strips$x$bottom)) {
	if (inside_x || as.numeric(axis_height_bottom) == 0) {
	panel_table <- gtable_add_rows(panel_table, max_height(strips$x$bottom), -2)
	panel_table <- gtable_add_grob(panel_table, strips$x$bottom, -2, panel_pos_col$l, clip = "on", name = paste0("strip-b-", seq_along(strips$x$bottom)), z = 2)
	} else {
	panel_table <- gtable_add_rows(panel_table, strip_padding, -1)
	panel_table <- gtable_add_rows(panel_table, max_height(strips$x$bottom), -1)
	panel_table <- gtable_add_grob(panel_table, strips$x$bottom, -1, panel_pos_col$l, clip = "on", name = paste0("strip-b-", seq_along(strips$x$bottom)), z = 2)
	}
	}
	} else {
	if (!is.null(strips$x$top)) {
	if (inside_x || as.numeric(axis_height_top) == 0) {
	panel_table <- gtable_add_rows(panel_table, max_height(strips$x$top), 1)
	panel_table <- gtable_add_grob(panel_table, strips$x$top, 2, panel_pos_col$l, clip = "on", name = paste0("strip-t-", seq_along(strips$x$top)), z = 2)
	} else {
	panel_table <- gtable_add_rows(panel_table, strip_padding, 0)
	panel_table <- gtable_add_rows(panel_table, max_height(strips$x$top), 0)
	panel_table <- gtable_add_grob(panel_table, strips$x$top, 1, panel_pos_col$l, clip = "on", name = paste0("strip-t-", seq_along(strips$x$top)), z = 2)
	}
	}
	}
	panel_pos_rows <- panel_rows(panel_table)
	if (switch_y) {
	if (!is.null(strips$y$left)) {
	if (inside_y || as.numeric(axis_width_left) == 0) {
	panel_table <- gtable_add_cols(panel_table, max_width(strips$y$left), 1)
	panel_table <- gtable_add_grob(panel_table, strips$y$left, panel_pos_rows$t, 2, clip = "on", name = paste0("strip-l-", seq_along(strips$y$left)), z = 2)
	} else {
	panel_table <- gtable_add_cols(panel_table, strip_padding, 0)
	panel_table <- gtable_add_cols(panel_table, max_width(strips$y$left), 0)
	panel_table <- gtable_add_grob(panel_table, strips$y$left, panel_pos_rows$t, 1, clip = "on", name = paste0("strip-l-", seq_along(strips$y$left)), z = 2)
	}
	}
	} else {
	if (!is.null(strips$y$right)) {
	if (inside_y || as.numeric(axis_width_right) == 0) {
	panel_table <- gtable_add_cols(panel_table, max_width(strips$y$right), -2)
	panel_table <- gtable_add_grob(panel_table, strips$y$right, panel_pos_rows$t, -2, clip = "on", name = paste0("strip-r-", seq_along(strips$y$right)), z = 2)
	} else {
	panel_table <- gtable_add_cols(panel_table, strip_padding, -1)
	panel_table <- gtable_add_cols(panel_table, max_width(strips$y$right), -1)
	panel_table <- gtable_add_grob(panel_table, strips$y$right, panel_pos_rows$t, -1, clip = "on", name = paste0("strip-r-", seq_along(strips$y$right)), z = 2)
	}
	}
	}

[Polkas]

More precise proof.

debug(ggplot2:::calc_element)
p <- ggplot(mtcars, aes(disp, drat)) + geom_point()
p + facet_grid(am ~ cyl, switch = "both")
# in debug mode
theme[["strip.text.y.left"]]
# It is a list , previously (before PR) it was a NULL

[teunbrand]

The suspicious thing about the visual test, is that the text is rotated by -90 degrees instead of typical 90 (only for theme_test()).
After doing some digging, I found the difference comes from ggplot2:::plot_theme(). Using CRAN ggplot2:

library(ggplot2) # CRAN version

p <- ggplot(mtcars, aes(disp, drat)) + geom_point() + theme_test() + 
  facet_grid(am ~ cyl, switch = "both")
b <- ggplot_build(p)
old <- calc_element("strip.text.y.left", ggplot2:::plot_theme(b$plot))

Likewise with a small edit to mimic this PR:

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

p <- ggplot(mtcars, aes(disp, drat)) + geom_point() + theme_test() + 
  facet_grid(am ~ cyl, switch = "both")
b <- ggplot_build(p)
new <- calc_element("strip.text.y.left", ggplot2:::plot_theme(b$plot))

The difference is in the angles:

waldo::compare(old, new)
#> `old$angle`:  90
#> `new$angle`: -90

^{Created on 2022-11-30 by the reprex package (v2.0.1)}

In particular, these lines give different results, as strip.text.y.left is now no longer considered 'missing', so it doesn't get updated with a reasonable default (angle = 90).

ggplot2/R/theme.r

Lines 444 to 445 in 63125db

	missing <- setdiff(names(default), names(theme))
	theme[missing] <- default[missing]

The simplest 'fix' would be to add strip.text.y.left = element_text(angle = 90) to theme_test(), as all other themes (except for theme_void()) inherit from theme_gray() in some way, which has a sensible strip.text.y.left element.

[yutannihilation]

Wow, thanks you two so much for investigating! The fix sounds good to me. I'll try to understated the problem.

[yutannihilation]

The simplest 'fix' would be to add strip.text.y.left = element_text(angle = 90) to theme_test(), as all other themes (except for theme_void()) inherit from theme_gray() in some way, which has a sensible strip.text.y.left element.

Sorry for the delay. I think this fix is good enough. strip.text.y.left was added to theme_gray() on https://github.com/tidyverse/ggplot2/pull/3683/files, but not to theme_test(). I think this was simply an oversight. I'm not sure if there can be some better structure for testing, but let's not change today.

@Polkas
Could you add strip.text.y.left = element_text(angle = 90) after this line? It should fix the test.

ggplot2/R/theme-defaults.r

Line 621 in 5ebe979

strip.text.y = element_text(angle = -90),

[Polkas]

@yutannihilation, thanks for a clear recommendation. I just applied it, and the _snaps for a few plots have to be updated.

[yutannihilation]

Thanks, but I think the change is not what @teunbrand and I recommended...? The suggestion was to add strip.text.y.left, not to modify strip.text.y. Sorry, I think I don't get your intention here.

[Polkas]

I understand that strip.text.y.left and strip.text.y.right inherit from strip.text.y, this was the source of the confusion. Sorry It was late, I see it now clear we want to be direct here with strip.

Ps. I learned a new thing that git can overwrite chmod of the file (false positive), I had to fix it manually for one file here with git update-index --chmod=+w tests/testthat/_snaps/geom-dotplot/stackratio-1-5.svg

[yutannihilation]

I guess you mistakenly committed tests/testthat/_snaps/theme/?

[Polkas]

I guess you mistakenly committed tests/testthat/_snaps/theme/?

I guess you mistakenly committed tests/testthat/_snaps/theme/?

I created another issue connected with the arm64 #5110. I will remove these files.

[yutannihilation]

Oh, curious. Anyway, this pull request looks perfect now. Thanks!