Merge pull request #1388 from ropensci/has-fill

Fix the definition of has_fill()

Author: cpsievert

R/plotly_build.R

@@ -1001,4 +1001,13 @@ supplyUserPalette <- function(default, user) {
 
 # helper functions
 array_ok <- function(attr) isTRUE(tryNULL(attr$arrayOk))
-has_fill <- function(trace) isTRUE(trace$fill %in% c('tozeroy', 'tozerox', 'tonexty', 'tonextx', 'toself', 'tonext'))
+has_fill <- function(trace) {
+  trace_type <- trace[["type"]] %||% "scatter"
+  # if trace type has fillcolor, but no fill attribute, then fill is always relevant
+  has_fillcolor <- has_attr(trace_type, "fillcolor")
+  has_fill <- has_attr(trace_type, "fill")
+  if (has_fillcolor && !has_fill) return(TRUE)
+  fill <- trace[["fill"]] %||% "none"
+  if (has_fillcolor && isTRUE(fill != "none")) return(TRUE)
+  FALSE
+}

tests/figs/plotly-color/plotly-color-box-color-stroke.svg

@@ -63,6 +63,17 @@ test_that("Mapping a numeric variable to color works", {
   expect_true(all(0 <= markerScale$colorscale[,1] & markerScale$colorscale[,1] <= 1))
 })
 
+test_that("color/stroke mapping with box translates correctly", {
+  d <- data.frame(x = rep(c("A", "B"), each = 5), y = rnorm(10))
+  l <- plot_ly(d) %>% 
+    add_boxplot(x = ~x, y = ~y, color = ~x, colors = c('A' = "blue", 'B' = "red"), stroke = I("black")) %>%
+    expect_traces(2, "box-color-stroke")
+  expect_true(l$data[[1]]$fillcolor == toRGB("blue", 0.5))
+  expect_true(l$data[[2]]$fillcolor == toRGB("red", 0.5))
+  expect_true(l$data[[1]]$line$color == toRGB("black"))
+  expect_true(l$data[[2]]$line$color == toRGB("black"))
+})
+
 test_that("Custom RColorBrewer pallette works for numeric variable", {
   p <- plot_ly(iris, x = ~Sepal.Length, y = ~Petal.Length, 
                color = ~Petal.Width, colors = "Greens")