Fix exporting plots to SVG #4835
Conversation
| @@ -91,7 +106,7 @@ export const ZoomedInPlot: React.FC<ZoomedInPlotProps> = ({ | |||
| actions: { | |||
| compiled: false, | |||
| editor: false, | |||
| export: true, | |||
| export: false, | |||
There was a problem hiding this comment.
[F] As per this thread these actions don't work. We might be able to drop this altogether.
There was a problem hiding this comment.
need to keep the attribute or the actions do not show up
| const actions: HTMLDivElement | null | undefined = | ||
| zoomedInPlotRef.current?.querySelector('.vega-actions') | ||
| if (!actions) { | ||
| return | ||
| } | ||
|
|
||
| appendActionToVega('SVG', actions, () => { | ||
| void view.toSVG().then(svg => { |
There was a problem hiding this comment.
[Q] @julieg18 do you remember if there is a way to close the menu on click?
There was a problem hiding this comment.
Looks like ;(vegaActions.parentNode as HTMLElement).removeAttribute('open') works
There was a problem hiding this comment.
;(vegaActions.parentNode as HTMLElement).removeAttribute('open') works
Yes, we adjust the same attribute when we're opening the menu automatically for the ZoomedInPlot.
mattseddon
force-pushed
the
fix-export-plot
branch
from
ee03efa to
36c68a5
Compare
| ThemeProperty.FONT | ||
| ]) | ||
|
|
||
| const fullThemedSvg = preventSvgTruncation(themedSvg) |
There was a problem hiding this comment.
[F] Without this we end up with SVGs that look like this:
I have spent several hours looking for alternatives including things like https://github.com/tsayen/dom-to-image.
IMO this is good enough for now.
| @@ -433,6 +438,25 @@ export class WebviewMessages { | |||
| writeFile(file.path, plotId) | |||
| } | |||
|
|
|||
| private async exportPlotAsSvg(svg: string) { | |||
| const file = await showSaveDialog( | |||
| join(this.dvcRoot, 'visualization.svg'), | |||
There was a problem hiding this comment.
[F] I lost a couple of SVGs because they were saved into a folder that VS Code couldn't open. Personally, I think this is a better default.
There was a problem hiding this comment.
Do we want to give the file a unique name (the title of the plot maybe)?
There was a problem hiding this comment.
I recreated exactly what vega does but I can update.
mattseddon
force-pushed
the
fix-export-plot
branch
from
0ed309a to
8720c26
Compare
| @@ -433,6 +438,25 @@ export class WebviewMessages { | |||
| writeFile(file.path, plotId) | |||
| } | |||
|
|
|||
| private async exportPlotAsSvg(svg: string) { | |||
| const file = await showSaveDialog( | |||
| join(this.dvcRoot, 'visualization.svg'), | |||
There was a problem hiding this comment.
Do we want to give the file a unique name (the title of the plot maybe)?
mattseddon
force-pushed
the
fix-export-plot
branch
from
8720c26 to
d95340a
Compare
|
Code Climate has analyzed commit d95340a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 97.8% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.0% (0.0% change). View more on Code Climate. |
1/2
main<- this <- #4849This PR fixes exporting plots to SVGs. It also removes exporting plots to PNGs as this currently does not work.
PTAL, I've left comments inline.
Demo
https://github.com/iterative/vscode-dvc/assets/37993418/1ff4c704-5f42-41d2-985e-e6841c4014feScreen.Recording.2023-10-17.at.3.08.36.pm.mov