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

Feature: add orientation markers tool #1089

Merged
merged 17 commits into from
Oct 7, 2019
Merged

Conversation

frolic06
Copy link
Contributor

  • 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, ...)
    Feature

  • What is the current behavior? (You can also link to an open issue here)
    No

  • What is the new behavior (if this is a feature change)?
    It adds a tool called OrientationMarkersTool, which displays orientation markers like in version 2.X

orientation markers

Some code:
const orientationMarkers = cornerstoneTools['OrientationMarkersTool'];
cornerstoneTools.addToolForElement(element, orientationMarkers);
cornerstoneTools.setToolActiveForElement(this.element, 'OrientationMarkers', {});

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

  • Other information:

@codecov
Copy link

codecov bot commented Sep 30, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@fc4e1ff). Click here to learn what that means.
The diff coverage is 27.27%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1089   +/-   ##
=========================================
  Coverage          ?   17.83%           
=========================================
  Files             ?      274           
  Lines             ?     8428           
  Branches          ?     1420           
=========================================
  Hits              ?     1503           
  Misses            ?     5759           
  Partials          ?     1166
Impacted Files Coverage Δ
src/stackTools/stackPrefetch.js 5.67% <ø> (ø)
src/tools/OrientationMarkersTool.js 27.27% <27.27%> (ø)

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 fc4e1ff...8df6d62. Read the comment docs.

@dannyrb dannyrb self-requested a review September 30, 2019 15:33
@dannyrb
Copy link
Member

dannyrb commented Sep 30, 2019

@frolic06 is this ready for review?

@frolic06
Copy link
Contributor Author

Yes

@frolic06
Copy link
Contributor Author

frolic06 commented Oct 1, 2019

Hi @dannyrb ,
there is an issue related to this PR: #967

@frolic06
Copy link
Contributor Author

frolic06 commented Oct 3, 2019

@dannyrb did you have to review this PR ?
Can I help you in some way ?

Copy link
Member

@dannyrb dannyrb left a comment

Choose a reason for hiding this comment

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

  1. Thanks for creating an example page
  2. The new tool's implementation looks good 👍
  3. A couple of small comments that need to be addressed before merge.
  4. It would be nice if you could add this to our netlify example as an "overlay" tool, and give the example images any meta they may need to render orientations markers.

src/index.js Outdated Show resolved Hide resolved
src/stackTools/stackPrefetch.js Show resolved Hide resolved
src/tools/OrientationMarkersTool.test.js Outdated Show resolved Hide resolved
@dannyrb
Copy link
Member

dannyrb commented Oct 7, 2019

@dannyrb
Copy link
Member

dannyrb commented Oct 7, 2019

Thanks for your hard work on this, @frolic06! ^_^

@dannyrb dannyrb merged commit 3f5e46c into cornerstonejs:master Oct 7, 2019
@dannyrb
Copy link
Member

dannyrb commented Oct 7, 2019

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants