From 2f728380070728e8139a038093595e6303061269 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Wed, 6 Mar 2019 16:45:13 -0600
Subject: [PATCH 1/4] Ensure size mapping always leads to array of appropriate
 length

---
 R/utils.R                         | 18 ++++++++++++++----
 tests/testthat/test-plotly-size.R | 10 ++++++++--
 tests/testthat/test-plotly.R      |  3 ++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index 9dbfa4b217..5b6cf640e6 100644
--- a/R/utils.R
+++ b/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$marker[["size"]] <- default(i(rep(s, n)))
+      }
     }
     
     # do the same for "sub-attributes"
diff --git a/tests/testthat/test-plotly-size.R b/tests/testthat/test-plotly-size.R
index 3ca0fa6096..0bf55592c4 100644
--- a/tests/testthat/test-plotly-size.R
+++ b/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,11 @@ 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_equivalent(d[[1]]$marker$size, rep(30, 10))
 })
diff --git a/tests/testthat/test-plotly.R b/tests/testthat/test-plotly.R
index 9a18e0919c..803c18284d 100644
--- a/tests/testthat/test-plotly.R
+++ b/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)
   

From 6791e4ffe7b44e265bce4a2cb6c44c8418396ddd Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Thu, 7 Mar 2019 15:36:02 -0600
Subject: [PATCH 2/4] marker.size expansion should be aware of lat/lon as well

---
 R/utils.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/utils.R b/R/utils.R
index 5b6cf640e6..3d39576bd7 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -510,7 +510,7 @@ verify_attr <- function(proposed, schema) {
         # 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"]])
+        n <- length(proposed[["x"]] %||% proposed[["y"]] %||% proposed[["lat"]] %||% proposed[["lon"]])
         proposed$marker[["size"]] <- default(i(rep(s, n)))
       }
     }

From 7db57c87e324fa45b643054347ca5d7051aafa22 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Thu, 7 Mar 2019 15:36:38 -0600
Subject: [PATCH 3/4] fix test expectations

---
 tests/testthat/test-plotly-sf.R   | 6 +++---
 tests/testthat/test-plotly-size.R | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/testthat/test-plotly-sf.R b/tests/testthat/test-plotly-sf.R
index 9e02f06bdf..8916421544 100644
--- a/tests/testthat/test-plotly-sf.R
+++ b/tests/testthat/test-plotly-sf.R
@@ -240,14 +240,14 @@ 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_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_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 +256,7 @@ 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_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)
diff --git a/tests/testthat/test-plotly-size.R b/tests/testthat/test-plotly-size.R
index 0bf55592c4..87b972dff5 100644
--- a/tests/testthat/test-plotly-size.R
+++ b/tests/testthat/test-plotly-size.R
@@ -22,5 +22,5 @@ test_that("sizemode is always respected", {
 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_equivalent(d[[1]]$marker$size, rep(30, 10))
+  expect_true(all(d[[1]]$marker$size == 30))
 })

From a73f5273371228146c25576734aeb88b3da8e410 Mon Sep 17 00:00:00 2001
From: Carson Sievert <cpsievert1@gmail.com>
Date: Thu, 7 Mar 2019 16:02:34 -0600
Subject: [PATCH 4/4] add tests for marker.size length

---
 tests/testthat/test-plotly-sf.R   | 3 +++
 tests/testthat/test-plotly-size.R | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tests/testthat/test-plotly-sf.R b/tests/testthat/test-plotly-sf.R
index 8916421544..8732382ef7 100644
--- a/tests/testthat/test-plotly-sf.R
+++ b/tests/testthat/test-plotly-sf.R
@@ -240,6 +240,7 @@ test_that("sizing constants", {
   p <- plot_mapbox(mn_pts, size = I(30)) %>% plotly_build()
   d <- p$x$data
   expect_length(d, 1)
+  expect_length(d[[1]]$marker$size, nrow(mn_pts))
   expect_true(all(d[[1]]$marker$size == 30))
   expect_true(d[[1]]$marker$sizemode == "area")
   
@@ -247,6 +248,7 @@ test_that("sizing constants", {
   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_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)
@@ -256,6 +258,7 @@ 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_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)
diff --git a/tests/testthat/test-plotly-size.R b/tests/testthat/test-plotly-size.R
index 87b972dff5..715ac202ad 100644
--- a/tests/testthat/test-plotly-size.R
+++ b/tests/testthat/test-plotly-size.R
@@ -22,5 +22,6 @@ test_that("sizemode is always respected", {
 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))
 })