Skip to content

print-time html dependencies should use relative instead of absolute paths #1384

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

Merged
merged 4 commits into from
Nov 27, 2018

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Oct 31, 2018

Partially fixes #1376, but crosstalk::crosstalkLibs() needs to adopt these same changes for those dependencies to be portable as well (see rstudio/crosstalk#63). Note that, unfortunately, since partial_bundle() downloads the relevant plotly.js bundle at print-time, I don't think there is a fool-proof way to make that portable, but one could do partial_bundle(p, local = FALSE) in that case if they really need the R object to be portable.

DEVELOPMENT NOTES

With these changes, file paths are now resolved in the htmltools namespace, rather than htmlwidgets, so the usual hack to get pkgload::load_all() to work will have to be extended/revised to shim in the appropriate namespace -- r-lib/pkgload#69

TESTING NOTES

After installing plotly and crosstalk, make sure the following code will produce the image below

# devtools::install_github("ropensci/plotly#1384")
# devtools::install_github("rstudio/crosstalk#63")

# This should download and render a R plotly object that I created with this PR and uploaded to my dropbox
readRDS(url("https://www.dropbox.com/s/4trwd7dvm73pb7v/plotly-test.rds?dl=1"))

screen shot 2018-11-06 at 11 25 46 am

@cpsievert cpsievert force-pushed the relative-dependency-paths branch from fd101af to 9f2a56d Compare October 31, 2018 19:50
@cpsievert cpsievert requested a review from jcheng5 October 31, 2018 20:01
@cpsievert cpsievert force-pushed the relative-dependency-paths branch 2 times, most recently from 25cafd1 to dbce3c0 Compare November 5, 2018 22:07
@cpsievert cpsievert force-pushed the relative-dependency-paths branch from 5bb1124 to c488c5e Compare November 6, 2018 16:10
Copy link
Contributor

@jcheng5 jcheng5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to indicate htmltools >=0.3.6 in DESCRIPTION (we just made this mistake in Shiny v1.2). Otherwise, LGTM.

@cpsievert cpsievert merged commit 1e8f9b9 into master Nov 27, 2018
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.

Reading an R plotly object saved as RDS across different systems
2 participants