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

feat(imageIdSpecificStateManager): add set imageIdToolState function #1341

Merged
merged 4 commits into from
Jul 30, 2021

Conversation

mix3d
Copy link
Contributor

@mix3d mix3d commented Dec 16, 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, ...)
    A function to replace the tool-specific tool state in the imageId toolstate global manager.

  • What is the current behavior? (You can also link to an open issue here)
    There is currently no function to consume the same output from getImageIdToolState(), (an object like {'data':[...]}) which is the state of a specific tool for the given imageId. addElementToolState() requires a single item input,
    and without a replace function, requires both a clearImageIdToolState() step and an iteration over all items to add into the tool's id-specific state, or restoring the ENTIRE imageId tool state manager with data from ALL tools.

  • What is the new behavior (if this is a feature change)?
    New function on the globalImageIdToolStateManager object.

  • 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 Dec 16, 2020

Codecov Report

Merging #1341 (8d114b3) into master (fae1bed) will increase coverage by 0.05%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1341      +/-   ##
==========================================
+ Coverage   19.53%   19.58%   +0.05%     
==========================================
  Files         286      286              
  Lines       10120    10128       +8     
  Branches     2065     2066       +1     
==========================================
+ Hits         1977     1984       +7     
- Misses       6930     6931       +1     
  Partials     1213     1213              
Impacted Files Coverage Δ
src/stateManagement/imageIdSpecificStateManager.js 51.85% <87.50%> (+6.19%) ⬆️

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 fae1bed...8d114b3. Read the comment docs.

…ctions

There is currently no function to consume the same output from getImageIdToolState, which is the
state of a specific tool for the given imageId. addElementToolState requires a single item input,
and without replace, requires both a clear step and an iteration over all items to add into the
tool's id-specific state.
@mix3d mix3d force-pushed the feature/replaceToolState branch from cd05ac4 to 3650e05 Compare December 16, 2020 22:08
Copy link
Collaborator

@kevinleedrum kevinleedrum left a comment

Choose a reason for hiding this comment

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

This seems perfectly in line with the existing methods, and I'm sure there are plenty of other teams who could benefit from this.

@mix3d
Copy link
Contributor Author

mix3d commented Jan 6, 2021

@dannyrb ?

@swederik
Copy link
Member

swederik commented Jan 8, 2021

Can we call it set instead of replace? Otherwise seems fine to me

@mix3d mix3d changed the title feat(imageIdSpecificStateManager): add replace imageIdToolState function feat(imageIdSpecificStateManager): add set imageIdToolState function Jul 29, 2021
@mix3d
Copy link
Contributor Author

mix3d commented Jul 29, 2021

@swederik renamed functions. Once the test pass I'll merge it.

@swederik swederik merged commit 112149e into cornerstonejs:master Jul 30, 2021
@dannyrb
Copy link
Member

dannyrb commented Jul 30, 2021

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mix3d mix3d deleted the feature/replaceToolState branch August 2, 2021 17:31
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.

4 participants