Repository navigation
fix(microscopy): show the ROIs again when toggling their visibility - #6303
Conversation
toggleROIsVisibility read this.showROIs without calling it, so the toggleAnnotations command could hide the ROIs but never show them again, while the isROIsVisible flag still flipped. CodeQL reports the line as js/useless-expression. The extension had no Jest config, so the new test comes with one, the same as the dicom-pdf extension. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe microscopy ROI visibility toggle now calls ChangesMicroscopy ROI visibility
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to ROI visibility can be restored after hiding, and the regression test is included in the configured Jest workflow. No actionable merge risk was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI runs `pnpm run test:unit:ci` in each package, and the microscopy extension had no such script, so its tests never ran. Add the same test:unit scripts that the cornerstone extension uses. Remove formatDICOMDate.test.js: it only tested a re-export of the @ohif/ui-next helper, which ui-next already tests, and importing the whole ui-next barrel fails under the extension's Babel transform. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Context
MicroscopyService.toggleROIsVisibilityhides the ROIs on the first call, but never shows them again:The second branch reads the method and does not call it. The flag still flips, so after two toggles the service says the ROIs are visible while every viewer keeps them hidden. The
toggleAnnotationscommand of the microscopy extension is the only caller.CodeQL reports this line as
js/useless-expression("This expression has no effect").Changes & Results
extensions/dicom-microscopy/src/services/MicroscopyService.ts: callthis.showROIs().extensions/dicom-microscopy/src/services/MicroscopyService.test.ts: new test. The first toggle callshideROIson the managed viewer and setsisROIsVisibletofalse. The second toggle callsshowROIsand sets it back totrue.extensions/dicom-microscopy/jest.config.js: new, the same asextensions/dicom-pdf/jest.config.js. The extension had no Jest config, so the rootprojectsglob did not pick up its tests.Before: after a hide,
toggleAnnotationscannot show the ROIs again.After:
toggleAnnotationsalternates between hide and show.Verified locally:
master:showROIsis called 0 times on the second toggle.this.isROIsVisible = !this.isROIsVisible) makes both tests fail, onisROIsVisibleand on thehideROIscall count.javascript-code-scanningandjavascript-code-qualitysuites) on this branch againstmaster: thejs/useless-expressionalert is gone, and there is no new alert.pnpm run test:unit:ci,pnpm run lint:compiler:ciandpnpm run compiler:coverage:cipass. Prettier passes on the changed files.Testing
cd extensions/dicom-microscopy && pnpm exec jest src/services/MicroscopyService.test.tstoggleAnnotationscommand twice (for example from a hotkey bound to it). The ROIs hide, then show again.Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals. (No public API change.)
Tested Environment
Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work.
🤖 Generated with Claude Code
Summary by CodeRabbit