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

Fix plotly dependencies #7

Closed
wants to merge 1 commit into from
Closed

Fix plotly dependencies #7

wants to merge 1 commit into from

Conversation

alanocallaghan
Copy link
Contributor

Address #6

@AliciaSchep
Copy link
Contributor

Thanks @alanocallaghan for noticing and fixing this issue!

One issue I am noticing though with new plotly + this patch is that in an rmarkdown only the first plot made by iheatmapr shows up in the html. However, if a regular plotly plot is added as well, then all the plots show up...

@alanocallaghan
Copy link
Contributor Author

If you post an example I'll look into it

@AliciaSchep
Copy link
Contributor

The vignette is an example -- adding plotly::plot_ly(x =1:5,y=1:5) in a chunk anywhere in the document seems to make the problem go away

@alanocallaghan
Copy link
Contributor Author

Bizarre! A workaround would be to put that in a block starting {r, echo=FALSE} but that's not a great fix

@AliciaSchep
Copy link
Contributor

AliciaSchep commented May 31, 2017

It seems like adding in the additional dependencies that plotly r package uses, in particular crosstalk::crosstalkLibs() is necessary for multiple iheatmapr plots to show up in rmarkdown. Adding a plotly plot was a fix because the html would then load those libraries when making the plot.

@AliciaSchep AliciaSchep mentioned this pull request Jun 1, 2017
13 tasks
@AliciaSchep
Copy link
Contributor

Hi @alanocallaghan, I decided to address the problem a different way, by removing the dependency on plotly r package in order to have more control over things like the version of plotly.js and other aspects of making the htmlwidget (as outlined in #7). I'm going to close this pull request and the corresponding issue. Thanks again for detecting this problem and suggesting a fix!

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 this pull request may close these issues.

2 participants