search

plotly / plotly.R

An interactive graphing library for R
https://plotly-r.com
Other
2.55k stars 625 forks source link

config modeBarButtonsToRemove fails if length is 1 #1433

Open wkdavis opened 5 years ago

wkdavis commented 5 years ago

The plot silently fails to render and throws a javascript error when listing only 1 button for removal with config(modeBarButtonsToRemove = c(...))

# Basic failure
plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = c("hoverCompareCartesian"))

plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = c("zoom2d"))

# Fails even when button is superfluous ("zoom3d" not needed for a 2d plot)
plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = c("zoom3d"))

# Fails when passed as a scalar
plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = "hoverCompareCartesian")
// javscript error from browser
 Uncaught TypeError: x.config.modeBarButtonsToRemove.push is not a function
    at Object.renderValue (plotly.js:155)
    at exports.OutputBinding.shinyBinding.renderValue (htmlwidgets.js:516)
    at exports.OutputBinding.onValueChange (output_binding.js:16)
    at exports.OutputBinding.delegator.(:7541/anonymous function) [as onValueChange] (http://127.0.0.1:7541/htmlwidgets-1.3/htmlwidgets.js:112:23)
    at OutputBindingAdapter.onValueChange (output_binding_adapter.js:21)
    at ShinyApp.receiveOutput (shinyapp.js:332)
    at ShinyApp.<anonymous> (shinyapp.js:544)
    at ShinyApp._sendMessagesToHandlers (shinyapp.js:529)
    at ShinyApp.dispatchMessage (shinyapp.js:515)
    at WebSocket.c.onmessage (shinyapp.js:112)
renderValue @ plotly.js:155
shinyBinding.renderValue @ htmlwidgets.js:516
onValueChange @ output_binding.js:16
delegator.(anonymous function) @ htmlwidgets.js:112
onValueChange @ output_binding_adapter.js:21
receiveOutput @ shinyapp.js:332
(anonymous) @ shinyapp.js:544
_sendMessagesToHandlers @ shinyapp.js:529
dispatchMessage @ shinyapp.js:515
c.onmessage @ shinyapp.js:112

Adding a second, even irrelevant value, fixes the error.

# Remove 2 buttons
plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = c("hoverCompareCartesian","zoom2d"))

# zoom3d would not appear by default because this is a 2d plot, but adding it to the argument prevents the error
plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
  config(modeBarButtonsToRemove = c("hoverCompareCartesian","zoom3d"))

I am using plotly 4.8.0

cpsievert commented 5 years ago

It works when you provide a list:

plot_ly(data = mtcars, x = ~mpg, y = ~wt) %>%
    config(modeBarButtonsToRemove = list("hoverCompareCartesian"))

I suppose I could wrap modeBarButtonsToRemove in I() internally though to make this easier to use

wkdavis commented 5 years ago

That makes sense, especially considering that all of the examples in the documentation use the list() verbiage. Perhaps I've come to rely too heavily on the interchangeability of list() and c() in R functions. That said, it might be a convenient feature to wrap it with I() if that doesn't impact anything else.

cpsievert commented 5 years ago

Now that the plotly.js exports a schema for the config, there's a possibility of converting to arrays automatically, but not until https://github.com/plotly/plotly.js/issues/3731 is addressed