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: allow saving to same SR series #4889

Merged
merged 6 commits into from
Apr 7, 2025

Conversation

pedrokohler
Copy link
Collaborator

@pedrokohler pedrokohler commented Mar 25, 2025

Context

Currently, users are unable to save to an existing SR series — each save operation creates a new series. This pull request introduces functionality that allows users to save a new instance to an existing SR series.

The behavior remains consistent with the current loading mechanism: the most recent instance will be the one displayed, as there is still no implemented method for selecting which instance to load. This change simply enables the ability to add additional instances to the same SR series, which mirrors the behavior seen when multiple instances already exist.

Changes & Results

Before:
Can only create new series
image

After:
Can still create new series
image

But can also save to a previsouly existing SR series
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:

feat: improve series labels

feat: improve series labels

feat: add series date and time to new SR instances

refactor: extract dicom date time

feat: generalize nextSRSeriesNumber

chore: refactor dicom date time
Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for ohif-platform-docs failed. Why did it fail? →

Name Link
🔨 Latest commit 3ae6d4a
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/67f04718e8f19d0008a88e85

Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit 3ae6d4a
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/67f0471862908d00082621f4

@sedghi
Copy link
Member

sedghi commented Mar 25, 2025

Bill had a PR for allowing SR to save to the same series, can you see if we need that too? it was long ago

@pedrokohler
Copy link
Collaborator Author

Bill had a PR for allowing SR to save to the same series, can you see if we need that too? it was long ago

@sedghi The other PR has been closed and won't be used. Bill asked me to open this new one.

@wayfarer3130
Copy link
Contributor

Bill had a PR for allowing SR to save to the same series, can you see if we need that too? it was long ago

@sedghi The other PR has been closed and won't be used. Bill asked me to open this new one.

This new PR is based on the old one, but uses the new ui-next dialog setup, and doesn't do anything weird with loading/display annotations.

There is still the ability to load previous versions, but that is an extension of this one. This menu will only appear when saving to a study that already has SRs, and it will default to the old behaviour.

@wayfarer3130 wayfarer3130 requested a review from sedghi March 25, 2025 16:39
@sedghi
Copy link
Member

sedghi commented Mar 27, 2025

@dan-rukas Can you please design this?

Criteria:
When saving a measurement the user has two options.

  1. Whether to create a completely new series and name it and hit save
  2. Whether to choose an already existing series (from a dropdown) and CAN"T rename it (only name is shown) and save

@dan-rukas
Copy link
Member

Here's a quick mock-up to add some labels over the options. "Report Name" can still keep focus when the dialog comes up to allow for quick typing.

Screenshot 2025-04-04 at 2 05 44 PM

@pedrokohler
Copy link
Collaborator Author

Style changes done:

image

@sedghi sedghi merged commit 62e5a62 into OHIF:master Apr 7, 2025
15 of 17 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.

4 participants