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

Add offline Write function #25

Closed

Conversation

LeviHarrison
Copy link

First off, I'd just like to thank you for all the work you've done on this project. I just started using it and it's been really useful.

In the project I'm using go-plotly for, I wanted to be able to serve the figure with my own HTTP server in a route handler, so I added a function to write the figure HTML to a writer. I thought I might open a PR in case this was useful in the upstream.

Signed-off-by: Levi Harrison <git@leviharrison.dev>
@MetalBlueberry
Copy link
Owner

MetalBlueberry commented Jun 25, 2024

First of all, Thank you very much for your interest in the project and the contribution ❤️

I understand why this function may be useful, but I believe it is not worth including it on the offline package. This package is meant to give an easy interface to display an image locally or just save it to a file you can easily share as a single offline webpage.

Given you are trying to register this as a handler, it makes me think you are attempting to create some kind of with multiple figures along other functonality. This approach will serve a complete webpage on a single handler, which is not ideal on this scenario.

A proper implementation will require the handlers to just serve json data for the figure, which can be obtained by just converting the figure to json. Then you will need custom javascript code to properly plot that information on the frontend.

So with that argument, I don't think we should merge this.

Can you explain your usecase? just in case I'm missing something. I'm happy to discuss options.

@MetalBlueberry
Copy link
Owner

Closing now to keep the repository clean, feel free to reopen

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