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: OverlayTool now considers overlay initial position #1177

Merged
merged 1 commit into from
Feb 13, 2020

Conversation

fexman
Copy link
Contributor

@fexman fexman commented Feb 13, 2020

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    The initial position for DICOM overlays is now considered in OverlayTool.

  • What is the current behavior? (You can also link to an open issue here)
    The origin of DICOM overlays from the overlayPlaneModule was fixed to 0,0.

  • What is the new behavior (if this is a feature change)?

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No

  • Other information:

@fexman fexman requested a review from dannyrb February 13, 2020 13:51
@dannyrb dannyrb merged commit 7db55a6 into cornerstonejs:master Feb 13, 2020
@dannyrb
Copy link
Member

dannyrb commented Feb 13, 2020

Thanks for the PR @fexman
Quick check, do we need to guard against overlay.x and overlay.y being NaN/undefined/null?

@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #1177 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1177   +/-   ##
=======================================
  Coverage   18.86%   18.86%           
=======================================
  Files         284      284           
  Lines        8498     8498           
  Branches     1446     1446           
=======================================
  Hits         1603     1603           
  Misses       5715     5715           
  Partials     1180     1180
Impacted Files Coverage Δ
src/tools/OverlayTool.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

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

@fexman
Copy link
Contributor Author

fexman commented Feb 13, 2020

Maybe. Should I create another PR and add this?

@dannyrb
Copy link
Member

dannyrb commented Feb 13, 2020

🎉 This PR is included in version 4.12.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dannyrb
Copy link
Member

dannyrb commented Feb 13, 2020

Updated here: 4e430ba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants