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

Extract the render-to-string portion of to_html() into its own public method #14

Merged
merged 1 commit into from Jun 28, 2020

Conversation

Xymist
Copy link
Contributor

@Xymist Xymist commented Jun 4, 2020

Add a .to_inline_html() method to Plot which can then be sent elsewhere for rendering.

For example, when using in Evcxr this turns this construction:

let plotly_file = "/tmp/plot.html";
plot.to_html(plotly_file);
let plotly_contents = fs::read_to_string(plotly_file).unwrap();
println!("EVCXR_BEGIN_CONTENT text/html\n{}\nEVCXR_END_CONTENT", plotly_contents);
fs::remove_file(plotly_file)?;

into this one-liner:

println!("EVCXR_BEGIN_CONTENT text/html\n{}\nEVCXR_END_CONTENT", plot.to_inline_html());

I debated adding a helper method for that wrapping - I'm happy to do so but I feel it would be presumptive without an OK from @igiagkiozis since they might not wish to officially support such usage, and interested parties probably are using DARN anyway which is a better place for it.

Closes #12 as long as people are happy to do their own root div or Evcxr content wrapping.

@shahinrostami
Copy link

@Xymist I just found this whilst looking to introduce some improvements for DARN... I want to make some changes to reduce notebook sizes (sometimes 6-7MB) to 10-100KB. I'm torn between forking igiagkiozis/plotly and making these improvements directly or introducing them as a post-processing step in DARN.

@igiagkiozis
Copy link
Collaborator

Hi @Xymist, please re-direct the pull request to the dev branch.

@Xymist Xymist changed the base branch from master to dev June 23, 2020 15:08
@Xymist
Copy link
Contributor Author

Xymist commented Jun 23, 2020

@igiagkiozis - Done, thank you.

@igiagkiozis igiagkiozis merged commit ad74b00 into plotly:dev Jun 28, 2020
@igiagkiozis
Copy link
Collaborator

@Xymist I just found this whilst looking to introduce some improvements for DARN... I want to make some changes to reduce notebook sizes (sometimes 6-7MB) to 10-100KB. I'm torn between forking igiagkiozis/plotly and making these improvements directly or introducing them as a post-processing step in DARN.

@shahinrostami the size is due to every plot being self contained, which means that plotly.js is copied in every single plot. This is quite inefficient for this use case. I'm currently looking into alternative ways to better support this use case.

@igiagkiozis
Copy link
Collaborator

@Xymist, @shahinrostami I've pushed an implementation that works with inline html nicely without the extra js dependencies. You can try it out in the latest dev branch; this feature will be released in version 0.5.xx

@Xymist Xymist deleted the allow_use_in_evcxr branch July 3, 2020 09:23
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.

Expose inline html as a String
3 participants