From bc3e36a4f256ab468a9a482e4288268864bf2d45 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 15 Sep 2023 11:51:34 +0200 Subject: [PATCH 1/8] Divorce building and drawing/assembly --- R/guides-.R | 34 ++++++++++++++++++++++++++++------ R/plot-build.R | 5 ++--- tests/testthat/test-guides.R | 2 +- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/R/guides-.R b/R/guides-.R index 163559b99c..d3ba02b252 100644 --- a/R/guides-.R +++ b/R/guides-.R @@ -282,7 +282,7 @@ Guides <- ggproto( position, theme, labels) { position <- legend_position(position) - no_guides <- zeroGrob() + no_guides <- guides_list() if (position == "none") { return(no_guides) } @@ -318,10 +318,7 @@ Guides <- ggproto( if (length(guides$guides) == 0) { return(no_guides) } - - # Draw and assemble - grobs <- guides$draw(theme) - guides$assemble(grobs, theme) + guides }, # Setup routine for resolving and validating guides based on paired scales. @@ -494,7 +491,32 @@ Guides <- ggproto( }, # Combining multiple guides in a guide box - assemble = function(grobs, theme) { + assemble = function(self, theme, position) { + + if (length(self$guides) < 1) { + return(zeroGrob()) + } + + position <- legend_position(position) + theme$legend.key.width <- theme$legend.key.width %||% theme$legend.key.size + theme$legend.key.height <- theme$legend.key.height %||% theme$legend.key.size + + + default_direction <- if (position == "inside") "vertical" else position + theme$legend.box <- theme$legend.box %||% default_direction + theme$legend.direction <- theme$legend.direction %||% default_direction + theme$legend.box.just <- theme$legend.box.just %||% switch( + position, + inside = c("center", "center"), + vertical = c("left", "top"), + horizontal = c("center", "top") + ) + + grobs <- self$draw(theme) + if (length(grobs) < 1) { + return(zeroGrob()) + } + # Set spacing theme$legend.spacing <- theme$legend.spacing %||% unit(0.5, "lines") theme$legend.spacing.y <- theme$legend.spacing.y %||% theme$legend.spacing diff --git a/R/plot-build.R b/R/plot-build.R index 2c1695e350..7d5f507c16 100644 --- a/R/plot-build.R +++ b/R/plot-build.R @@ -177,9 +177,8 @@ ggplot_gtable.ggplot_built <- function(data) { position <- "manual" } - legend_box <- plot$guides$build( - plot$scales, plot$layers, plot$mapping, position, theme, plot$labels - ) + guides <- plot$guides$build(plot$scales, plot$layers, plot$mapping, position, theme, plot$labels) + legend_box <- guides$assemble(theme, position) if (is.zero(legend_box)) { position <- "none" diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index ac704fdf79..5ae8b5375d 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -158,7 +158,7 @@ test_that("guide_none() can be used in non-position scales", { plot$labels ) - expect_identical(guides, zeroGrob()) + expect_length(guides$guides, 0) }) test_that("Using non-position guides for position scales results in an informative error", { From 245d48b0d67d6a643380dabcd06af4cb123dcebe Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 15 Sep 2023 13:25:51 +0200 Subject: [PATCH 2/8] Fix test guide orders --- tests/testthat/test-draw-key.R | 12 ++++++++---- tests/testthat/test-geom-dotplot.R | 3 ++- tests/testthat/test-guides.R | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-draw-key.R b/tests/testthat/test-draw-key.R index aeba592a6c..340ffb4c6d 100644 --- a/tests/testthat/test-draw-key.R +++ b/tests/testthat/test-draw-key.R @@ -7,14 +7,16 @@ test_that("alternative key glyphs work", { expect_doppelganger("time series and polygon key glyphs", ggplot(df, aes(x, y)) + geom_line(aes(color = "line"), key_glyph = "timeseries") + - geom_point(aes(fill = z), pch = 21, size = 3, key_glyph = "polygon") + geom_point(aes(fill = z), pch = 21, size = 3, key_glyph = "polygon") + + guides(fill = guide_legend(order = 1)) ) # specify key glyph by function expect_doppelganger("rectangle and dotplot key glyphs", ggplot(df, aes(x, y)) + geom_line(aes(color = "line"), key_glyph = draw_key_rect) + - geom_point(aes(fill = z), pch = 21, size = 3, stroke = 2, key_glyph = draw_key_dotplot) + geom_point(aes(fill = z), pch = 21, size = 3, stroke = 2, key_glyph = draw_key_dotplot) + + guides(fill = guide_legend(order = 1)) ) }) @@ -43,11 +45,13 @@ test_that("horizontal key glyphs work", { expect_doppelganger("horizontal boxplot and crossbar", p + geom_boxplot(aes(y = group1, color = group1), stat = "identity") + - geom_crossbar(aes(y = group2, fill = group2)) + geom_crossbar(aes(y = group2, fill = group2)) + + guides(color = guide_legend(order = 1)) ) expect_doppelganger("horizontal linerange and pointrange", p + geom_linerange(aes(y = group1, color = group1)) + - geom_pointrange(aes(y = group2, shape = group2)) + geom_pointrange(aes(y = group2, shape = group2)) + + guides(color = guide_legend(order = 1)) ) }) diff --git a/tests/testthat/test-geom-dotplot.R b/tests/testthat/test-geom-dotplot.R index eedd4ba4e3..a095158937 100644 --- a/tests/testthat/test-geom-dotplot.R +++ b/tests/testthat/test-geom-dotplot.R @@ -196,7 +196,8 @@ test_that("geom_dotplot draws correctly", { ) ) + geom_dotplot(binwidth = .4, fill = "red", col = "blue") + - continuous_scale("stroke", palette = function(x) scales::rescale(x, to = c(1, 6))) + continuous_scale("stroke", palette = function(x) scales::rescale(x, to = c(1, 6))) + + guides(linetype = guide_legend(order = 1)) ) # Stacking groups diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 5ae8b5375d..fed331d6d1 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -559,7 +559,7 @@ test_that("guides are positioned correctly", { dat <- data_frame(x = LETTERS[1:3], y = 1) p2 <- ggplot(dat, aes(x, y, fill = x, colour = 1:3)) + geom_bar(stat = "identity") + - guides(color = "colorbar") + + guides(color = guide_colourbar(order = 1)) + theme_test() + theme(legend.background = element_rect(colour = "black")) From c3a178a20b6c7617fefe64d073496398888df78b Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 15 Sep 2023 13:30:36 +0200 Subject: [PATCH 3/8] Disconnect theme from build step --- R/coord-cartesian-.R | 2 +- R/guide-.R | 5 ++++- R/guide-axis.R | 2 +- R/guide-bins.R | 32 +++++++++++++++------------ R/guide-colorbar.R | 36 +++++++++++++++--------------- R/guide-legend.R | 13 ++++++----- R/guide-none.R | 2 +- R/guide-old.R | 2 +- R/guides-.R | 43 +++++++++++------------------------- R/plot-build.R | 2 +- tests/testthat/test-guides.R | 15 +++++-------- 11 files changed, 71 insertions(+), 83 deletions(-) diff --git a/R/coord-cartesian-.R b/R/coord-cartesian-.R index 8451873b84..1bf1b292cb 100644 --- a/R/coord-cartesian-.R +++ b/R/coord-cartesian-.R @@ -151,5 +151,5 @@ panel_guides_grob <- function(guides, position, theme) { return(zeroGrob()) } pair <- guides$get_position(position) - pair$guide$draw(theme, pair$params) + pair$guide$draw(theme, params = pair$params) } diff --git a/R/guide-.R b/R/guide-.R index ae774d30c9..28b6804cfa 100644 --- a/R/guide-.R +++ b/R/guide-.R @@ -280,11 +280,14 @@ Guide <- ggproto( # Main drawing function that organises more specialised aspects of guide # drawing. - draw = function(self, theme, params = self$params) { + draw = function(self, theme, position = NULL, direction = NULL, + params = self$params) { key <- params$key # Setup parameters and theme + params$position <- params$position %||% position + params$direction <- params$direction %||% direction params <- self$setup_params(params) elems <- self$setup_elements(params, self$elements, theme) elems <- self$override_elements(params, elems, theme) diff --git a/R/guide-axis.R b/R/guide-axis.R index eac32b2b98..ef6766273a 100644 --- a/R/guide-axis.R +++ b/R/guide-axis.R @@ -434,7 +434,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, !!aes := c(0, 1), !!opp := opp_value ) - guide$draw(theme, params) + guide$draw(theme, params = params) } draw_axis_labels <- function(break_positions, break_labels, label_element, is_vertical, diff --git a/R/guide-bins.R b/R/guide-bins.R index 63c75bd0bd..f20adee759 100644 --- a/R/guide-bins.R +++ b/R/guide-bins.R @@ -298,15 +298,15 @@ GuideBins <- ggproto( params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) + params$key <- key + params + }, + + setup_params = function(params) { params$direction <- arg_match0( - params$direction %||% direction, + params$direction, c("horizontal", "vertical"), arg_nm = "direction" ) - if (params$direction == "vertical") { - key$.value <- 1 - key$.value - } - - params$key <- key valid_label_pos <- switch( params$direction, "horizontal" = c("bottom", "top"), @@ -320,10 +320,6 @@ GuideBins <- ggproto( "not {.val {params$label.position}}." )) } - params - }, - - setup_params = function(params) { params <- GuideLegend$setup_params(params) params$byrow <- FALSE params$rejust_labels <- FALSE @@ -345,10 +341,15 @@ GuideBins <- ggproto( } key$.label[c(1, n_labels)[!params$show.limits]] <- "" - just <- if (params$direction == "horizontal") { - elements$text$vjust - } else { - elements$text$hjust + just <- switch( + params$direction, + horizontal = elements$text$vjust, + vertical = elements$text$hjust, + 0.5 + ) + + if (params$direction == "vertical") { + key$.value <- 1 - key$.value } list(labels = flip_element_grob( @@ -363,6 +364,9 @@ GuideBins <- ggproto( }, build_ticks = function(key, elements, params, position = params$position) { + if (params$direction == "vertical") { + key$.value <- 1 - key$.value + } key$.value[c(1, nrow(key))[!params$show.limits]] <- NA Guide$build_ticks(key$.value, elements, params, params$label.position) }, diff --git a/R/guide-colorbar.R b/R/guide-colorbar.R index 4917679ebf..c56e7de3f7 100644 --- a/R/guide-colorbar.R +++ b/R/guide-colorbar.R @@ -344,27 +344,10 @@ GuideColourbar <- ggproto( }, extract_params = function(scale, params, - title = waiver(), direction = "vertical", ...) { + title = waiver(), ...) { params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) - params$direction <- arg_match0( - params$direction %||% direction, - c("horizontal", "vertical"), arg_nm = "direction" - ) - valid_label_pos <- switch( - params$direction, - "horizontal" = c("bottom", "top"), - "vertical" = c("right", "left") - ) - params$label.position <- params$label.position %||% valid_label_pos[1] - if (!params$label.position %in% valid_label_pos) { - cli::cli_abort(paste0( - "When {.arg direction} is {.val {params$direction}}, ", - "{.arg label.position} must be one of {.or {.val {valid_label_pos}}}, ", - "not {.val {params$label.position}}." - )) - } limits <- c(params$decor$value[1], params$decor$value[nrow(params$decor)]) params$key$.value <- rescale( @@ -402,6 +385,23 @@ GuideColourbar <- ggproto( }, setup_params = function(params) { + params$direction <- arg_match0( + params$direction, + c("horizontal", "vertical"), arg_nm = "direction" + ) + valid_label_pos <- switch( + params$direction, + "horizontal" = c("bottom", "top"), + "vertical" = c("right", "left") + ) + params$label.position <- params$label.position %||% valid_label_pos[1] + if (!params$label.position %in% valid_label_pos) { + cli::cli_abort(paste0( + "When {.arg direction} is {.val {params$direction}}, ", + "{.arg label.position} must be one of {.or {.val {valid_label_pos}}}, ", + "not {.val {params$label.position}}." + )) + } params$title.position <- arg_match0( params$title.position %||% switch(params$direction, vertical = "top", horizontal = "left"), diff --git a/R/guide-legend.R b/R/guide-legend.R index 4667e50388..917a0a507f 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -244,7 +244,7 @@ GuideLegend <- ggproto( available_aes = "any", - hashables = exprs(title, key$.label, direction, name), + hashables = exprs(title, key$.label, name), elements = list( background = "legend.background", @@ -260,14 +260,10 @@ GuideLegend <- ggproto( ), extract_params = function(scale, params, - title = waiver(), direction = NULL, ...) { + title = waiver(), ...) { params$title <- scale$make_title( params$title %|W|% scale$name %|W|% title ) - params$direction <- arg_match0( - params$direction %||% direction, - c("horizontal", "vertical"), arg_nm = "direction" - ) if (isTRUE(params$reverse %||% FALSE)) { params$key <- params$key[nrow(params$key):1, , drop = FALSE] } @@ -346,6 +342,11 @@ GuideLegend <- ggproto( }, setup_params = function(params) { + params$direction <- arg_match0( + params$direction %||% direction, + c("horizontal", "vertical"), arg_nm = "direction" + ) + if ("title.position" %in% names(params)) { params$title.position <- arg_match0( params$title.position %||% diff --git a/R/guide-none.R b/R/guide-none.R index ae26a8a1e9..5c0b2d35e2 100644 --- a/R/guide-none.R +++ b/R/guide-none.R @@ -35,7 +35,7 @@ GuideNone <- ggproto( }, # Draw nothing - draw = function(self, params, theme) { + draw = function(self, ...) { zeroGrob() } ) diff --git a/R/guide-old.R b/R/guide-old.R index 2320b0bbf2..cd2dce49f1 100644 --- a/R/guide-old.R +++ b/R/guide-old.R @@ -90,7 +90,7 @@ GuideOld <- ggproto( train = function(self, params, scale, aesthetic = NULL, title = waiver(), direction = NULL) { params$title <- scale$make_title(params$title %|W|% scale$name %|W|% title) - params$direction <- params$direction %||% direction + params$direction <- params$direction %||% direction %||% "vertical" params <- guide_train(params, scale, aesthetic) params }, diff --git a/R/guides-.R b/R/guides-.R index d3ba02b252..5526d2eb42 100644 --- a/R/guides-.R +++ b/R/guides-.R @@ -278,28 +278,10 @@ Guides <- ggproto( # 5. Guides$assemble() # arrange all guide grobs - build = function(self, scales, layers, default_mapping, - position, theme, labels) { + build = function(self, scales, layers, labels) { - position <- legend_position(position) + # Empty guides list no_guides <- guides_list() - if (position == "none") { - return(no_guides) - } - - theme$legend.key.width <- theme$legend.key.width %||% theme$legend.key.size - theme$legend.key.height <- theme$legend.key.height %||% theme$legend.key.size - - - default_direction <- if (position == "inside") "vertical" else position - theme$legend.box <- theme$legend.box %||% default_direction - theme$legend.direction <- theme$legend.direction %||% default_direction - theme$legend.box.just <- theme$legend.box.just %||% switch( - position, - inside = c("center", "center"), - vertical = c("left", "top"), - horizontal = c("center", "top") - ) # Setup and train on scales scales <- scales$non_position_scales()$scales @@ -307,7 +289,7 @@ Guides <- ggproto( return(no_guides) } guides <- self$setup(scales) - guides$train(scales, theme$legend.direction, labels) + guides$train(scales, labels) if (length(guides$guides) == 0) { return(no_guides) } @@ -411,14 +393,13 @@ Guides <- ggproto( # Loop over every guide-scale combination to perform training # A strong assumption here is that `scales` is parallel to the guides - train = function(self, scales, direction, labels) { + train = function(self, scales, labels) { params <- Map( function(guide, param, scale, aes) { guide$train( param, scale, aes, - title = labels[[aes]], - direction = direction + title = labels[[aes]] ) }, guide = self$guides, @@ -482,9 +463,9 @@ Guides <- ggproto( }, # Loop over every guide, let them draw their grobs - draw = function(self, theme) { + draw = function(self, theme, position, direction) { Map( - function(guide, params) guide$draw(theme, params), + function(guide, params) guide$draw(theme, position, direction, params), guide = self$guides, params = self$params ) @@ -498,11 +479,13 @@ Guides <- ggproto( } position <- legend_position(position) + if (position == "none") { + return(zeroGrob()) + } + default_direction <- if (position == "inside") "vertical" else position + theme$legend.key.width <- theme$legend.key.width %||% theme$legend.key.size theme$legend.key.height <- theme$legend.key.height %||% theme$legend.key.size - - - default_direction <- if (position == "inside") "vertical" else position theme$legend.box <- theme$legend.box %||% default_direction theme$legend.direction <- theme$legend.direction %||% default_direction theme$legend.box.just <- theme$legend.box.just %||% switch( @@ -512,7 +495,7 @@ Guides <- ggproto( horizontal = c("center", "top") ) - grobs <- self$draw(theme) + grobs <- self$draw(theme, position, default_direction) if (length(grobs) < 1) { return(zeroGrob()) } diff --git a/R/plot-build.R b/R/plot-build.R index 7d5f507c16..c7ea877ec7 100644 --- a/R/plot-build.R +++ b/R/plot-build.R @@ -177,7 +177,7 @@ ggplot_gtable.ggplot_built <- function(data) { position <- "manual" } - guides <- plot$guides$build(plot$scales, plot$layers, plot$mapping, position, theme, plot$labels) + guides <- plot$guides$build(plot$scales, plot$layers, plot$labels) legend_box <- guides$assemble(theme, position) if (is.zero(legend_box)) { diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index fed331d6d1..97a82d26e6 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -152,9 +152,6 @@ test_that("guide_none() can be used in non-position scales", { guides <- guides$build( plot$scales, plot$layers, - plot$mapping, - "right", - theme_gray(), plot$labels ) @@ -181,7 +178,7 @@ test_that("guide merging for guide_legend() works as expected", { guides <- guides_list(NULL) guides <- guides$setup(scales$scales) - guides$train(scales$scales, "vertical", labs()) + guides$train(scales$scales, labs()) guides$merge() guides$params } @@ -282,11 +279,11 @@ test_that("legend reverse argument reverses the key", { guides <- guides$setup(list(scale)) guides$params[[1]]$reverse <- FALSE - guides$train(list(scale), "horizontal", labels = labs()) + guides$train(list(scale), labels = labs()) fwd <- guides$get_params(1)$key guides$params[[1]]$reverse <- TRUE - guides$train(list(scale), "horizontal", labels = labs()) + guides$train(list(scale), labels = labs()) rev <- guides$get_params(1)$key expect_equal(fwd$colour, rev(rev$colour)) @@ -301,10 +298,10 @@ test_that("guide_coloursteps and guide_bins return ordered breaks", { key <- g$train(scale = scale, aesthetic = "colour")$key expect_true(all(diff(key$.value) > 0)) - # Bins guide is decreasing order + # Bins guide is increasing order g <- guide_bins() - key <- g$train(scale = scale, aesthetics = "colour", direction = "vertical")$key - expect_true(all(diff(key$.value) < 0)) + key <- g$train(scale = scale, aesthetics = "colour")$key + expect_true(all(diff(key$.value) > 0)) }) From eef083d45d2e4a3ef3994cd95d4983e541588458 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 13 Oct 2023 11:38:45 +0200 Subject: [PATCH 4/8] Move guide building to `ggplot_build()` --- R/plot-build.R | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/R/plot-build.R b/R/plot-build.R index c7ea877ec7..bc6a9a8706 100644 --- a/R/plot-build.R +++ b/R/plot-build.R @@ -84,11 +84,18 @@ ggplot_build.ggplot <- function(plot) { layout$setup_panel_params() data <- layout$map_position(data) - # Train and map non-position scales + # Hand off position guides to layout + layout$setup_panel_guides(plot$guides, plot$layers) + + # Train and map non-position scales and guides npscales <- scales$non_position_scales() if (npscales$n() > 0) { lapply(data, npscales$train_df) + plot$guides <- plot$guides$build(npscales, plot$layers, plot$labels) data <- lapply(data, npscales$map_df) + } else { + # Assign empty guides if there are no non-position scales + plot$guides <- guides_list() } # Fill in defaults etc. @@ -168,7 +175,6 @@ ggplot_gtable.ggplot_built <- function(data) { geom_grobs <- by_layer(function(l, d) l$draw_geom(d, layout), plot$layers, data, "converting geom to grob") - layout$setup_panel_guides(plot$guides, plot$layers) plot_table <- layout$render(geom_grobs, data, theme, plot$labels) # Legends @@ -177,8 +183,7 @@ ggplot_gtable.ggplot_built <- function(data) { position <- "manual" } - guides <- plot$guides$build(plot$scales, plot$layers, plot$labels) - legend_box <- guides$assemble(theme, position) + legend_box <- plot$guides$assemble(theme, position) if (is.zero(legend_box)) { position <- "none" From 72ef96bf90c3aefd16d796b9a928bb378dccf531 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 13 Oct 2023 11:39:07 +0200 Subject: [PATCH 5/8] Some error messages are now thrown earlier --- tests/testthat/test-guides.R | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 30b1d0067a..1aa13db169 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -107,8 +107,7 @@ test_that("a warning is generated when guides are drawn at a location that doesn plot <- ggplot(mpg, aes(class, hwy)) + geom_point() + scale_y_continuous(guide = guide_axis(position = "top")) - built <- expect_silent(ggplot_build(plot)) - expect_warning(ggplot_gtable(built), "Position guide is perpendicular") + expect_warning(ggplot_build(plot), "Position guide is perpendicular") }) test_that("a warning is not generated when a guide is specified with duplicate breaks", { @@ -162,9 +161,7 @@ test_that("Using non-position guides for position scales results in an informati p <- ggplot(mpg, aes(cty, hwy)) + geom_point() + scale_x_continuous(guide = guide_legend()) - - built <- ggplot_build(p) - expect_snapshot_warning(ggplot_gtable(built)) + expect_snapshot_warning(ggplot_build(p)) }) test_that("guide merging for guide_legend() works as expected", { @@ -813,8 +810,7 @@ test_that("a warning is generated when guides( = FALSE) is specified", { # warn on scale_*(guide = FALSE) p <- ggplot(df, aes(x, y, colour = x)) + scale_colour_continuous(guide = FALSE) - built <- expect_silent(ggplot_build(p)) - expect_snapshot_warning(ggplot_gtable(built)) + expect_snapshot_warning(ggplot_build(p)) }) test_that("guides() warns if unnamed guides are provided", { From 127d5da7f2d7054b25d2c1272a3f78401a852b00 Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Fri, 13 Oct 2023 12:08:53 +0200 Subject: [PATCH 6/8] Adapt `guide_old()` --- R/guide-old.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/guide-old.R b/R/guide-old.R index cd2dce49f1..88b50f9f37 100644 --- a/R/guide-old.R +++ b/R/guide-old.R @@ -107,9 +107,10 @@ GuideOld <- ggproto( guide_geom(params, layers, default_mapping = NULL) }, - draw = function(self, theme, params) { + draw = function(self, theme, position = NULL, direction = NULL, params) { + params$direction <- params$direction %||% direction %||% "placeholder" params$title.position <- params$title.position %||% switch( - params$direction %||% "placeholder", + params$direction, vertical = "top", horizontal = "left", NULL ) From cc9b75f4075db176e6071ac6b80d17a82bf9be5d Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:27:01 +0200 Subject: [PATCH 7/8] Expose `data` to `process_layers()` --- R/guide-.R | 15 +++++++++++---- R/guide-colorbar.R | 20 ++------------------ R/guide-legend.R | 25 +++++++++++++++---------- R/guide-old.R | 2 +- R/guides-.R | 8 ++++---- R/plot-build.R | 2 +- 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/R/guide-.R b/R/guide-.R index 28b6804cfa..f3ee472cdd 100644 --- a/R/guide-.R +++ b/R/guide-.R @@ -117,9 +117,12 @@ new_guide <- function(..., available_aes = "any", super) { #' `params$hash`. This ensures that e.g. `guide_legend()` can display both #' `shape` and `colour` in the same guide. #' -#' - `get_layer_key()` Extract information from layers. This can be used to -#' check that the guide's aesthetic is actually in use, or to gather -#' information about how legend keys should be displayed. +#' - `process_layers()` Extract information from layers. This acts mostly +#' as a filter for which layers to include and these are then (typically) +#' forwarded to `get_layer_key()`. +#' +#' - `get_layer_key()` This can be used to gather information about how legend +#' keys should be displayed. #' #' - `setup_params()` Set up parameters at the beginning of drawing stages. #' It can be used to overrule user-supplied parameters or perform checks on @@ -253,7 +256,11 @@ Guide <- ggproto( # Function for extracting information from the layers. # Mostly applies to `guide_legend()` and `guide_binned()` - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { + self$get_layer_key(params, layers, data) + }, + + get_layer_key = function(params, layers, data = NULL) { return(params) }, diff --git a/R/guide-colorbar.R b/R/guide-colorbar.R index c56e7de3f7..7e71eaba0c 100644 --- a/R/guide-colorbar.R +++ b/R/guide-colorbar.R @@ -364,24 +364,8 @@ GuideColourbar <- ggproto( return(list(guide = self, params = params)) }, - get_layer_key = function(params, layers) { - - guide_layers <- lapply(layers, function(layer) { - - matched_aes <- matched_aes(layer, params) - - # Check if this layer should be included - if (include_layer_in_guide(layer, matched_aes)) { - layer - } else { - NULL - } - }) - - if (length(compact(guide_layers)) == 0) { - return(NULL) - } - return(params) + get_layer_key = function(params, layers, data = NULL) { + params }, setup_params = function(params) { diff --git a/R/guide-legend.R b/R/guide-legend.R index c20938d7ae..341bee47c8 100644 --- a/R/guide-legend.R +++ b/R/guide-legend.R @@ -287,17 +287,26 @@ GuideLegend <- ggproto( }, # Arrange common data for vertical and horizontal legends - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { + + include <- vapply(layers, function(layer) { + aes <- matched_aes(layer, params) + include_layer_in_guide(layer, aes) + }, logical(1)) + + if (!any(include)) { + return(NULL) + } + + self$get_layer_key(params, layers[include], data[include]) + }, + + get_layer_key = function(params, layers, data) { decor <- lapply(layers, function(layer) { matched_aes <- matched_aes(layer, params) - # Check if this layer should be included - if (!include_layer_in_guide(layer, matched_aes)) { - return(NULL) - } - if (length(matched_aes) > 0) { # Filter out aesthetics that can't be applied to the legend n <- lengths(layer$aes_params, use.names = FALSE) @@ -334,10 +343,6 @@ GuideLegend <- ggproto( # Remove NULL geoms params$decor <- compact(decor) - - if (length(params$decor) == 0) { - return(NULL) - } return(params) }, diff --git a/R/guide-old.R b/R/guide-old.R index 88b50f9f37..b2a137fffd 100644 --- a/R/guide-old.R +++ b/R/guide-old.R @@ -103,7 +103,7 @@ GuideOld <- ggproto( guide_transform(params, coord, panel_params) }, - get_layer_key = function(params, layers) { + process_layers = function(self, params, layers, data = NULL) { guide_geom(params, layers, default_mapping = NULL) }, diff --git a/R/guides-.R b/R/guides-.R index e82f35f15c..76bac43de0 100644 --- a/R/guides-.R +++ b/R/guides-.R @@ -278,7 +278,7 @@ Guides <- ggproto( # 5. Guides$assemble() # arrange all guide grobs - build = function(self, scales, layers, labels) { + build = function(self, scales, layers, labels, layer_data) { # Empty guides list no_guides <- guides_list() @@ -304,7 +304,7 @@ Guides <- ggproto( # Merge and process layers guides$merge() - guides$process_layers(layers) + guides$process_layers(layers, layer_data) if (length(guides$guides) == 0) { return(no_guides) } @@ -447,9 +447,9 @@ Guides <- ggproto( }, # Loop over guides to let them extract information from layers - process_layers = function(self, layers) { + process_layers = function(self, layers, data = NULL) { self$params <- Map( - function(guide, param) guide$get_layer_key(param, layers), + function(guide, param) guide$process_layers(param, layers, data), guide = self$guides, param = self$params ) diff --git a/R/plot-build.R b/R/plot-build.R index bc6a9a8706..10ffaa9ae5 100644 --- a/R/plot-build.R +++ b/R/plot-build.R @@ -91,7 +91,7 @@ ggplot_build.ggplot <- function(plot) { npscales <- scales$non_position_scales() if (npscales$n() > 0) { lapply(data, npscales$train_df) - plot$guides <- plot$guides$build(npscales, plot$layers, plot$labels) + plot$guides <- plot$guides$build(npscales, plot$layers, plot$labels, data) data <- lapply(data, npscales$map_df) } else { # Assign empty guides if there are no non-position scales From 9066c649fa0876760ad6a9750801070ea4051a2c Mon Sep 17 00:00:00 2001 From: Teun van den Brand <49372158+teunbrand@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:03:16 +0100 Subject: [PATCH 8/8] Add news bullet --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 254c49abd1..658b967414 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* (internal) guide building is now part of `ggplot_build()` instead of + `ggplot_gtable()` to allow guides to observe unmapped data (#5483). + * Legend titles no longer take up space if they've been removed by setting `legend.title = element_blank()` (@teunbrand, #3587).