WIP feat: expose hanging protocol stage to default #3300
+41
−28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for ohif-platform-viewer ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for ohif-platform-docs canceled.
|
wayfarer3130
reviewed
Apr 4, 2023
| public run( | ||
| { studies, displaySets, activeStudy }, | ||
| protocolId, | ||
| { stageIndex } |
There was a problem hiding this comment.
Can you pass all the options here rather than extracting stageIndex? There are reasons it might be desirable to pass initial display conditions to the run method.
wayfarer3130
reviewed
Apr 4, 2023
| @@ -75,7 +79,8 @@ function defaultRouteInit( | |||
| // hanging protocol in the mode configuration | |||
| hangingProtocolService.run( | |||
| { studies, activeStudy, displaySets }, | |||
| hangingProtocolId | |||
| hangingProtocolId, | |||
| { stageIndex: hangingProtocolStageIndex || 0 } | |||
There was a problem hiding this comment.
This should not pass in 0 if it isn't specified in the URL. There is matching rules to decide the initial protocol stage, and specifically passing it in will disable some of those.
wayfarer3130
reviewed
Apr 4, 2023
| studyInstanceUIDs, | ||
| dataSource, | ||
| filters, | ||
| hangingProtocol: hangingProtocolIdToUse, |
There was a problem hiding this comment.
This needs to start getting passed in as a set of options to the route init, as there may well be others that we want here.
|
@Ouwen Also for this one, any one free you have to work on this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
This PR is meant to expose the stage index as an initial setting from the mode. The final implementation needs some discussion before it gets merged.
{}with named key inputs it is a mix of Object and positional argsChanges & Results
This PR tries to expose stage selection or stage matching to the user.
@sedghi