-
Notifications
You must be signed in to change notification settings - Fork 126
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
Feature: Mermaid pan & zoom #295
base: master
Are you sure you want to change the base?
Conversation
Hey @mjbvz, can I get a review on this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! Sorry it took me so long to test it
The panning and zooming seems to work well. However to start I think it should be presented differently. Let's do:
-
Add a new vscode configuration setting that lets you enable/disable pan zoom entirely. It can default to on, but there needs to be a way to turn the feature off entirely
-
Only show the pan/zoom controls when you hover over the diagram
-
Potentially add a command or right click option on the diagram that overrides the VS Code configuration. So if I have pan zoom enabled in my settings, I'd like the command to be able to turn it off in case it's causing any issues
A few other points I noticed:
-
The colors of the controls seem off, especially on dark themes. If you can, reuse some of the vscode theme colors. These should all be available inside of the markdown preview webview
-
The diagram get cut off. I'd try to get it match what GitHub does instead where the diagram is centered and takes up more horizontal space
I will take a more detailed look once some of those points are addressed. Let me know if you have any questions about these
Hey @mjbvz , thanks for getting back to me! I just slightly confused and want to clarify a few things:
or these controls after pan/zoom is enabled
|
Sure, following up:
|
This PR adds pan & zoom functionality in mermaid MD preview. There's a previous PR that attempted to add this functionality but had some issues. This PR tries address the issues mentioned in the comments.
#125
The changes include:
throw error
when a diagram fails to parse. From what I can see when mermaid library fails to parse the contents, we would catch the error and return a error message content. But for some reason it makes another throw error, so when you have multiple diagrams and one breaks it breaks everything else.after remove
throw error