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

[Unitary Hack] Save Python qsharp-widgets RE output to PNG #1604

Merged
merged 15 commits into from
Jun 14, 2024

Conversation

nirajvenkat
Copy link
Contributor

@nirajvenkat nirajvenkat commented Jun 1, 2024

Submission for Unitary Hack issue: fixes #1063

This change adds the ability to save the resource estimates diagram locally as a PNG. For this I added a new dependency called html2canvas that takes a DOM and renders it to an image. This currently works only on IPython qsharp-widgets, could be extended to the VSCode version.

Additionally we take into account the color theme within VSCode (which is provided in the stylesheet qsharp-ux.css) and this is passed in as the background color for rendering. Tested on (dark/light) Modern VSCode themes.

uh.mp4

Current issues:

@nirajvenkat nirajvenkat changed the title [Unitary Hack] Initial draft: save to PNG [Unitary Hack] Save qsharp-widgets RE output to PNG Jun 1, 2024
@nirajvenkat nirajvenkat changed the title [Unitary Hack] Save qsharp-widgets RE output to PNG [Unitary Hack] Save Python qsharp-widgets RE output to PNG Jun 1, 2024
@nirajvenkat
Copy link
Contributor Author

@microsoft-github-policy-service agree

@nirajvenkat nirajvenkat marked this pull request as ready for review June 1, 2024 14:31
@billti
Copy link
Member

billti commented Jun 1, 2024

We are very conservative in this project on taking new dependencies. I see you added one called 'html2canvas'. Looking at that project I can see it has over 900 open issues, hasn't had a commit for 2 years, and the README states

"The script is still in a very experimental state, so I don't recommend using it in a production environment nor start building applications with it yet, as there will be still major changes made".

Based on the above, I'm hesitant to take a change which adds this dependency.

@nirajvenkat
Copy link
Contributor Author

Would it be ok to add a different dependency? This one for example, is more active and has less outstanding issues.

Also the way I have been doing development is to first run the watcher using npm run dev, make some changes, after which I switch to a jupyter notebook (also in VSCode) and must reload the kernel to get the newest version built by the watcher, then re-run the cell. Is there any way to attach a debugger or to view print logs?

@billti
Copy link
Member

billti commented Jun 2, 2024

We use SVG a lot, as well as dynamic DOM nodes (via Preact) for our widgets, so issues such as bubkoo/html-to-image#425, bubkoo/html-to-image#463, and bubkoo/html-to-image#392 could come into play (I've been optimizing some SVG sizes via def/use lately, so likely to be even more of an issue going forward).

Sadly again, the project seems unmaintained. The last release was 18 months ago, and all issues since just have an automated response and no follow up (including this one 🤣 bubkoo/html-to-image#455)

@nirajvenkat
Copy link
Contributor Author

Also the way I have been doing development is to first run the watcher using npm run dev, make some changes, after which I switch to a jupyter notebook (also in VSCode) and must reload the kernel to get the newest version built by the watcher, then re-run the cell. Is there any way to attach a debugger or to view print logs?

@minestarks @ivanbasov Do you have any pointers on a faster way to work on qsharp-widgets?

@minestarks
Copy link
Member

minestarks commented Jun 3, 2024

@nirajvenkat quickest dev loop I can think of is:

  • In the widgets/ directory, run pip install . This will recompile the widgets.
  • In the notebook, restart the kernel.
  • Rerun the cell.

You can leave the notebook open as you rebuild - you just have to restart the kernel.

As for debugging the JavaScript, I haven't done this myself so I don't know how well it works, but try the "Developer: Open Webview Developer Tools" command in VS Code. It brings up the Chrome developer tools. Then you can use the familiar Chrome Developer Tools (element inspector etc) and the JavaScript debugger on the widget code.

@nirajvenkat
Copy link
Contributor Author

@minestarks Thanks, I totally forgot about the browser Dev Tools, looks like it has what I was looking for.

@billti I am working on bringing in the same functionality from the above sources, but just the pieces relevant to this task. So we should not require any external dependencies.

@nirajvenkat
Copy link
Contributor Author

Fixed prior issues, ready for review.

Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

Thank you!! This works great functionally but I have some style / UI requests.

npm/qsharp/ux/qsharp-ux.css Outdated Show resolved Hide resolved
npm/qsharp/ux/estimatesOverview.tsx Show resolved Hide resolved
npm/qsharp/ux/estimatesOverview.tsx Outdated Show resolved Hide resolved
npm/qsharp/ux/saveImageUtil.tsx Show resolved Hide resolved
@nirajvenkat
Copy link
Contributor Author

@minestarks Not sure if I can finish up in time for UH submission, so to speed thing up I have proposed commit 6ef794f. This would be saving to PNG, and a simpler API for the UI side that takes in the React RefObject and optionally a filename and does the rest.

@minestarks
Copy link
Member

We should be good on the hackathon deadline. This is so close, but it doesn't work in VS Code for me. Probably because VS Code webview doesn't support window.open? Feel free to take a stab at implementing proper VS Code extension support (possibly using showSaveDialog ?) but that may be tricky and not required for this PR.

In the meantime please disable it for VS Code (consider taking this commit to pass an allowSaveImage flag ) .

Once VS Code is good to go and the other comments are resolved I'll merge this. We're OK on the deadline.

nirajvenkat and others added 2 commits June 13, 2024 06:26
Removes unnecessary exports, and we continue to use createRef because of casting troubles
@nirajvenkat
Copy link
Contributor Author

@minestarks I was thinking that based on the below setting, it should have been okay for you to commit directly to this branch. Not sure what the convention is.
image
In any case this merge was possible on my end. Tested it out by building VSIX package. The button does not show up at all, is this expected?

@minestarks
Copy link
Member

@minestarks I was thinking that based on the below setting, it should have been okay for you to commit directly to this branch. Not sure what the convention is.

@nirajvenkat sorry, it just didn't occur to me that I could push to your branch. Glad you were able to pull the changes regardless

Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

Please consider the useRef suggestion if you can make it work - otherwise looks good. Thanks for the contribution!

@nirajvenkat
Copy link
Contributor Author

I am investigating an issue with this branch where the UI is flickering when trying to interact with the points on the graph. Specifically the tooltip that appears when you hover over points keeps flickering. I have a hunch on what it may be but will continue to investigate tomorrow.

Replace 12px that was accidentally removed in prior commit
@minestarks minestarks added this pull request to the merge queue Jun 14, 2024
Merged via the queue into microsoft:main with commit 7985568 Jun 14, 2024
16 checks passed
@nirajvenkat nirajvenkat deleted the unitaryhack branch June 16, 2024 13:24
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.

Add ability to save the Resource Estimator space-time diagram as a file (png/jpg/svg)
3 participants