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

Address SliceThickness.toFixed TypeError #3510

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

mccle
Copy link
Contributor

@mccle mccle commented Jun 30, 2023

Context

Fixes Issue 3465. This error resulted from the SliceThickness variable in extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx having a string value and being unable to call toFixed.

Changes & Results

I added parseFloat to a couple of lines in extensions/cornerstone-dicom-seg/src/viewports/OHIFCornerstoneSEGViewport.tsx and it seems to have fixed the problem I was having before and renders segmentations. These are my changes (starting at line 346):

...
            thickness: SliceThickness ? `${parseFloat(SliceThickness).toFixed(2)}mm` : '',
            spacing:
              SpacingBetweenSlices !== undefined
                ? `${parseFloat(SpacingBetweenSlices).toFixed(2)}mm`
                : '',
...

Before adding these changes, I was faced with a 'Something Went Wrong' warning screen as depicted in Issue 3465. After applying them, I was successfully able to view segmentations, as shown here:

Screenshot 2023-06-29 at 3 16 24 PM

Testing

To test my changes, use OHIF in combination with a DCM4CHEE archive. Select a SEG/MR study from the main page and launch the basic viewer. Click on the segmentation to open it. It should now be visible even in cases where SliceThickness has a string value.

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: Mac OS X 10.15
  • Node version: 16.15.0
  • Browser: Firefox 114.0

@netlify
Copy link

netlify bot commented Jun 30, 2023

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 7cdf6bb
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/649e28c3aa3da00008202429
😎 Deploy Preview https://deploy-preview-3510--ohif-dev.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jun 30, 2023

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit 7cdf6bb
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/649e28c3d96c120008cbfe44
😎 Deploy Preview https://deploy-preview-3510--ohif-platform-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #3510 (7cdf6bb) into master (14974b6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3510   +/-   ##
=======================================
  Coverage   42.81%   42.81%           
=======================================
  Files          82       82           
  Lines        1448     1448           
  Branches      338      338           
=======================================
  Hits          620      620           
  Misses        665      665           
  Partials      163      163           

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 f7a435c...7cdf6bb. Read the comment docs.

@sedghi
Copy link
Member

sedghi commented Jul 3, 2023

Thanks a lot!

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.

2 participants