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

fix: Toggle different study #3370

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

wayfarer3130
Copy link
Contributor

Context

Drag and drop of a different study to the MPR mode, and then turning off the MPR mode should still work and preserve the old hanging protocol correctly even if the active study has changed.

Changes & Results

It is probably difficult to see this change in the current version of OHIF because once a study is active in terms of the hanging protocol, there isn't an option to change which study is active.

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:

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for ohif-platform-viewer canceled.

Name Link
🔨 Latest commit eb7cc42
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-viewer/deploys/645557545dfebf0008be1893

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for ohif-platform-docs canceled.

Name Link
🔨 Latest commit 0b75512
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/651d880df37d46000863ebe5

// Just use the protocol id as the toggle id for toggling
// as the stage shouldn't matter, and one wants to restore even across
// active studies.
const toggleID = protocolId;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is now possible to toggle on a hanging protcool with multiple stages, and then it won't toggle off until a different hanging protocol is applied or the toggle is directly called. That allows for toggling on a mode such as MPR, and having commands which either apply a given stage or toggle the stage on if it is off, and those same commands will toggle the HP off.

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #3370 (0b75512) into master (8a335bd) will decrease coverage by 0.29%.
Report is 21 commits behind head on master.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3370      +/-   ##
==========================================
- Coverage   46.23%   45.95%   -0.29%     
==========================================
  Files          78       78              
  Lines        1276     1284       +8     
  Branches      312      314       +2     
==========================================
  Hits          590      590              
- Misses        548      554       +6     
- Partials      138      140       +2     
Files Coverage Δ
...ils/metadataProvider/getPixelSpacingInformation.js 0.00% <0.00%> (ø)
platform/core/src/log.js 14.28% <0.00%> (-85.72%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a755cce...0b75512. Read the comment docs.

@cypress
Copy link

cypress bot commented May 5, 2023

Passing run #3609 ↗︎

0 39 0 0 Flakiness 0

Details:

Merge branch 'master' of github.com:OHIF/Viewers into pr/fix/toggle-different-st...
Project: Viewers Commit: 0b755126e4
Status: Passed Duration: 05:46 💡
Started: Oct 4, 2023 3:54 PM Ended: Oct 4, 2023 3:59 PM

Review all test suite changes for PR #3370 ↗︎

@wayfarer3130 wayfarer3130 requested a review from jbocce June 9, 2023 15:49
@wayfarer3130 wayfarer3130 changed the base branch from v3-stable to master July 31, 2023 19:32
@netlify
Copy link

netlify bot commented Jul 31, 2023

Deploy Preview for ohif-dev canceled.

Name Link
🔨 Latest commit 0b75512
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/651d880dcfb30f0008bd1f47

@wayfarer3130 wayfarer3130 requested a review from sedghi July 31, 2023 19:34
@sedghi
Copy link
Member

sedghi commented Aug 1, 2023

@wayfarer3130 my PR which reduces the complexity of viewportGrid should be reviewed/merged first before this since that is the way to go forward

@sedghi
Copy link
Member

sedghi commented Sep 18, 2023

I can't reproduce what you mentioned in the Context in the PR description, it shows the first DS as you wanted

CleanShot.2023-09-18.at.11.04.29.mp4

@sedghi sedghi removed the request for review from jbocce October 4, 2023 14:47
@sedghi
Copy link
Member

sedghi commented Oct 4, 2023

I gave it another shot, but couldn't replicate the problem. So, I'll hold off on merging this for now until we can reproduce it step by step.

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.

None yet

2 participants