Merge pull request #1480 from ropensci/marker-size-fix

Ensure size mapping always leads to array of appropriate length

Author: cpsievert

R/utils.R

@@ -497,12 +497,22 @@ verify_attr <- function(proposed, schema) {
       proposed$name <- uniq(proposed$name)
     }
 
-    # if marker.sizemode='area', make sure marker.size is boxed up 
-    # (otherwise, when marker.size is a constant, it always sets the diameter!)
+    # if marker.size was populated via `size` arg (i.e., internal map_size()), 
+    # then it should _always_ be an array
+    # of appropriate length...
+    # (when marker.size is a constant, it always sets the diameter!)
     # https://codepen.io/cpsievert/pen/zazXgw
     # https://github.com/plotly/plotly.js/issues/2735
-    if ("area" %in% proposed$marker$sizemode) {
-      proposed$marker[["size"]] <- i(proposed$marker[["size"]])
+    if (is.default(proposed$marker$size)) {
+      s <- proposed$marker[["size"]]
+      if (length(s) == 1) {
+        # marker.size could be of length 1, but we may have multiple 
+        # markers -- in that case, if marker.size is an array 
+        # of length 1 will result in just one marker
+        # https://codepen.io/cpsievert/pen/aMmOza
+        n <- length(proposed[["x"]] %||% proposed[["y"]] %||% proposed[["lat"]] %||% proposed[["lon"]])
+        proposed$marker[["size"]] <- default(i(rep(s, n)))
+      }
     }
 
     # do the same for "sub-attributes"

tests/testthat/test-plotly-sf.R

@@ -240,14 +240,16 @@ test_that("sizing constants", {
   p <- plot_mapbox(mn_pts, size = I(30)) %>% plotly_build()
   d <- p$x$data
   expect_length(d, 1)
-  expect_true(d[[1]]$marker$size == 30)
+  expect_length(d[[1]]$marker$size, nrow(mn_pts))
+  expect_true(all(d[[1]]$marker$size == 30))
   expect_true(d[[1]]$marker$sizemode == "area")
 
   # span controls marker.line.width
   p <- plot_ly(mn_pts, size = I(30), span = I(10), stroke = I("black")) %>% plotly_build()
   d <- p$x$data
   expect_length(d, 1)
-  expect_true(d[[1]]$marker$size == 30)
+  expect_length(d[[1]]$marker$size, nrow(mn_pts))
+  expect_true(all(d[[1]]$marker$size == 30))
   expect_true(d[[1]]$marker$sizemode == "area")
   expect_true(d[[1]]$marker$line$width == 10)
   expect_true(d[[1]]$marker$line$color == toRGB("black"))
@@ -256,7 +258,8 @@ test_that("sizing constants", {
   p <- plot_ly(mn_pts, size = I(20), error_x = list(value = 5)) %>% plotly_build()
   d <- p$x$data
   expect_length(d, 1)
-  expect_true(d[[1]]$marker$size == 20)
+  expect_length(d[[1]]$marker$size, nrow(mn_pts))
+  expect_true(all(d[[1]]$marker$size == 20))
   expect_true(d[[1]]$marker$sizemode == "area")
   expect_true(d[[1]]$error_x$value == 5)
   expect_true(d[[1]]$error_x$width == 20)

tests/testthat/test-plotly-size.R

@@ -4,7 +4,7 @@ test_that("sizemode is always respected", {
 
   # https://github.com/ropensci/plotly/issues/1133
   p <- plot_ly(mtcars, x = ~wt, y = ~wt, size = ~wt, color = ~as.factor(carb)) 
-  d <- plotly_build(p)$x$data
+  d <- expect_doppelganger_built(p, "sizemode")$data
   expect_length(d, length(unique(mtcars$carb)))
 
   for (i in seq_along(d)) {
@@ -16,5 +16,12 @@ test_that("sizemode is always respected", {
     s <- d[[i]]$marker$size
     if (length(s) == 1) expect_is(s, "AsIs")
   }
-  
+})
+
+
+test_that("size maps correctly to marker.size", {
+  p <- plot_ly(x = 1:10, y = 1:10, size = I(30))
+  d <- expect_doppelganger_built(p, "marker.size")$data
+  expect_length(d[[1]]$marker$size, 10)
+  expect_true(all(d[[1]]$marker$size == 30))
 })

tests/testthat/test-plotly.R

@@ -192,12 +192,13 @@ test_that("Complex example works", {
   l <- expect_traces(p, 3, "time-series-summary")
 })
 
-
 test_that("span/size controls errorbar thickness/width", {
 
   p <- plot_ly(x = 1:10, y = 1:10, error_x = list(value = 3), error_y = list(value = 2), span = I(5), size = I(10), stroke = I("black"), color = I("red")) %>%
     plotly_build()
 
+  expect_doppelganger_built(p, "errorbar.width")
+  
   d <- p$x$data
   expect_length(d, 1)