Skip to content
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

feat: add download study dialog by adding ohif-idc-extension #67

Merged
merged 6 commits into from
Sep 25, 2024

Conversation

pedrokohler
Copy link
Collaborator

@pedrokohler pedrokohler commented Aug 30, 2024

Context

Solves #62

Changes & Results

image
image

Testing

Checklist

PR

  • [] My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • [] My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • [] The documentation page has been updated as necessary for any public API
    additions or removals.

Tested Environment

  • [] OS:
  • [] Node version:
  • [] Browser:

Copy link
Collaborator

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking good! You will need to add/install the extension to the package.json of the app folder since the link is just virtual (local). Here:

"@ohif/extension-cornerstone-dicom-sr": "3.9.0-beta.74",

Also, since we are not using a publish npm package you need to point to the other repo's main branch e.g.

"ohif-gcp-extension": "https://github.com/ImagingDataCommons/ohif-gcp-extension#main",

@pedrokohler
Copy link
Collaborator Author

It's looking good! You will need to add/install the extension to the package.json of the app folder since the link is just virtual (local). Here:

"@ohif/extension-cornerstone-dicom-sr": "3.9.0-beta.74",

Also, since we are not using a publish npm package you need to point to the other repo's main branch e.g.

"ohif-gcp-extension": "https://github.com/ImagingDataCommons/ohif-gcp-extension#main",

@igoroctaviano I believe it's good to go now. Please check.

@fedorov
Copy link
Member

fedorov commented Sep 12, 2024

Related to OHIF#4367

@fedorov fedorov changed the title feat: add download study dialog feat: add download study dialog by adding ohif-idc-extension Sep 12, 2024
platform/app/.webpack/webpack.pwa.js Outdated Show resolved Hide resolved
platform/app/package.json Outdated Show resolved Hide resolved
@igoroctaviano igoroctaviano self-requested a review September 16, 2024 14:27
Copy link
Collaborator

@igoroctaviano igoroctaviano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@igoroctaviano
Copy link
Collaborator

@pedrokohler lets wait Andrey to test it before merge.

@fedorov
Copy link
Member

fedorov commented Sep 24, 2024

Can you please include the updated screenshots, after the PR above is merged, since I expect the UI is different than in the original screenshot at the top of this thread?

Also, for the popup text, instead of "install the idc cli", can you please put "install the idc-index python package"?

@pedrokohler
Copy link
Collaborator Author

Can you please include the updated screenshots, after the PR above is merged, since I expect the UI is different than in the original screenshot at the top of this thread?

Also, for the popup text, instead of "install the idc cli", can you please put "install the idc-index python package"?

@fedorov:

image
image

The changes above were pushed.

@fedorov
Copy link
Member

fedorov commented Sep 25, 2024

LGTM Pedro! If you can, could you hyperlink idc-index? This way the user can easily learn more about the package before installing it. If hyperlinking is not straightforward, it's fine with me to merge as is!

@pedrokohler pedrokohler merged commit b848ed4 into master Sep 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants