Viewers
Viewers copied to clipboard
Added an extension for downloading an entire study
related issue: https://github.com/OHIF/Viewers/issues/2430#issue-907754649
- User can trigger the download of a complete study, via toolbar button
- User gets detailed status information in a modal view
-
- Individual steps (downloading, zipping, saving)
-
- a pull request for the dicomweb-client client has been issued. With this version the user sees also the progress of the download in terms of downloaded bytes (see https://github.com/dcmjs-org/dicomweb-client/pull/44#issue-676285054).
- [x] Brief description of changes
- [x] Links to any relevant issues
- [x] Required status checks are passing
- [x] User cases if changes impact the user's experience
- [ ]
@mentiona maintainer to request a review
Codecov Report
Merging #2454 (065fde3) into master (fe57c00) will decrease coverage by
0.18%. The diff coverage is0.49%.
:exclamation: Current head 065fde3 differs from pull request most recent head e058beb. Consider uploading reports for the commit e058beb to get more accurate results
@@ Coverage Diff @@
## master #2454 +/- ##
==========================================
- Coverage 12.68% 12.50% -0.19%
==========================================
Files 306 307 +1
Lines 8233 8360 +127
Branches 1593 1654 +61
==========================================
+ Hits 1044 1045 +1
- Misses 5797 5862 +65
- Partials 1392 1453 +61
| Impacted Files | Coverage Δ | |
|---|---|---|
| platform/core/src/classes/StudyLoadingListener.js | 1.02% <0.00%> (ø) |
|
| ...latform/core/src/classes/metadata/StudyMetadata.js | 1.28% <0.00%> (-0.02%) |
:arrow_down: |
| ...m/core/src/utils/loadAndCacheDerivedDisplaySets.js | 0.00% <0.00%> (ø) |
|
| ...r/src/connectedComponents/ConnectedStudyBrowser.js | 0.00% <0.00%> (ø) |
|
| platform/viewer/src/connectedComponents/Viewer.js | 0.00% <0.00%> (ø) |
|
| ...tform/viewer/src/connectedComponents/ViewerMain.js | 0.00% <0.00%> (ø) |
|
| ...rm/viewer/src/googleCloud/DicomStorePickerModal.js | 0.00% <ø> (ø) |
|
| platform/viewer/src/index.js | 0.00% <ø> (ø) |
|
| platform/core/src/utils/naturalizeSOPClassUID.js | 0.74% <0.74%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update c59d664...e058beb. Read the comment docs.
Hey @Christian-e-S, great job here :) 🎖️
So, I just quickly tested the feature and it seems to be working fine. We have some of this logic in another extension (debugging) but I think it its currently broken, maybe we are better off accepting this separate extension and dumping the old download logic from debugging (what do you think @swederik?)
I have some comments regarding the modal though:

- I think toolbar button should not be enabled in master by default (@pieper @swederik do we have liability concerns?)
- When clicking to open the modal, it downloads right away but I think we should add a 'download' button there to avoid clicking the button in the toolbar by mistake and starting a huge downloading request in the background
- The size was not updated after downloading
- There's no visible loading indicator, I thought it was stuck with a blank status: ...
- You can leverage the notification service to display notifications e.g. 'Study downloaded successfully' or something
Extras
- What about adding options in the modal to download display set (series) based or instance-based (we can tackle this in another branch)
- Another extra case that is really not important right now (just thought about it): Adding an option to disable and enable a save button in the viewport overlay so users can download the active viewport
Hey @igoroctaviano!
Thank you very much for your review!
Please find my comment below:
Hey @Christian-e-S, great job here :) 🎖️
So, I just quickly tested the feature and it seems to be working fine. We have some of this logic in another extension (debugging) but I think it its currently broken, maybe we are better off accepting this separate extension and dumping the old download logic from debugging (what do you think @swederik?)
I have some comments regarding the modal though:
- I think toolbar button should not be enabled in master by default (@pieper @swederik do we have liability concerns?)
I agree, this makes absolutely sense!
- When clicking to open the modal, it downloads right away but I think we should add a 'download' button there to avoid clicking the button in the toolbar by mistake and starting a huge downloading request in the background
We had it like this, but users complained about the additional click. In some use cases, users use the online viewer just for previewing images, and then download the series to be able to use the specific features of the tool that they are used to, so downloading becomes their major use case and they want to have it efficient.
- The size was not updated after downloading
This works only with a recent commit of the dicomweb-client https://github.com/dcmjs-org/dicomweb-client/commit/c292726a421cd84e6a34aec4e72fbbad3c87621c that was created from my pull request to the dicomweb-client repository. This is required to get progress information in terms of bytes downloaded.
Should we wait until this gets into the next release of dicomweb-client?

- There's no visible loading indicator, I thought it was stuck with a blank status: ...
See comment above. With the latest master branch of the dicomweb-client you can see the transferred bytes progressing, during the download, which gives a good indication:


- You can leverage the notification service to display notifications e.g. 'Study downloaded successfully' or something
Agreed
Extras
- What about adding options in the modal to download display set (series) based or instance-based (we can tackle this in another branch)
- Another extra case that is really not important right now (just thought about it): Adding an option to disable and enable a save button in the viewport overlay so users can download the active viewport
I like these ideas.
@igoroctaviano what is the status on this?
@sedghi codewise LGTM but we need to check if we want to merge it (I know that there are some implications to having an easy download button in a viewer).
base has changed, read more here https://github.com/OHIF/Viewers/issues/3477
@Christian-e-S , do you have this extension to v3? rgds and great job.
@Christian-e-S , do you have this extension to v3? rgds and great job.
Did you ever try this with v3? If so, how did you go about it? I don't even know how to get started with adding the extension.
@threehappypenguins No we haven't done it in v3