diff --git a/NEWS.md b/NEWS.md index dd222c6a1a..ee9db64c34 100644 --- a/NEWS.md +++ b/NEWS.md @@ -57,6 +57,8 @@ * `geom_sf()` now removes rows that contain missing `shape`/`size`/`colour` (#3483, @yutannihilation) +* Fix a bug when `show.legend` is a named logical vector (#3461, @yutannihilation). + # ggplot2 3.2.1 This is a patch release fixing a few regressions introduced in 3.2.0 as well as diff --git a/R/guide-colorbar.r b/R/guide-colorbar.r index e84ada36a0..4250443b9d 100644 --- a/R/guide-colorbar.r +++ b/R/guide-colorbar.r @@ -244,10 +244,28 @@ guide_geom.colorbar <- function(guide, layers, default_mapping) { guide_layers <- lapply(layers, function(layer) { matched <- matched_aes(layer, guide, default_mapping) - if (length(matched) > 0 && (isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend))) { + if (length(matched) == 0) { + # This layer does not use this guide + return(NULL) + } + + # check if this layer should be included, different behaviour depending on + # if show.legend is a logical or a named logical vector + if (is_named(layer$show.legend)) { + layer$show.legend <- rename_aes(layer$show.legend) + show_legend <- layer$show.legend[matched] + # we cannot use `isTRUE(is.na(show_legend))` here because + # 1. show_legend can be multiple NAs + # 2. isTRUE() was not tolerant for a named TRUE + show_legend <- show_legend[!is.na(show_legend)] + include <- length(show_legend) == 0 || any(show_legend) + } else { + include <- isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend) + } + + if (include) { layer } else { - # This layer does not use this guide NULL } }) diff --git a/R/guide-legend.r b/R/guide-legend.r index ecd55fdb52..3558d38b1f 100644 --- a/R/guide-legend.r +++ b/R/guide-legend.r @@ -250,10 +250,14 @@ guide_geom.legend <- function(guide, layers, default_mapping) { # check if this layer should be included, different behaviour depending on # if show.legend is a logical or a named logical vector - if (!is.null(names(layer$show.legend))) { + if (is_named(layer$show.legend)) { layer$show.legend <- rename_aes(layer$show.legend) - include <- is.na(layer$show.legend[matched]) || - layer$show.legend[matched] + show_legend <- layer$show.legend[matched] + # we cannot use `isTRUE(is.na(show_legend))` here because + # 1. show_legend can be multiple NAs + # 2. isTRUE() was not tolerant for a named TRUE + show_legend <- show_legend[!is.na(show_legend)] + include <- length(show_legend) == 0 || any(show_legend) } else { include <- isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend) } diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 2bd7d0b508..6ef54fcaf5 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -44,6 +44,11 @@ test_that("show.legend handles named vectors", { p <- ggplot(df, aes(x = x, y = y, color = x, shape = factor(y))) + geom_point(size = 20, show.legend = c(color = FALSE, shape = FALSE)) expect_equal(n_legends(p), 0) + + # c.f.https://github.com/tidyverse/ggplot2/issues/3461 + p <- ggplot(df, aes(x = x, y = y, color = x, shape = factor(y))) + + geom_point(size = 20, show.legend = c(shape = FALSE, color = TRUE)) + expect_equal(n_legends(p), 1) }) test_that("axis_label_overlap_priority always returns the correct number of elements", {