graph: fix PNG download button #4759
Conversation
psybuzz
force-pushed
the
pz-g-png
branch
2 times, most recently
from
037149a
to
bdf139a
Compare
| @@ -199,6 +201,11 @@ class TfGraphApp extends LegacyElementMixin(PolymerElement) { | |||
| } | |||
| } | |||
| _fit() { | |||
| (this.$$('#graphboard') as any).fit(); | |||
| ((this.$$('#graphboard') as unknown) as TfGraphBoard).fit(); | |||
There was a problem hiding this comment.
Why do we need to cast through unknowns here? It works fine for me
locally if casting directly to TfGraphBoard.
There was a problem hiding this comment.
Ah true, it was actually unnecessary. Even with as TfGraphBoard, the internal sync seems to not like this, so I've set it back to as any for now.
On that note, I still haven't figured out how to trigger the internal check in OSS, as simply adding compiler option declaration: true doesn't do the trick.
| const element = document.createElement('a'); | ||
| element.href = URL.createObjectURL(blob); | ||
| element.download = filename; | ||
| element.click(); |
There was a problem hiding this comment.
I’ve tested this and it seems to work for me, but I’m a bit surprised.
I expected a synthetic e.click() event to not trigger a download.
Do you know what I’m misunderstanding?
There was a problem hiding this comment.
My understanding is that el.click(), while synthetic, still triggers the browser's "default behavior", as well as any 'click' listeners registered on the element. I'm not sure that really answers the root of your question, I looked briefly at the spec but didn't see anything definitive. Similar to download, calling .click on other types of elements may trigger native behaviors such as page navigation, opening the browser file selector dialog, submitting a form, etc.
| @@ -1131,6 +1130,8 @@ export class TfGraphControls extends LegacyElementMixin(PolymerElement) { | |||
| }) | |||
| _legendOpened: boolean = true; | |||
|
|
|||
| private _downloadFilename = ''; | |||
There was a problem hiding this comment.
Should this default to 'graph.png' for consistency with the setter?
|
Added a test sync cl to the description: cl/362151624 Ended up needing to go back to Thanks for the review! |
The "Download as PNG" button in the Graph dashboard was broken
during the Polymer 2 -> 3 migration. Before,
<tf-graph-minimap>relied on assumptions that
<tf-graph-controls>would provide an<a>tag for downloading in the DOM. This is broken functionallybecause the element is not accessible from another shadow subtree.
This fixes the download button by properly plumbing a 'click' event
handler from the controls to trigger code in the minimap.
Manually tested that clicking the button properly opens or saves
a PNG to the filesystem using Chrome and Firefox.
Googlers, see test sync cl/362151624
Fixes #3714
For reviewers, the component tree structure looks like: