Skip to content

Commit b2f7d3d

Browse files
committed
Fix #2467: multi-aesthetic scales and identity scale handling
1 parent defff7d commit b2f7d3d

File tree

6 files changed

+31
-7
lines changed

6 files changed

+31
-7
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ See the [plotly.js releases page](https://github.com/plotly/plotly.js/releases)
3030
* Closed #2455, #2460: `ggplotly()` no longer creates empty shapes when `panel.border` is `element_blank()` (ggplot2 4.0.0 compatibility).
3131
* Closed #2466: `ggplotly()` no longer errors when `scale_*_manual()` has unused aesthetics (e.g., `aesthetics = c("colour", "fill")` when only colour is used).
3232
* Closed #2305: `ggplotly()` now respects `geom_boxplot(outlier.shape = NA)` to hide outlier points.
33+
* Closed #2467: `ggplotly()` now correctly shows legends and splits traces when scales have multiple aesthetics.
3334

3435
# plotly 4.11.0
3536

R/layers2traces.R

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,16 @@ layers2traces <- function(data, prestats_data, layout, p) {
7878
}, data, hoverTextAes)
7979

8080
# draw legends only for discrete scales
81+
# Register each aesthetic separately for proper legend matching (fixes #2467)
82+
# When a scale has multiple aesthetics (e.g., c("colour", "fill")), we need
83+
# individual entries so "colour_plotlyDomain" matches discreteScales[["colour"]]
84+
# Skip identity scales (guide = "none") as they don't produce legends or trace splitting
8185
discreteScales <- list()
8286
for (sc in p$scales$non_position_scales()$scales) {
83-
if (sc$is_discrete()) {
84-
nm <- paste(sc$aesthetics, collapse = "_")
85-
discreteScales[[nm]] <- sc
87+
if (sc$is_discrete() && !identical(sc$guide, "none")) {
88+
for (aes_name in sc$aesthetics) {
89+
discreteScales[[aes_name]] <- sc
90+
}
8691
}
8792
}
8893
# Convert "high-level" geoms to their "low-level" counterpart

tests/testthat/_snaps/ggplot-boxplot/boxplot-fillcolor.svg

Lines changed: 1 addition & 1 deletion
Loading

tests/testthat/_snaps/ggplot-point/all-shapes.svg

Lines changed: 1 addition & 1 deletion
Loading

tests/testthat/_snaps/ggridges/varying-fill-colours.svg

Lines changed: 1 addition & 1 deletion
Loading

tests/testthat/test-ggplot-color.R

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,25 @@ test_that("scale_*_manual with unused aesthetics does not error (#2466)", {
1717
aesthetics = c("colour", "fill")
1818
)
1919
# Should not error with "undefined columns selected"
20-
# (Note: trace splitting with multi-aesthetic scales is a separate issue #2467)
2120
expect_error(plotly_build(ggplotly(p)), NA)
2221
})
2322

23+
test_that("multi-aesthetic scales show legend and split traces correctly (#2467)", {
24+
# When scale has aesthetics = c('colour', 'fill') and both are used,
25+
# traces should be split by the variable and legend should appear
26+
p <- ggplot(iris, aes(Sepal.Length, Sepal.Width, colour = Species, fill = Species)) +
27+
geom_point(shape = 21) +
28+
scale_colour_manual(
29+
values = c("setosa" = "red", "versicolor" = "blue", "virginica" = "green"),
30+
aesthetics = c("colour", "fill")
31+
)
32+
L <- plotly_build(ggplotly(p))$x
33+
# Should have 3 traces (one per Species level)
34+
expect_equal(length(L$data), 3)
35+
# Should show legend
36+
expect_true(any(sapply(L$data, function(d) isTRUE(d$showlegend))))
37+
# Trace names should be the species names
38+
trace_names <- sapply(L$data, function(d) d$name)
39+
expect_true(all(c("setosa", "versicolor", "virginica") %in% trace_names))
40+
})
41+

0 commit comments

Comments
 (0)