-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ Add component for downloading the static report #1312
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #1312 +/- ##
==========================================
+ Coverage 43.04% 43.07% +0.02%
==========================================
Files 145 145
Lines 4330 4325 -5
Branches 999 998 -1
==========================================
- Hits 1864 1863 -1
+ Misses 2454 2450 -4
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
cd65479
to
e946639
Compare
12027e0
to
86b3404
Compare
c87c7c0
to
bb4ca97
Compare
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.
Looking good, and I'm fine with merging as-is, but I wonder if it's worth trying this tweak
const a = document.createElement("a"); | ||
a.style.display = "none"; | ||
a.href = url; | ||
a.download = `analysis-${application.name}.` + mimeType; | ||
|
||
document.body.appendChild(a); | ||
a.click(); |
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.
Oh wow, this is very clever but I'm surprised it's necessary. I think we've used the file-saver library in the past to achieve the same thing, but tbh this might be more elegant.
I wonder if you even need to do the fetch in JS... could we just have the href of this anchor element be the URL we're fetching and let the click event do all the work? If that works that would mean the browser handles the whole download and can show progress natively instead of having it appear as a delayed instant-download.
bb4ca97
to
fab4e47
Compare
590c394
to
b50ce0a
Compare
Signed-off-by: ibolton336 <ibolton@redhat.com>
b50ce0a
to
c75a470
Compare
<a>
element to trigger the download with a localized loading state.