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

export both plotly plot and legend images #1126

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

moontrip
Copy link
Contributor

@moontrip moontrip commented Jul 3, 2024

Hi @dmfalke This is an attempt to possibly address #946, especially based on the last/latest comment, #946 (comment).

I have tried many ways, even trying to use html-to-image, which is a forked version of the dom-to-image and more updated library, but realized that dom-to-image was already installed and worked just fine for our purpose so I sticked to it. There were several issues but I could manage to find ways. From my various tests, this seems to work as intended, but just in case, I made this as a draft PR as there may be a better way 🤔

@moontrip moontrip requested a review from dmfalke July 3, 2024 01:37
@dmfalke
Copy link
Member

dmfalke commented Jul 10, 2024

Thank you for working on this, @moontrip. I think this approach seems reasonable, but it will require some UX discussion. Also, we don't want to hard code DOM element IDs, as it creates an implicit coupling between the legend implementation and the ExportPlotToImageButton component.

I noticed that the associated issue is still in "Needs triage". I'm not sure this is considered urgent, at the moment. However, I think this PR serves as a good proof-of-concept. Perhaps a future iteration would be to pass an array of objects (maybe specifying DOM refs and file names) to ExportPlotToImageButton, which would be used to generate the image files. And, instead of rendering ExportPlotToImageButton in the plot components, it could be rendered in the viz components.

@moontrip
Copy link
Contributor Author

@dmfalke Thank you for your review and comments 👍 Yes as this PR is draft, I just wanted to test if it would work, without considering it too much: in part, I wanted to find something to do😅 Not quite sure if we will ever do it but yes it can be served as a proof-of-concept at least 😄

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