Skip to content

Commit 621f783

Browse files
committed
Merge branch 'master' into scattergl
2 parents 46ea041 + c3c62ec commit 621f783

File tree

6 files changed

+96
-6
lines changed

6 files changed

+96
-6
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
* Bug fix for linking views with crosstalk where the source of the selection is an aggregated trace (see #1218).
2323
* Fixed algorithm for coercing the proposed layout to the plot schema (see #1156).
2424
* grid conversions in `ggplotly()` weren't respected a specified `height`/`width` in RStudio (see #1190).
25+
* `add_*()` no longer inherits `crosstalk::SharedData` key information when `inherit = FALSE`, see #1242.
26+
* The `limits` argument of `colorbar()` wasn't being applied to `line.color`/`line.cmin`/`line.cmax` (see #1236)
2527

2628
# 4.7.1
2729

R/add.R

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ add_trace <- function(p, ...,
5757

5858
# "native" plotly arguments
5959
attrs <- list(...)
60+
attrs$inherit <- inherit
6061

6162
if (!is.null(attrs[["group"]])) {
6263
warning("The group argument has been deprecated. Use group_by() or split instead.")

R/helpers.R

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,23 @@ colorbar_built <- function(p, ..., limits = NULL, which = 1) {
6161
p$x$data[[i]]$zmax <- limits[2]
6262
} else {
6363
# since the colorscale is in a different trace, retrain all traces
64+
# TODO: without knowing which traces are tied to the colorscale,
65+
# there are always going to be corner-cases where limits are applied
66+
# to more traces than it should
6467
p$x$data <- lapply(p$x$data, function(x) {
68+
type <- x[["type"]] %||% "scatter"
6569
col <- x$marker[["color"]]
66-
x$marker[["color"]][col < limits[1] | limits[2] < col] <- default(NA)
67-
x$marker[["cmin"]] <- default(limits[1])
68-
x$marker[["cmax"]] <- default(limits[2])
70+
if (has_color_array(type, "marker") && is.numeric(col)) {
71+
x$marker[["color"]][col < limits[1] | limits[2] < col] <- default(NA)
72+
x$marker[["cmin"]] <- default(limits[1])
73+
x$marker[["cmax"]] <- default(limits[2])
74+
}
75+
col <- x$line[["color"]]
76+
if (has_color_array(type, "line") && is.numeric(col)) {
77+
x$line[["color"]][col < limits[1] | limits[2] < col] <- default(NA)
78+
x$line[["cmin"]] <- default(limits[1])
79+
x$line[["cmax"]] <- default(limits[2])
80+
}
6981
x
7082
})
7183
}

R/plotly_build.R

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ plotly_build.plotly <- function(p, registerFrames = TRUE) {
132132
)
133133

134134
# attach crosstalk info, if necessary
135-
if (crosstalk_key() %in% names(dat)) {
135+
if (crosstalk_key() %in% names(dat) && isTRUE(trace[["inherit"]])) {
136136
trace[["key"]] <- trace[["key"]] %||% dat[[crosstalk_key()]]
137137
trace[["set"]] <- trace[["set"]] %||% attr(dat, "set")
138138
}
@@ -310,7 +310,7 @@ plotly_build.plotly <- function(p, registerFrames = TRUE) {
310310
mappingAttrs <- c(
311311
"alpha", "alpha_stroke", npscales(), paste0(npscales(), "s"),
312312
".plotlyGroupIndex", ".plotlyMissingIndex",
313-
".plotlyTraceIndex", ".plotlyVariableMapping"
313+
".plotlyTraceIndex", ".plotlyVariableMapping", "inherit"
314314
)
315315
for (j in mappingAttrs) {
316316
traces[[i]][[j]] <- NULL
@@ -840,7 +840,6 @@ map_color <- function(traces, stroke = FALSE, title = "", colorway, na.color = "
840840
}
841841

842842
# make sure the colorscale is going to convert to JSON nicely
843-
# TODO:
844843
traces[[i]]$marker$colorscale <- as_df(traces[[i]]$marker$colorscale)
845844
}
846845
}

tests/testthat/test-animate-highlight.R

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,32 @@ test_that("SharedData produces key/set in ggplotly", {
2424
expect_false(tr$`_isSimpleKey` %||% FALSE)
2525
})
2626

27+
28+
29+
test_that("crosstalk keys are inherited in a layer with inherit = FALSE", {
30+
31+
p <- txhousing %>%
32+
group_by(city) %>%
33+
crosstalk::SharedData$new(~city, "Select a city") %>%
34+
plot_ly(x = ~date, y = ~median) %>%
35+
add_lines(alpha = 0.2) %>%
36+
add_ribbons(
37+
x = c(2016, 2017), ymin = c(150000, 160000), ymax = c(200000, 190000),
38+
inherit = FALSE
39+
)
40+
41+
b <- plotly_build(p)
42+
# second trace should have key/set info
43+
expect_null(b$x$data[[2]][["key"]])
44+
expect_null(b$x$data[[2]][["set"]])
45+
# first trace should
46+
k <- unique(b$x$data[[1]]$key)
47+
expect_equal(sort(k[!is.na(k)]), sort(unique(txhousing$city)))
48+
expect_true(b$x$data[[1]][["set"]] == "Select a city")
49+
})
50+
51+
52+
2753
# Ignore for now https://github.com/ggobi/ggally/issues/264
2854
#test_that("SharedData produces key/set in ggpairs", {
2955
# p <- GGally::ggpairs(m, columns = 1:3)

tests/testthat/test-plotly-colorbar.R

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,58 @@ test_that("positioning with multiple colorbars and legends", {
135135

136136
expect_true(b$x$layout$legend$y == 0.5)
137137
expect_true(b$x$layout$legend$yanchor == "top")
138+
})
139+
140+
141+
test_that("Colorbar limits controls marker.color and line.color", {
138142

143+
# https://github.com/ropensci/plotly/issues/1236
144+
p <- plot_ly(
145+
mtcars, x = ~hp, y = ~cyl, z = ~mpg, color = ~mpg,
146+
type = "scatter3d", mode = "lines+markers"
147+
)
148+
149+
b <- plotly_build(p)
150+
expect_length(b$x$data, 2)
151+
expect_true(all(b$x$data[[1]]$marker$color == mtcars$mpg))
152+
expect_true(all(b$x$data[[1]]$line$color == mtcars$mpg))
153+
expect_true(b$x$data[[1]]$marker$cmin == min(mtcars$mpg))
154+
expect_true(b$x$data[[1]]$marker$cmax == max(mtcars$mpg))
155+
expect_true(b$x$data[[1]]$line$cmin == min(mtcars$mpg))
156+
expect_true(b$x$data[[1]]$line$cmax == max(mtcars$mpg))
157+
158+
p2 <- colorbar(p, limits = c(0, 100))
159+
b2 <- plotly_build(p2)
160+
expect_length(b2$x$data, 2)
161+
expect_true(all(b2$x$data[[1]]$marker$color == mtcars$mpg))
162+
expect_true(all(b2$x$data[[1]]$line$color == mtcars$mpg))
163+
expect_true(b2$x$data[[1]]$marker$cmin == 0)
164+
expect_true(b2$x$data[[1]]$marker$cmax == 100)
165+
expect_true(b2$x$data[[1]]$line$cmin == 0)
166+
expect_true(b2$x$data[[1]]$line$cmax == 100)
167+
168+
p3 <- colorbar(p, limits = c(20, 100))
169+
b3 <- plotly_build(p3)
170+
mpg <- mtcars$mpg
171+
mpg[mpg < 20] <- NA
172+
expect_true(Reduce(`&`, Map(identical, b3$x$data[[1]]$marker$color, mpg)))
173+
expect_true(Reduce(`&`, Map(identical, b3$x$data[[1]]$line$color, mpg)))
174+
expect_true(b3$x$data[[1]]$marker$cmin == 20)
175+
expect_true(b3$x$data[[1]]$marker$cmax == 100)
176+
expect_true(b3$x$data[[1]]$line$cmin == 20)
177+
expect_true(b3$x$data[[1]]$line$cmax == 100)
178+
})
179+
180+
test_that("colorbar limits shouldn't control non-color-scale mapping(s)", {
139181

182+
p <- plot_ly(x = 1:10, y = 1:10, color = 1:10) %>%
183+
add_markers() %>%
184+
add_lines(x = 1:3, y = 1:3, color = I('red')) %>%
185+
colorbar(limits = c(1, 5))
140186

187+
b <- plotly_build(p)
188+
expect_length(b$x$data, 3)
141189

190+
expect_true(sum(is.na(b$x$data[[1]]$marker$color)) == 5)
191+
expect_true(b$x$data[[2]]$line$color == toRGB("red"))
142192
})

0 commit comments

Comments
 (0)