Skip to content

Commit b68912d

Browse files
Handle fine-grained show.legend properly (#3490)
1 parent 18c540e commit b68912d

File tree

4 files changed

+34
-5
lines changed

4 files changed

+34
-5
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@
5757

5858
* `geom_sf()` now removes rows that contain missing `shape`/`size`/`colour` (#3483, @yutannihilation)
5959

60+
* Fix a bug when `show.legend` is a named logical vector (#3461, @yutannihilation).
61+
6062
# ggplot2 3.2.1
6163

6264
This is a patch release fixing a few regressions introduced in 3.2.0 as well as

R/guide-colorbar.r

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,28 @@ guide_geom.colorbar <- function(guide, layers, default_mapping) {
244244
guide_layers <- lapply(layers, function(layer) {
245245
matched <- matched_aes(layer, guide, default_mapping)
246246

247-
if (length(matched) > 0 && (isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend))) {
247+
if (length(matched) == 0) {
248+
# This layer does not use this guide
249+
return(NULL)
250+
}
251+
252+
# check if this layer should be included, different behaviour depending on
253+
# if show.legend is a logical or a named logical vector
254+
if (is_named(layer$show.legend)) {
255+
layer$show.legend <- rename_aes(layer$show.legend)
256+
show_legend <- layer$show.legend[matched]
257+
# we cannot use `isTRUE(is.na(show_legend))` here because
258+
# 1. show_legend can be multiple NAs
259+
# 2. isTRUE() was not tolerant for a named TRUE
260+
show_legend <- show_legend[!is.na(show_legend)]
261+
include <- length(show_legend) == 0 || any(show_legend)
262+
} else {
263+
include <- isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend)
264+
}
265+
266+
if (include) {
248267
layer
249268
} else {
250-
# This layer does not use this guide
251269
NULL
252270
}
253271
})

R/guide-legend.r

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,14 @@ guide_geom.legend <- function(guide, layers, default_mapping) {
250250

251251
# check if this layer should be included, different behaviour depending on
252252
# if show.legend is a logical or a named logical vector
253-
if (!is.null(names(layer$show.legend))) {
253+
if (is_named(layer$show.legend)) {
254254
layer$show.legend <- rename_aes(layer$show.legend)
255-
include <- is.na(layer$show.legend[matched]) ||
256-
layer$show.legend[matched]
255+
show_legend <- layer$show.legend[matched]
256+
# we cannot use `isTRUE(is.na(show_legend))` here because
257+
# 1. show_legend can be multiple NAs
258+
# 2. isTRUE() was not tolerant for a named TRUE
259+
show_legend <- show_legend[!is.na(show_legend)]
260+
include <- length(show_legend) == 0 || any(show_legend)
257261
} else {
258262
include <- isTRUE(is.na(layer$show.legend)) || isTRUE(layer$show.legend)
259263
}

tests/testthat/test-guides.R

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ test_that("show.legend handles named vectors", {
4444
p <- ggplot(df, aes(x = x, y = y, color = x, shape = factor(y))) +
4545
geom_point(size = 20, show.legend = c(color = FALSE, shape = FALSE))
4646
expect_equal(n_legends(p), 0)
47+
48+
# c.f.https://github.com/tidyverse/ggplot2/issues/3461
49+
p <- ggplot(df, aes(x = x, y = y, color = x, shape = factor(y))) +
50+
geom_point(size = 20, show.legend = c(shape = FALSE, color = TRUE))
51+
expect_equal(n_legends(p), 1)
4752
})
4853

4954
test_that("axis_label_overlap_priority always returns the correct number of elements", {

0 commit comments

Comments
 (0)