Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Troubles with the use of dygraphs #356

Closed
odelaigue opened this issue Nov 7, 2019 · 6 comments · Fixed by #357
Closed

Troubles with the use of dygraphs #356

odelaigue opened this issue Nov 7, 2019 · 6 comments · Fixed by #357

Comments

@odelaigue
Copy link

With htmlwidgets 1.5.1, there are some big troubles with some functions of the dygraphs package (e.g. dyStackedBarGroup or dyShadow).
Its is OK with the version 1.3.
I don't know if you should fix htmlwidgets or @pshevtsov should adapt dygraphs to version 1.5.1.

library(dygraphs)
library(magrittr)

## dataset
lungDeaths <- cbind(fdeaths, mdeaths, ldeaths, foo = fdeaths/2, bar = fdeaths/3)

## A bunch of different plotters together:
dygraph(lungDeaths) %>%
  dyRangeSelector() %>%
  dyBarSeries('bar') %>% 
  dyStemSeries('mdeaths') %>% 
  dyShadow('foo') %>% 
  dyFilledLine('fdeaths')

## Stacked Bar and Ribbon Graphs:  
dygraph(lungDeaths) %>% 
  dySeries('mdeaths', axis = 'y2') %>%
  dyAxis('y', valueRange = c(-100, 1000)) %>% 
  dyStackedBarGroup(c('ldeaths', 'fdeaths'))
@wch
Copy link
Contributor

wch commented Nov 7, 2019

For the record, here's what I see when I run the first one:

image

@wch
Copy link
Contributor

wch commented Nov 7, 2019

I've bisected this to c998fae

cpsievert added a commit to cpsievert/htmlwidgets that referenced this issue Nov 7, 2019
ramnathv#356.

The problem with evaluating without parentheses first is that named function declarations are valid expressions that return undefined (whereas evaluating with parentheses returns the function, as expected).

It's true there may be weird corner-cases where switching this order leads to something else that's undesirable, but we've been operating with the 'with parentheses' model for so long that switching the ordering now will minimize backward-compatibility issues
@cpsievert
Copy link
Collaborator

Here's a potential fix for this issue remotes::install_github("ramnathv/htmlwidgets#177")

jcheng5 pushed a commit that referenced this issue Nov 8, 2019
#356. (#357)

The problem with evaluating without parentheses first is that named function declarations are valid expressions that return undefined (whereas evaluating with parentheses returns the function, as expected).

It's true there may be weird corner-cases where switching this order leads to something else that's undesirable, but we've been operating with the 'with parentheses' model for so long that switching the ordering now will minimize backward-compatibility issues
@odelaigue
Copy link
Author

Thank you very much for your responsiveness!

@odelaigue
Copy link
Author

Do you think you will soon be submitting the latest version of htmlwidgets on the CRAN?
Currently, packages using dygraphs still present potential troubles with htmlwidgets version 1.5.1.

@timelyportfolio
Copy link
Collaborator

Thanks for all the work on this, but it seems rstudio/dygraphs#237 is still not working even after the fix. I'll dig a little when I have some time.

schloerke added a commit to schloerke/htmlwidgets that referenced this issue Sep 30, 2020
* upstream/master:
  Bump dev version
  Export JSEvals (ramnathv#381)
  Add missing roxygenize step (ramnathv#370)
  Add reportTheme arg to shinyWidgetOutput() (ramnathv#361)
  Try evaluating code with parentheses before without parentheses, closes ramnathv#356. (ramnathv#357)
  Bump version for development
  v1.5.1 release candidate (ramnathv#353)
  Add a comment; abstract out duplication in logic (ramnathv#354)
  Prevent staticRender from being called unconditionally when htmlwidgets.js is loaded after page load
  Bump version; update NEWS
  Use  to schedule staticRender() iff jQuery 3 or higher is present in shinyMode
  Bump version
  Bump version for release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants