fix(microscopy): Fix measurement visibility toggle #5725
Conversation
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Please update your branch with master and push the result. Thanks. |
|
I did some testing and each of the various measurements can be hidden and displayed. I did notice #5722 which is a separate issue but continues to exist. It will/should be handled in a separate PR. |
| * Toggle visibility of a specific ROI | ||
| * @param uid The UID of the ROI to toggle | ||
| */ | ||
| toggleROIVisibility(uid) { |
There was a problem hiding this comment.
We are clobbering toggleROIVisibility that is on line 396. If we want to replace the old one, then this code should replace above. However I don't think that is what we want to do because the one on line 396 is toggling ALL the visibility of ALL ROIs. So this one should be renamed. Please come up with an appropriate name and/or consider asking AI for a name - it is good with that. 😊
There was a problem hiding this comment.
Apologies. I see the name difference now. But they are still too similar in my opinion.
| * @param {boolean} isVisible - Whether the ROI should be visible or hidden | ||
| */ | ||
| toggleROIVisibility(uid, roiGraphic, isVisible) { | ||
| if (isVisible) { |
There was a problem hiding this comment.
In my opinion, adding and removing the ROI is NOT the same as hiding and showing it - eventhough it could be argued that is one way to implement visibility. Furthermore, if we look at hideROIs and showROIs they are NOT doing that to hide and show the ROIs. I looked further and unfortunately the underlying DICOM microscopy viewer does NOT appear to have an API for showing and hiding individual annotations. We may need to look into this further.
| toggleAnnotations: () => { | ||
| microscopyService.toggleROIsVisibility(); | ||
| }, | ||
| toggleMeasurementVisibility: ({ uid }) => { |
There was a problem hiding this comment.
I think this name should be consistent with using the annotations instead of measurements.
jbocce
left a comment
jbocce
left a comment
There was a problem hiding this comment.
Thanks for this PR. You have a lot of the fundamentals needed for adding the visibility toggle. 👍 Your proposal for toggling the visibility using add and remove needs to be revisited. It is likely best to add methods in the DICOM micrsoscopy viewer API (https://github.com/ImagingDataCommons/dicom-microscopy-viewer/blob/master/src/viewer.js) to accomplish this.
There was a problem hiding this comment.
I know this was existing prior to your changes, but line 397 has an error. It should be this.showROIs() (i.e. an actual method invocation). We should update this.
2 participants
Context
Fixes #5659
This PR fixes the inability to hide/show individual measurements in microscopy mode using the eye icon in the measurements panel.
It Implements visibility toggle functionality in the dicom-microscopy extension
Changes
isVisibleproperty andsetVisibility()method to track annotation visibility statetoggleROIVisibility()method to coordinate visibility changes across all managedviewers
toggleROIVisibility()method to add/remove ROI from the dicom-microscopy-viewertoggleMeasurementVisibilitycommandResults
Before
The measurement stays visible when the user clicks on the eye icon.
After
The measurement can be hidden and shown when the user clicks on the eye icon.
Screen.Recording.2026-01-18.at.7.52.46.PM.mov
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment