diff --git a/NEWS.md b/NEWS.md index 174a6a732f..2bf1e22ed5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,8 @@ * Closed #2483: `save_image()` no longer embeds Windows file paths directly into Python source passed to `reticulate`, fixing static image export with Kaleido on Windows. +* #2495: `ggplotly()` now keeps merged legend titles in sync with their legend entries, fixing a case where an upstream change in `{rlang}`'s hashing could silently reorder combined legend titles (e.g. `factor(vs)
factor(cyl)`) out of step with the entries they label. + # plotly 4.12.0 diff --git a/R/ggplotly.R b/R/ggplotly.R index 1c4461fc15..afe8befbf3 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -1592,6 +1592,23 @@ get_gdefs_ggproto <- function(scales, theme, plot, layers, layer_data) { if (length(guides$guides) > 0) { guides$merge() guides$process_layers(layers, layer_data) + # guides$merge() sorts guides by their explicit `order` (guide_legend(order=)), + # but breaks ties (the common case, since `order` defaults to 0/unset for + # everyone) using a content hash that bears no relation to aesthetic + # declaration order (ggplot2's `Guides$merge()`, via rlang::hash()). + # layers2traces() names legend entries in aesthetic declaration order (see + # `discreteScales`), so on ties, guides must be put back in that same + # order for the combined legend title (built below) to agree with the + # entries it labels -- while still respecting any explicit `order`. + guide_order <- vapply(guides$params, function(p) { + o <- p$order %||% 0 + if (identical(o, 0)) 99L else as.integer(o) + }, integer(1)) + declaration_order <- match(guides$aesthetics, aesthetics) + ord <- order(guide_order, declaration_order) + guides$guides <- guides$guides[ord] + guides$params <- guides$params[ord] + guides$aesthetics <- guides$aesthetics[ord] } # Add old legend/colorbar classes to guide params so that ggplotly() code # can continue to work the same way it always has diff --git a/tests/testthat/_snaps/ggplot-size/size-global-scaling.svg b/tests/testthat/_snaps/ggplot-size/size-global-scaling.svg index 98c56dd7f3..ac1dbc9f20 100644 --- a/tests/testthat/_snaps/ggplot-size/size-global-scaling.svg +++ b/tests/testthat/_snaps/ggplot-size/size-global-scaling.svg @@ -1 +1 @@ -2340.40.81.21.6populationcountryParaguayPeruPhilippineseduilln +2340.40.81.21.6countrypopulationParaguayPeruPhilippineseduilln diff --git a/tests/testthat/_snaps/ggplot-theme/theme-marker-default.svg b/tests/testthat/_snaps/ggplot-theme/theme-marker-default.svg index cb7d459437..79dc683dc8 100644 --- a/tests/testthat/_snaps/ggplot-theme/theme-marker-default.svg +++ b/tests/testthat/_snaps/ggplot-theme/theme-marker-default.svg @@ -1 +1 @@ -2340.40.81.21.6populationcountryParaguayPeruPhilippineseduilln +2340.40.81.21.6countrypopulationParaguayPeruPhilippineseduilln diff --git a/tests/testthat/test-ggplot-facets.R b/tests/testthat/test-ggplot-facets.R index d8fd07c627..c862a2f86f 100644 --- a/tests/testthat/test-ggplot-facets.R +++ b/tests/testthat/test-ggplot-facets.R @@ -6,9 +6,9 @@ test_that("6 facets becomes 6 panels", { barley <- structure(list( yield = c(27, 48.86667, 27.43334, 39.93333, 32.96667, 28.96667, 43.06666, 55.2, 28.76667, 38.13333, 29.13333, 29.66667, 35.13333, 47.33333, 25.76667, 40.46667, 29.66667, 25.7, 39.9, 50.23333, 26.13333, 41.33333, 23.03333, 26.3, 36.56666, 63.8333, 43.76667, 46.93333, 29.76667, 33.93333, 43.26667, 58.1, 28.7, 45.66667, 32.16667, 33.6, 36.6, 65.7667, 30.36667, 48.56666, 24.93334, 28.1, 32.76667, 48.56666, 29.86667, 41.6, 34.7, 32, 24.66667, 46.76667, 22.6, 44.1, 19.7, 33.06666, 39.3, 58.8, 29.46667, 49.86667, 34.46667, 31.6, 26.9, 33.46667, 34.36666, 32.96667, 22.13333, 22.56667, 36.8, 37.73333, 35.13333, 26.16667, 14.43333, 25.86667, 27.43334, 38.5, 35.03333, 20.63333, 16.63333, 22.23333, 26.8, 37.4, 38.83333, 32.06666, 32.23333, 22.46667, 29.06667, 49.2333, 46.63333, 41.83333, 20.63333, 30.6, 26.43334, 42.2, 43.53334, 34.33333, 19.46667, 22.7, 25.56667, 44.7, 47, 30.53333, 19.9, 22.5, 28.06667, 36.03333, 43.2, 25.23333, 26.76667, 31.36667, 30, 41.26667, 44.23333, 32.13333, 15.23333, 27.36667, 38, 58.16667, 47.16667, 35.9, 20.66667, 29.33333), variety = structure(c(3L, 3L, 3L, 3L, 3L, 3L, 7L, 7L, 7L, 7L, 7L, 7L, 1L, 1L, 1L, 1L, 1L, 1L, 5L, 5L, 5L, 5L, 5L, 5L, 10L, 10L, 10L, 10L, 10L, 10L, 8L, 8L, 8L, 8L, 8L, 8L, 2L, 2L, 2L, 2L, 2L, 2L, 6L, 6L, 6L, 6L, 6L, 6L, 4L, 4L, 4L, 4L, 4L, 4L, 9L, 9L, 9L, 9L, 9L, 9L, 3L, 3L, 3L, 3L, 3L, 3L, 7L, 7L, 7L, 7L, 7L, 7L, 1L, 1L, 1L, 1L, 1L, 1L, 5L, 5L, 5L, 5L, 5L, 5L, 10L, 10L, 10L, 10L, 10L, 10L, 8L, 8L, 8L, 8L, 8L, 8L, 2L, 2L, 2L, 2L, 2L, 2L, 6L, 6L, 6L, 6L, 6L, 6L, 4L, 4L, 4L, 4L, 4L, 4L, 9L, 9L, 9L, 9L, 9L, 9L), - .Label = c("Svansota", "No. 462", "Manchuria", "No. 475", "Velvet", "Peatland", "Glabron", "No. 457", "Wisconsin No. 38", "Trebi"), class = "factor"), - year = structure(c(2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L), .Label = c("1932", "1931"), class = "factor"), - site = structure(c(3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L), .Label = c("Grand Rapids", "Duluth", "University Farm", "Morris", "Crookston", "Waseca"), class = "factor")), + levels = c("Svansota", "No. 462", "Manchuria", "No. 475", "Velvet", "Peatland", "Glabron", "No. 457", "Wisconsin No. 38", "Trebi"), class = "factor"), + year = structure(c(2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 2L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L, 1L), levels = c("1932", "1931"), class = "factor"), + site = structure(c(3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L, 3L, 6L, 4L, 5L, 1L, 2L), levels = c("Grand Rapids", "Duluth", "University Farm", "Morris", "Crookston", "Waseca"), class = "factor")), row.names = c("1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55", "56", "57", "58", "59", "60", "61", "62", "63", "64", "65", "66", "67", "68", "69", "70", "71", "72", "73", "74", "75", "76", "77", "78", "79", "80", "81", "82", "83", "84", "85", "86", "87", "88", "89", "90", "91", "92", "93", "94", "95", "96", "97", "98", "99", "100", "101", "102", "103", "104", "105", "106", "107", "108", "109", "110", "111", "112", "113", "114", "115", "116", "117", "118", "119", "120"), class = "data.frame" ) diff --git a/tests/testthat/test-ggplot-legend.R b/tests/testthat/test-ggplot-legend.R index 9dd92ea96c..50f362d21b 100644 --- a/tests/testthat/test-ggplot-legend.R +++ b/tests/testthat/test-ggplot-legend.R @@ -1,5 +1,4 @@ - expect_traces <- function(gg, n.traces, name){ stopifnot(is.numeric(n.traces)) L <- expect_doppelganger_built(gg, paste0("legend-", name)) @@ -28,9 +27,11 @@ test_that("Discrete colour and shape get merged into one legend", { expect_identical( nms, paste0("(", d$vs, ",", d$cyl, ")") ) + # legend title segments must appear in the same order as the aesthetics + # within each entry's name (e.g. "(vs_value,cyl_value)" above), since both + # describe the same combined legend legend_title <- info$layout$legend$title$text - expect_match(legend_title, "^factor\\(vs\\)") - expect_match(legend_title, "factor\\(cyl\\)$") + expect_identical(legend_title, "factor(vs)
factor(cyl)") }) @@ -89,8 +90,11 @@ test_that("legend can be manipulated via guides(aes = guide_xxx())", { theme(axis.text.x = element_text(angle = 90)) info <- expect_doppelganger_built(gg, "respect-guides") - + expect_equivalent(sum(sapply(info$data, "[[", "showlegend")), 4) + # explicit guide `order` must be respected, even though it disagrees with + # the shape-before-color order the aesthetics were declared in above + expect_identical(info$layout$legend$title$text, "Period
") }) p <- ggplot(mtcars, aes(x = mpg, y = wt, color = factor(vs))) + diff --git a/tests/testthat/test-group2NA.R b/tests/testthat/test-group2NA.R index 9321c4ce50..81873266a8 100644 --- a/tests/testthat/test-group2NA.R +++ b/tests/testthat/test-group2NA.R @@ -63,7 +63,7 @@ test_that("group2NA() yields the correct result", { id = c(27L, NA, 3L, 8L, 9L, 18L, 19L, 20L, 21L, 26L, 28L, 32L, 1L, 2L, 30L, NA, 4L, 6L, 10L, 11L, 5L, 7L, 12L, 13L, 14L, 15L, 16L, 17L, 22L, 23L, 24L, 25L, 29L, 31L)), - .Names = c("mpg", "cyl", "disp", "hp", "drat", "wt", "qsec", "vs", "am", "gear", "carb", "id"), + names = c("mpg", "cyl", "disp", "hp", "drat", "wt", "qsec", "vs", "am", "gear", "carb", "id"), class = "data.frame", row.names = c(NA, 34L))