From 9352fd472bf57899f3ee20620f1713e0f26f2ae1 Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Mon, 5 Aug 2024 07:27:12 +0200 Subject: [PATCH 1/3] Bug fix. Account for multi_line=FALSE when facet rows/cols are missing. --- R/ggplotly.R | 54 ++++++++++++++++++----------- tests/testthat/test-ggplot-facets.R | 41 ++++++++++++++++++++++ 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 1c2073f9b6..f4ea40f61c 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -917,11 +917,15 @@ gg2list <- function(p, width = NULL, height = NULL, # facet strips -> plotly annotations if (has_facet(plot)) { col_vars <- ifelse(inherits(plot$facet, "FacetWrap"), "facets", "cols") - col_txt <- paste( - plot$facet$params$labeller( - lay[names(plot$facet$params[[col_vars]])] - ), collapse = br() - ) + col_txt <- if (!length(names(plot$facet$params[[col_vars]])) == 0) { + paste( + plot$facet$params$labeller( + lay[names(plot$facet$params[[col_vars]])] + ), collapse = br() + ) + } else { + "" + } if (is_blank(theme[["strip.text.x"]])) col_txt <- "" if (inherits(plot$facet, "FacetGrid") && lay$ROW != 1) col_txt <- "" if (robust_nchar(col_txt) > 0) { @@ -934,22 +938,30 @@ gg2list <- function(p, width = NULL, height = NULL, strip <- make_strip_rect(xdom, ydom, theme, "top") gglayout$shapes <- c(gglayout$shapes, strip) } - row_txt <- paste( - plot$facet$params$labeller( - lay[names(plot$facet$params$rows)] - ), collapse = br() - ) - if (is_blank(theme[["strip.text.y"]])) row_txt <- "" - if (inherits(plot$facet, "FacetGrid") && lay$COL != nCols) row_txt <- "" - if (robust_nchar(row_txt) > 0) { - row_lab <- make_label( - row_txt, x = max(xdom), y = mean(ydom), - el = theme[["strip.text.y"]] %||% theme[["strip.text"]], - xanchor = "left", yanchor = "middle" - ) - gglayout$annotations <- c(gglayout$annotations, row_lab) - strip <- make_strip_rect(xdom, ydom, theme, "right") - gglayout$shapes <- c(gglayout$shapes, strip) + # Only FacetGrid has no cols + if (inherits(plot$facet, "FacetGrid")) { + row_txt <- if (!length(names(plot$facet$params$rows)) == 0) { + paste( + plot$facet$params$labeller( + lay[names(plot$facet$params$rows)] + ), + collapse = br() + ) + } else { + "" + } + if (is_blank(theme[["strip.text.y"]])) row_txt <- "" + if (lay$COL != nCols) row_txt <- "" + if (robust_nchar(row_txt) > 0) { + row_lab <- make_label( + row_txt, x = max(xdom), y = mean(ydom), + el = theme[["strip.text.y"]] %||% theme[["strip.text"]], + xanchor = "left", yanchor = "middle" + ) + gglayout$annotations <- c(gglayout$annotations, row_lab) + strip <- make_strip_rect(xdom, ydom, theme, "right") + gglayout$shapes <- c(gglayout$shapes, strip) + } } } } # end of panel loop diff --git a/tests/testthat/test-ggplot-facets.R b/tests/testthat/test-ggplot-facets.R index d8fd07c627..36d7a35af8 100644 --- a/tests/testthat/test-ggplot-facets.R +++ b/tests/testthat/test-ggplot-facets.R @@ -131,6 +131,47 @@ test_that("facet_grid translates simple labeller function", { ) }) +g <- ggplot(mtcars, aes(mpg, wt)) + + geom_point() + + facet_wrap( ~ vs + am, labeller = function(x) label_both(x, multi_line = FALSE)) + +test_that("facet_wrap accounts for multi_line=FALSE", { + info <- expect_doppelganger_built(g, "facet_wrap-labeller-no-multi-line") + txt <- sapply(info$layout$annotations, "[[", "text") + expect_true(all(!grepl("expression(list())", txt, fixed = TRUE))) + expect_true( + all(c("vs, am: 0, 0", "vs, am: 0, 1", "vs, am: 1, 0", "vs, am: 1, 1") %in% txt) + ) + expect_identical(length(txt), 6L) +}) + +g <- ggplot(mtcars, aes(mpg, wt)) + + geom_point() + +g_no_col <- g + + facet_grid(vs + am ~ ., labeller = function(x) label_both(x, multi_line = FALSE)) + +g_no_row <- g + + facet_grid(. ~ vs + am, labeller = function(x) label_both(x, multi_line = FALSE)) + +test_that("facet_grid accounts for multi_line=FALSE", { + info <- expect_doppelganger_built(g_no_col, "facet_grid-labeller-no-col") + txt <- sapply(info$layout$annotations, "[[", "text") + expect_true(all(!grepl("expression(list())", txt, fixed = TRUE))) + expect_true( + all(c("vs, am: 0, 0", "vs, am: 0, 1", "vs, am: 1, 0", "vs, am: 1, 1") %in% txt) + ) + expect_identical(length(txt), 6L) + + info <- expect_doppelganger_built(g_no_row, "facet_grid-labeller-no-col") + txt <- sapply(info$layout$annotations, "[[", "text") + expect_true(all(!grepl("expression(list())", txt, fixed = TRUE))) + expect_true( + all(c("vs, am: 0, 0", "vs, am: 0, 1", "vs, am: 1, 0", "vs, am: 1, 1") %in% txt) + ) + expect_identical(length(txt), 6L) +}) + p <- economics %>% tidyr::gather(variable, value, -date) %>% qplot(data = ., date, value) + facet_wrap(~variable, scale = "free_y", ncol = 2) From 805193b7f3c7e5b8c10f4d602807c78d38e3f54f Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Fri, 6 Sep 2024 08:58:27 +0200 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Carson Sievert --- R/ggplotly.R | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index f4ea40f61c..9a50f2aafe 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -917,15 +917,12 @@ gg2list <- function(p, width = NULL, height = NULL, # facet strips -> plotly annotations if (has_facet(plot)) { col_vars <- ifelse(inherits(plot$facet, "FacetWrap"), "facets", "cols") - col_txt <- if (!length(names(plot$facet$params[[col_vars]])) == 0) { - paste( - plot$facet$params$labeller( - lay[names(plot$facet$params[[col_vars]])] - ), collapse = br() - ) - } else { - "" - } + col_txt <- paste( + plot$facet$params$labeller( + lay[names(plot$facet$params[[col_vars]])] + ), collapse = br() + ) + if (length(names(plot$facet$params[[col_vars]])) == 0) col_txt <- "" if (is_blank(theme[["strip.text.x"]])) col_txt <- "" if (inherits(plot$facet, "FacetGrid") && lay$ROW != 1) col_txt <- "" if (robust_nchar(col_txt) > 0) { From 359ec28b7000943c2dd02c566c26fdb695f1a0fb Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Fri, 6 Sep 2024 09:17:18 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Carson Sievert --- R/ggplotly.R | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 9a50f2aafe..a311761289 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -937,16 +937,13 @@ gg2list <- function(p, width = NULL, height = NULL, } # Only FacetGrid has no cols if (inherits(plot$facet, "FacetGrid")) { - row_txt <- if (!length(names(plot$facet$params$rows)) == 0) { - paste( - plot$facet$params$labeller( - lay[names(plot$facet$params$rows)] - ), - collapse = br() - ) - } else { - "" - } + row_txt <- paste( + plot$facet$params$labeller( + lay[names(plot$facet$params$rows)] + ), + collapse = br() + ) + if (length(names(plot$facet$params$rows)) == 0) row_txt <- "" if (is_blank(theme[["strip.text.y"]])) row_txt <- "" if (lay$COL != nCols) row_txt <- "" if (robust_nchar(row_txt) > 0) {