From de2f442f5b1db73588f04b23a88af683082a37d9 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 5 Oct 2023 10:45:46 +0200 Subject: [PATCH 01/12] Resolve text margins --- R/guide-legend.R | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/R/guide-legend.R b/R/guide-legend.R index 4667e50388..25ce3cb7b3 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -437,6 +437,17 @@ GuideLegend <- ggproto( "cm", valueOnly = TRUE ) + if (is.null(params$label.theme$margin %||% theme$legend.text$margin) && + !inherits(elements$text, "element_blank")) { + i <- match(params$label.position, .trbl[c(3, 4, 1, 2)]) + elements$text$margin[i] <- elements$text$margin[i] + gap + } + if (is.null(params$title.theme$margin %||% theme$legend.title$margin) && + !inherits(elements$title, "element_blank")) { + i <- match(params$title.position, .trbl[c(3, 4, 1, 2)]) + elements$title$margin[i] <- elements$title$margin[i] + gap + } + # Evaluate backgrounds early if (!is.null(elements$background)) { elements$background <- ggname( From 131d40f309b6526039c7c5e9d187292650737957 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 5 Oct 2023 10:46:18 +0200 Subject: [PATCH 02/12] Remove gaps --- R/guide-legend.R | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/R/guide-legend.R b/R/guide-legend.R index 25ce3cb7b3..0ce703a1b3 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -532,8 +532,8 @@ GuideLegend <- ggproto( hgap <- elements$hgap %||% 0 widths <- switch( params$label.position, - "left" = list(label_widths, hgap, widths, hgap), - "right" = list(widths, hgap, label_widths, hgap), + "left" = list(label_widths, widths, hgap), + "right" = list(widths, label_widths, hgap), list(pmax(label_widths, widths), hgap * (!byrow)) ) widths <- head(vec_interleave(!!!widths), -1) @@ -541,8 +541,8 @@ GuideLegend <- ggproto( vgap <- elements$vgap %||% 0 heights <- switch( params$label.position, - "top" = list(label_heights, vgap, heights, vgap), - "bottom" = list(heights, vgap, label_heights, vgap), + "top" = list(label_heights, heights, vgap), + "bottom" = list(heights, label_heights, vgap), list(pmax(label_heights, heights), vgap * (byrow)) ) heights <- head(vec_interleave(!!!heights), -1) @@ -554,14 +554,14 @@ GuideLegend <- ggproto( # Combine title with rest of the sizes based on its position widths <- switch( params$title.position, - "left" = c(title_width, hgap, widths), - "right" = c(widths, hgap, title_width), + "left" = c(title_width, widths), + "right" = c(widths, title_width), c(widths, max(0, title_width - sum(widths))) ) heights <- switch( params$title.position, - "top" = c(title_height, vgap, heights), - "bottom" = c(heights, vgap, title_height), + "top" = c(title_height, heights), + "bottom" = c(heights, title_height), c(heights, max(0, title_height - sum(heights))) ) @@ -596,20 +596,20 @@ GuideLegend <- ggproto( switch( params$label.position, "top" = { - key_row <- key_row * 2 - label_row <- label_row * 2 - 2 + key_row <- key_row + df$R + label_row <- key_row - 1 }, "bottom" = { - key_row <- key_row * 2 - 2 - label_row <- label_row * 2 + key_row <- key_row + df$R - 1 + label_row <- key_row + 1 }, "left" = { - key_col <- key_col * 2 - label_col <- label_col * 2 - 2 + key_col <- key_col + df$C + label_col <- key_col - 1 }, "right" = { - key_col <- key_col * 2 - 2 - label_col <- label_col * 2 + key_col <- key_col + df$C - 1 + label_col <- key_col + 1 } ) @@ -617,8 +617,8 @@ GuideLegend <- ggproto( switch( params$title.position, "top" = { - key_row <- key_row + 2 - label_row <- label_row + 2 + key_row <- key_row + 1 + label_row <- label_row + 1 title_row <- 2 title_col <- seq_along(sizes$widths) + 1 }, @@ -627,8 +627,8 @@ GuideLegend <- ggproto( title_col <- seq_along(sizes$widths) + 1 }, "left" = { - key_col <- key_col + 2 - label_col <- label_col + 2 + key_col <- key_col + 1 + label_col <- label_col + 1 title_row <- seq_along(sizes$heights) + 1 title_col <- 2 }, From a115f080f1c57d492ba810a803e7d6f529ef7aa1 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 5 Oct 2023 10:46:49 +0200 Subject: [PATCH 03/12] Remove justification fuss in colourbar --- R/guide-colorbar.R | 7 ------- 1 file changed, 7 deletions(-) diff --git a/R/guide-colorbar.R b/R/guide-colorbar.R index 4917679ebf..2f49a34fdd 100644 --- a/R/guide-colorbar.R +++ b/R/guide-colorbar.R @@ -429,17 +429,10 @@ GuideColourbar <- ggproto( return(list(labels = zeroGrob())) } - just <- if (params$direction == "horizontal") { - elements$text$vjust - } else { - elements$text$hjust - } - list(labels = flip_element_grob( elements$text, label = validate_labels(key$.label), x = unit(key$.value, "npc"), - y = rep(just, nrow(key)), margin_x = FALSE, margin_y = TRUE, flip = params$direction == "vertical" From df52fd234fa909bc0e719b9f427d006a992a39d5 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 5 Oct 2023 11:10:00 +0200 Subject: [PATCH 04/12] Tweak tests --- .../rotated-guide-titles-and-labels.svg | 10 ++-- ...p-of-1cm-between-guide-title-and-guide.svg | 58 +++++++++---------- tests/testthat/test-guides.R | 8 +-- 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/tests/testthat/_snaps/guides/rotated-guide-titles-and-labels.svg b/tests/testthat/_snaps/guides/rotated-guide-titles-and-labels.svg index 1a4de9074b..8c8690be22 100644 --- a/tests/testthat/_snaps/guides/rotated-guide-titles-and-labels.svg +++ b/tests/testthat/_snaps/guides/rotated-guide-titles-and-labels.svg @@ -79,11 +79,11 @@ -5.0 -7.5 -10.0 -12.5 -15.0 +5.0 +7.5 +10.0 +12.5 +15.0 rotated guide titles and labels diff --git a/tests/testthat/_snaps/guides/vertical-gap-of-1cm-between-guide-title-and-guide.svg b/tests/testthat/_snaps/guides/vertical-gap-of-1cm-between-guide-title-and-guide.svg index 9abc788f7d..7fe9d6d17d 100644 --- a/tests/testthat/_snaps/guides/vertical-gap-of-1cm-between-guide-title-and-guide.svg +++ b/tests/testthat/_snaps/guides/vertical-gap-of-1cm-between-guide-title-and-guide.svg @@ -55,35 +55,35 @@ 3.0 x y - -y - - - - - - - - - - - -1.0 -1.5 -2.0 -2.5 -3.0 - -factor(x) - - - - - - -1 -2 -3 + +y + + + + + + + + + + + +1.0 +1.5 +2.0 +2.5 +3.0 + +factor(x) + + + + + + +1 +2 +3 vertical gap of 1cm between guide title and guide diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 9531abfa56..9d0fe87bbe 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -601,10 +601,10 @@ test_that("guides title and text are positioned correctly", { scale_fill_continuous(name = "the\ncontinuous\ncolorscale") ) expect_doppelganger("vertical gap of 1cm between guide title and guide", - p + theme(legend.spacing.y = grid::unit(1, "cm")) + p + theme(legend.title = element_text(margin = margin(b = 1, unit = "cm"))) ) expect_doppelganger("horizontal gap of 1cm between guide and guide text", - p + theme(legend.spacing.x = grid::unit(1, "cm")) + p + theme(legend.text = element_text(margin = margin(l = 1, unit = "cm"))) ) # now test label positioning, alignment, etc @@ -619,8 +619,8 @@ test_that("guides title and text are positioned correctly", { expect_doppelganger("guide title and text positioning and alignment via themes", p + theme( - legend.title = element_text(hjust = 0.5, margin = margin(t = 30)), - legend.text = element_text(hjust = 1, margin = margin(l = 5, t = 10, b = 10)) + legend.title = element_text(hjust = 0.5, margin = margin(t = 30, b = 5.5)), + legend.text = element_text(hjust = 1, margin = margin(l = 10.5, t = 10, b = 10)) ) ) From 6fa2bd6d24c8fb4357da9d596947cfc7096e7f3c Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Thu, 5 Oct 2023 11:10:11 +0200 Subject: [PATCH 05/12] Add news bullet --- NEWS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.md b/NEWS.md index eb6f82c140..f7de233b01 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # ggplot2 (development version) +* The spacing between legend keys and their labels, in addition to legends + and their titles, is now controlled by the text's `margin` setting. Not + specifying margins will automatically add appropriate text margins. This + leaves the `legend.spacing` dedicated to controlling the spacing between + different keys (#5455). + * New function `check_device()` for testing the availability of advanced graphics features introduced in R 4.1.0 onwards (@teunbrand, #5332). From 738d06a9be411c13908aa79261f3fbf074d9779d Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 3 Nov 2023 15:17:01 +0100 Subject: [PATCH 06/12] Use `key.spacing` to control between key spacing --- R/guide-legend.R | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/R/guide-legend.R b/R/guide-legend.R index 313d9884fc..a8e11603e7 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -143,6 +143,9 @@ guide_legend <- function( # Key size keywidth = NULL, keyheight = NULL, + key.spacing = NULL, + key.spacing.x = NULL, + key.spacing.y = NULL, # General direction = NULL, @@ -156,12 +159,24 @@ guide_legend <- function( ... ) { # Resolve key sizes - if (!inherits(keywidth, c("NULL", "unit"))) { + if (!(is.null(keywidth) | is.unit(keywidth))) { keywidth <- unit(keywidth, default.unit) } - if (!inherits(keyheight, c("NULL", "unit"))) { + if (!(is.null(keyheight) | is.unit(keyheight))) { keyheight <- unit(keyheight, default.unit) } + + # Resolve spacing + key.spacing.x <- key.spacing.x %||% key.spacing + if (!is.null(key.spacing.x) | is.unit(key.spacing.x)) { + key.spacing.x <- unit(key.spacing.x, default.unit) + } + key.spacing.y <- key.spacing.y %||% key.spacing + if (!is.null(key.spacing.y) | is.unit(key.spacing.y)) { + key.spacing.y <- unit(key.spacing.y, default.unit) + } + + if (!is.null(title.position)) { title.position <- arg_match0(title.position, .trbl) } @@ -187,6 +202,8 @@ guide_legend <- function( # Key size keywidth = keywidth, keyheight = keyheight, + key.spacing.x = key.spacing.x, + key.spacing.y = key.spacing.y, # General direction = direction, @@ -226,9 +243,10 @@ GuideLegend <- ggproto( keywidth = NULL, keyheight = NULL, + key.spacing.x = NULL, + key.spacing.y = NULL, # General - direction = NULL, override.aes = list(), nrow = NULL, ncol = NULL, @@ -436,8 +454,16 @@ GuideLegend <- ggproto( elements$text$size %||% 11 gap <- unit(gap * 0.5, "pt") # Should maybe be elements$spacing.{x/y} instead of the theme's spacing? - elements$hgap <- width_cm( theme$legend.spacing.x %||% gap) - elements$vgap <- height_cm(theme$legend.spacing.y %||% gap) + + if (params$direction == "vertical") { + # For backward compatibility, vertical default is no spacing + vgap <- params$key.spacing.y %||% unit(0, "pt") + } else { + vgap <- params$key.spacing.y %||% gap + } + + elements$hgap <- width_cm( params$key.spacing.x %||% gap) + elements$vgap <- height_cm(vgap) elements$padding <- convertUnit( elements$margin %||% margin(), "cm", valueOnly = TRUE @@ -540,7 +566,7 @@ GuideLegend <- ggproto( params$label.position, "left" = list(label_widths, widths, hgap), "right" = list(widths, label_widths, hgap), - list(pmax(label_widths, widths), hgap * (!byrow)) + list(pmax(label_widths, widths), hgap) ) widths <- head(vec_interleave(!!!widths), -1) @@ -549,7 +575,7 @@ GuideLegend <- ggproto( params$label.position, "top" = list(label_heights, heights, vgap), "bottom" = list(heights, label_heights, vgap), - list(pmax(label_heights, heights), vgap * (byrow)) + list(pmax(label_heights, heights), vgap) ) heights <- head(vec_interleave(!!!heights), -1) From e0ea44cf72d478928ea1ed43a08627d81c3cd0cd Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 3 Nov 2023 15:59:36 +0100 Subject: [PATCH 07/12] remove vestigial `legend.spacing` from guides --- R/guide-colorbar.R | 3 --- R/guide-legend.R | 3 --- 2 files changed, 6 deletions(-) diff --git a/R/guide-colorbar.R b/R/guide-colorbar.R index e0765fc3a6..265d6bec61 100644 --- a/R/guide-colorbar.R +++ b/R/guide-colorbar.R @@ -307,9 +307,6 @@ GuideColourbar <- ggproto( ticks_length = unit(0.2, "npc"), background = "legend.background", margin = "legend.margin", - spacing = "legend.spacing", - spacing.x = "legend.spacing.x", - spacing.y = "legend.spacing.y", key = "legend.key", key.height = "legend.key.height", key.width = "legend.key.width", diff --git a/R/guide-legend.R b/R/guide-legend.R index a8e11603e7..dad06546c9 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -267,9 +267,6 @@ GuideLegend <- ggproto( elements = list( background = "legend.background", margin = "legend.margin", - spacing = "legend.spacing", - spacing.x = "legend.spacing.x", - spacing.y = "legend.spacing.y", key = "legend.key", key.height = "legend.key.height", key.width = "legend.key.width", From 9ff8ff860aded008549f93502dd9f21f04a1998f Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 3 Nov 2023 16:01:42 +0100 Subject: [PATCH 08/12] add test for `key.spacing` --- .../guides/legend-with-widely-spaced-keys.svg | 109 ++++++++++++++++++ tests/testthat/test-guides.R | 10 ++ 2 files changed, 119 insertions(+) create mode 100644 tests/testthat/_snaps/guides/legend-with-widely-spaced-keys.svg diff --git a/tests/testthat/_snaps/guides/legend-with-widely-spaced-keys.svg b/tests/testthat/_snaps/guides/legend-with-widely-spaced-keys.svg new file mode 100644 index 0000000000..ee015eb369 --- /dev/null +++ b/tests/testthat/_snaps/guides/legend-with-widely-spaced-keys.svg @@ -0,0 +1,109 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +10 +15 +20 +25 +30 +35 + + + + + + + + + + +100 +200 +300 +400 +disp +mpg + +factor(carb) + + + + + + + + + + + + +1 +2 +3 +4 +6 +8 +legend with widely spaced keys + + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index f1898008d5..35951ab9df 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -338,6 +338,16 @@ test_that("guide_colourbar warns about discrete scales", { }) +test_that("guide_legend uses key.spacing correctly", { + p <- ggplot(mtcars, aes(disp, mpg, colour = factor(carb))) + + geom_point() + + guides(colour = guide_legend( + ncol = 2, key.spacing.y = 1, key.spacing.x = 2 + )) + + expect_doppelganger("legend with widely spaced keys", p) +}) + # Visual tests ------------------------------------------------------------ test_that("axis guides are drawn correctly", { From 00df5c7fa6e7c35194fcb646ca6af7af637da988 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 3 Nov 2023 16:39:08 +0100 Subject: [PATCH 09/12] document parameters --- R/guide-legend.R | 4 ++++ man/guide_legend.Rd | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/R/guide-legend.R b/R/guide-legend.R index dad06546c9..c4bf2ab195 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -42,6 +42,10 @@ #' @param keyheight A numeric or a [grid::unit()] object specifying #' the height of the legend key. Default value is `legend.key.height` or #' `legend.key.size` in [theme()]. +#' @param key.spacing,key.spacing.x,key.spacing.y A numeric or [grid::unit()] +#' object specifying the distance between key-label pairs in the horizontal +#' direction (`key.spacing.x`), vertical direction (`key.spacing.y`) or both +#' (`key.spacing`). #' @param direction A character string indicating the direction of the guide. #' One of "horizontal" or "vertical." #' @param default.unit A character string indicating [grid::unit()] diff --git a/man/guide_legend.Rd b/man/guide_legend.Rd index 21dcbe7833..224de5587a 100644 --- a/man/guide_legend.Rd +++ b/man/guide_legend.Rd @@ -17,6 +17,9 @@ guide_legend( label.vjust = NULL, keywidth = NULL, keyheight = NULL, + key.spacing = NULL, + key.spacing.x = NULL, + key.spacing.y = NULL, direction = NULL, default.unit = "line", override.aes = list(), @@ -74,6 +77,11 @@ the width of the legend key. Default value is \code{legend.key.width} or the height of the legend key. Default value is \code{legend.key.height} or \code{legend.key.size} in \code{\link[=theme]{theme()}}.} +\item{key.spacing, key.spacing.x, key.spacing.y}{A numeric or \code{\link[grid:unit]{grid::unit()}} +object specifying the distance between key-label pairs in the horizontal +direction (\code{key.spacing.x}), vertical direction (\code{key.spacing.y}) or both +(\code{key.spacing}).} + \item{direction}{A character string indicating the direction of the guide. One of "horizontal" or "vertical."} From 7766153146a630fe57a812d511151389c631b64b Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:48:24 +0100 Subject: [PATCH 10/12] Fix #5456 for bins --- R/guide-bins.R | 8 -------- 1 file changed, 8 deletions(-) diff --git a/R/guide-bins.R b/R/guide-bins.R index f20adee759..1f2228c8c3 100644 --- a/R/guide-bins.R +++ b/R/guide-bins.R @@ -341,13 +341,6 @@ GuideBins <- ggproto( } key$.label[c(1, n_labels)[!params$show.limits]] <- "" - just <- switch( - params$direction, - horizontal = elements$text$vjust, - vertical = elements$text$hjust, - 0.5 - ) - if (params$direction == "vertical") { key$.value <- 1 - key$.value } @@ -356,7 +349,6 @@ GuideBins <- ggproto( elements$text, label = key$.label, x = unit(key$.value, "npc"), - y = rep(just, nrow(key)), margin_x = FALSE, margin_y = TRUE, flip = params$direction == "vertical" From 6b8955f6bd3809edc74ec8241a2d1734eafdb042 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Wed, 15 Nov 2023 09:22:19 +0100 Subject: [PATCH 11/12] Use `||` instead of `|` --- R/guide-legend.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/guide-legend.R b/R/guide-legend.R index c4bf2ab195..5b76ded9a5 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -163,20 +163,20 @@ guide_legend <- function( ... ) { # Resolve key sizes - if (!(is.null(keywidth) | is.unit(keywidth))) { + if (!(is.null(keywidth) || is.unit(keywidth))) { keywidth <- unit(keywidth, default.unit) } - if (!(is.null(keyheight) | is.unit(keyheight))) { + if (!(is.null(keyheight) || is.unit(keyheight))) { keyheight <- unit(keyheight, default.unit) } # Resolve spacing key.spacing.x <- key.spacing.x %||% key.spacing - if (!is.null(key.spacing.x) | is.unit(key.spacing.x)) { + if (!is.null(key.spacing.x) || is.unit(key.spacing.x)) { key.spacing.x <- unit(key.spacing.x, default.unit) } key.spacing.y <- key.spacing.y %||% key.spacing - if (!is.null(key.spacing.y) | is.unit(key.spacing.y)) { + if (!is.null(key.spacing.y) || is.unit(key.spacing.y)) { key.spacing.y <- unit(key.spacing.y, default.unit) } From e314124c4ae137a62103cdf85bbbc28b8d1c4c79 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Mon, 20 Nov 2023 14:40:43 +0100 Subject: [PATCH 12/12] add comment --- R/guide-legend.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/guide-legend.R b/R/guide-legend.R index 5b76ded9a5..fe383f81d0 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -470,6 +470,9 @@ GuideLegend <- ggproto( "cm", valueOnly = TRUE ) + # When no explicit margin has been set, either in this guide or in the + # theme, we set a default text margin to leave a small gap in between + # the label and the key. if (is.null(params$label.theme$margin %||% theme$legend.text$margin) && !inherits(elements$text, "element_blank")) { i <- match(params$label.position, .trbl[c(3, 4, 1, 2)])