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 bug when export html, chart plotly not show #79

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

quangduong109
Copy link

Because the DOM has not finished loading, the require plotly function fails, in fact the require function is an async function, so it cannot be assigned to the Plotly variable.
The fix is to replace the text in the returned plot function to pass the Plotly variable into the render chart function.
Before fix:
image
image

@chazkii
Copy link
Collaborator

chazkii commented Oct 29, 2021

Hi @quangduong109 thanks for the work and PR! Before we can merge, can you please:

  • Remove all changes except the one in pybloqs/block/image.py - we will do a release ASAP with your doc fix
  • Squash commits with new message "bugfix - get plotly export to html working with Block"

@quangduong109
Copy link
Author

Hi @quangduong109 thanks for the work and PR! Before we can merge, can you please:

  • Remove all changes except the one in pybloqs/block/image.py - we will do a release ASAP with your doc fix
  • Squash commits with new message "bugfix - get plotly export to html working with Block"

I have updated. Please check and accept this pull request. Thank you ^^

@chazkii
Copy link
Collaborator

chazkii commented Oct 31, 2021

Hi @quangduong109, thanks, though you haven't squashed your commits - there should only be 1 commit to be merged. Though more importantly, I've just checked out your branch and plotly appears to still be broken, as seen when generating the docs via python setup.py build_sphinx. If you could please demonstrate how your change will fix an existing bug, then we can consider merging.
image

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