From 18c471fbf4843077faf0f6eb4d849c793c2cc4c8 Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Fri, 12 Jun 2020 11:16:09 +0200 Subject: [PATCH 1/5] Add fix to take account of linebreaks in yaxis ticklabels. --- R/ggplotly.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index d0c32fae2a..eaefa12dab 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -806,7 +806,12 @@ gg2list <- function(p, width = NULL, height = NULL, # do some stuff that should be done once for the entire plot if (i == 1) { - axisTickText <- longest_element(axisObj$ticktext) + # Split ticktext elements by "\n" to account for linebreaks + axisTickText <- sapply(as.character(axisObj$ticktext), strsplit, split = "\n", USE.NAMES = FALSE) + # Get longest element for each tick + axisTickText <- lapply(axisTickText, longest_element) + # Longest element of all ticks + axisTickText <- longest_element(axisTickText) side <- if (xy == "x") "b" else "l" # account for axis ticks, ticks text, and titles in plot margins # (apparently ggplot2 doesn't support axis.title/axis.text margins) From d441c09cecde1721923ef79d34f59562dfaae61c Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Wed, 17 Jun 2020 15:34:28 +0200 Subject: [PATCH 2/5] Remove unnecessary sapply. --- R/ggplotly.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index eaefa12dab..f630c065dd 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -807,7 +807,7 @@ gg2list <- function(p, width = NULL, height = NULL, # do some stuff that should be done once for the entire plot if (i == 1) { # Split ticktext elements by "\n" to account for linebreaks - axisTickText <- sapply(as.character(axisObj$ticktext), strsplit, split = "\n", USE.NAMES = FALSE) + axisTickText <- strsplit(as.character(axisObj$ticktext), split = "\n") # Get longest element for each tick axisTickText <- lapply(axisTickText, longest_element) # Longest element of all ticks From 98fd6e9dc40555532167f8cae81ae19b600572a5 Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Wed, 17 Jun 2020 15:35:29 +0200 Subject: [PATCH 3/5] Add test linebreaks are taken into account. --- .../ticktext-linebreaks-no-linebreaks.svg | 1 + .../figs/axes/ticktext-linebreaks-one-cat.svg | 1 + tests/figs/axes/ticktext-linebreaks.svg | 1 + tests/testthat/test-ticktext-linebreaks.R | 64 +++++++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 tests/figs/axes/ticktext-linebreaks-no-linebreaks.svg create mode 100644 tests/figs/axes/ticktext-linebreaks-one-cat.svg create mode 100644 tests/figs/axes/ticktext-linebreaks.svg create mode 100644 tests/testthat/test-ticktext-linebreaks.R diff --git a/tests/figs/axes/ticktext-linebreaks-no-linebreaks.svg b/tests/figs/axes/ticktext-linebreaks-no-linebreaks.svg new file mode 100644 index 0000000000..afee904c2d --- /dev/null +++ b/tests/figs/axes/ticktext-linebreaks-no-linebreaks.svg @@ -0,0 +1 @@ +012345ticktextticktext long_ticktext ticktextxy diff --git a/tests/figs/axes/ticktext-linebreaks-one-cat.svg b/tests/figs/axes/ticktext-linebreaks-one-cat.svg new file mode 100644 index 0000000000..566a45677f --- /dev/null +++ b/tests/figs/axes/ticktext-linebreaks-one-cat.svg @@ -0,0 +1 @@ +0.000.250.500.751.00ticktextlong_ticktextticktextxy diff --git a/tests/figs/axes/ticktext-linebreaks.svg b/tests/figs/axes/ticktext-linebreaks.svg new file mode 100644 index 0000000000..34fc751204 --- /dev/null +++ b/tests/figs/axes/ticktext-linebreaks.svg @@ -0,0 +1 @@ +012345ticktextticktextlong_ticktextticktextxy diff --git a/tests/testthat/test-ticktext-linebreaks.R b/tests/testthat/test-ticktext-linebreaks.R new file mode 100644 index 0000000000..f1db6b9560 --- /dev/null +++ b/tests/testthat/test-ticktext-linebreaks.R @@ -0,0 +1,64 @@ +context("axes") + +# Compute margin +comp_margin <- function(gg, axisTickText) { + plot <- ggfun("plot_clone")(gg) + theme <- ggfun("plot_theme")(plot) + elements <- names(which(sapply(theme, inherits, "element"))) + for (i in elements) { + theme[[i]] <- ggplot2::calc_element(i, theme) + } + + pm <- unitConvert(theme$plot.margin, "pixels") + gglayout <- list( + margin = list(t = pm[[1]], r = pm[[2]], b = pm[[3]], l = pm[[4]]) + ) + axisTitle <- theme[["axis.title.y"]] + axisObj <- list( + ticktext = "djdfkjdfdklj", + tickfont = text2font(theme[["axis.text"]], "width"), + ticklen = unitConvert(theme$axis.ticks.length, "pixels", "width") + ) + + gglayout$margin[["l"]] + axisObj$ticklen + + bbox(axisTickText, 0, axisObj$tickfont$size)[["width"]] + + bbox("y", 90, unitConvert(axisTitle, "pixels", "width"))[["width"]] +} + +expect_margin <- function(L, gg, ticktext) { + margin_l <- comp_margin(gg, ticktext) + expect_equal(round(L$layout$margin$l, 10), round(margin_l, 10)) +} + +# Linebreaks +d <- data.frame(x = c(1, 2, 3), y = c("ticktext\nlong_ticktext\nticktext", "ticktext", "ticktext")) +gg <- ggplot(d, aes(x, y)) + geom_bar(stat = "identity") + +test_that("ggplotly takes account of linebreaks in ticktext", { + # Visual Test + L <- expect_doppelganger_built(gg, "ticktext-linebreaks") + # ggplotly returns correct margin + expect_margin(L, gg, "long_ticktext") +}) + +# Linebreaks one category +d <- data.frame(x = c(1), y = c("ticktext\nlong_ticktext\nticktext")) +gg <- ggplot(d, aes(x, y)) + geom_bar(stat = "identity") + +test_that("ggplotly takes account of linebreaks in ticktext with only one category", { + # Visual Test + L <- expect_doppelganger_built(gg, "ticktext-linebreaks-one-cat") + # ggplotly returns correct margin + expect_margin(L, gg, "long_ticktext") +}) + +# No linebreaks +d <- data.frame(x = c(1, 2, 3), y = c("ticktext long_ticktext ticktext", "ticktext", "ticktext")) +gg <- ggplot(d, aes(x, y)) + geom_bar(stat = "identity") + +test_that("ggplotly works with no linebreaks in ticktext", { + # Visual Test + L <- expect_doppelganger_built(gg, "ticktext-linebreaks-no-linebreaks") + # ggplotly returns correct margin + expect_margin(L, gg, "ticktext long_ticktext ticktext") +}) From 1d8d39f08bdf17266ce1dc1e85ce2fd3c60377e6 Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Tue, 23 Jun 2020 11:42:24 +0200 Subject: [PATCH 4/5] Update R/ggplotly.R Co-authored-by: Carson Sievert --- R/ggplotly.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index f630c065dd..86d9f2eade 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -807,7 +807,7 @@ gg2list <- function(p, width = NULL, height = NULL, # do some stuff that should be done once for the entire plot if (i == 1) { # Split ticktext elements by "\n" to account for linebreaks - axisTickText <- strsplit(as.character(axisObj$ticktext), split = "\n") + axisTickText <- strsplit(as.character(axisObj$ticktext), split = "\n", fixed = TRUE) # Get longest element for each tick axisTickText <- lapply(axisTickText, longest_element) # Longest element of all ticks From a96abac24c77344b3ce15abdba2f5707dadec216 Mon Sep 17 00:00:00 2001 From: Stefan Moog Date: Tue, 23 Jun 2020 11:42:44 +0200 Subject: [PATCH 5/5] Update R/ggplotly.R Co-authored-by: Carson Sievert --- R/ggplotly.R | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 86d9f2eade..556aa12326 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -808,10 +808,7 @@ gg2list <- function(p, width = NULL, height = NULL, if (i == 1) { # Split ticktext elements by "\n" to account for linebreaks axisTickText <- strsplit(as.character(axisObj$ticktext), split = "\n", fixed = TRUE) - # Get longest element for each tick - axisTickText <- lapply(axisTickText, longest_element) - # Longest element of all ticks - axisTickText <- longest_element(axisTickText) + axisTickText <- longest_element(unlist(axisTickText)) side <- if (xy == "x") "b" else "l" # account for axis ticks, ticks text, and titles in plot margins # (apparently ggplot2 doesn't support axis.title/axis.text margins)