Skip to content

fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR - #6157

Merged
sedghi merged 1 commit into
masterfrom
fix/worklist-search-and-mpr-crosshairs
Jul 17, 2026
Merged

fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR#6157
sedghi merged 1 commit into
masterfrom
fix/worklist-search-and-mpr-crosshairs

Conversation

@sedghi

@sedghi sedghi commented Jul 17, 2026

Copy link
Copy Markdown
Member

Fixes #6152.

Problem

In the segmentation, usAnnotation, and preclinical-4d modes, the MPR Crosshairs tool was configured with no mouse binding and disableOnPassive: true. Without a binding it activates on plain Primary, so activating another Primary tool in the same mpr tool group (brush, zoom, pan) makes setToolActive evict Crosshairs and — because of disableOnPassive — fully disable it. The two tools therefore can't be active at the same time. Measurement tools were unaffected only because they aren't members of the mpr group.

Fix

Bind Crosshairs to Primary + Shift (matching the longitudinal/tmtv modes) so it lives on its own mouse binding and stays active alongside brush/zoom/pan. Three mode initToolGroups files, +34 lines.

Testing

Open a study in segmentation mode → MPR → enable Crosshairs → activate the brush (then zoom, then pan): Crosshairs stays active (Shift+drag to move it), and toggling between them no longer disables the other.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@sedghi
sedghi temporarily deployed to fork-pr-approval July 17, 2026 15:02 — with GitHub Actions Inactive
@netlify

netlify Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 782cb3a
🔍 Latest deploy log https://app.netlify.com/projects/ohif-dev/deploys/6a5a4fb6abef61000851ada2
😎 Deploy Preview https://deploy-preview-6157--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Crosshairs tool configurations in preclinical 4D, segmentation MPR, and ultrasound annotation MPR tool groups now require Primary mouse input with Shift.

Changes

Crosshairs bindings

Layer / File(s) Summary
Shift-modified Crosshairs activation
modes/preclinical-4d/src/initToolGroups.tsx, modes/segmentation/src/initToolGroups.ts, modes/usAnnotation/src/initToolGroups.js
Crosshairs bindings are explicitly configured for the Primary mouse button with the Shift modifier, with comments documenting mutual exclusion from plain-Primary tools.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the problem, fix, and testing, but it misses the template's Context/Changes & Results/Checklist sections and tested environment details. Add the missing template sections, especially a checked checklist and tested OS/Node/browser details, and rename the headings to match the repository template.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: keeping Crosshairs active with brush/zoom/pan in MPR, and follows the fix(scope) style.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/worklist-search-and-mpr-crosshairs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@platform/app/src/routes/WorkList/WorkList.tsx`:
- Around line 147-155: Update the comment above manualFiltering in the WorkList
table configuration to state that the table must not re-filter client-side
because filtering is performed upstream. Keep the existing
manualFiltering={true} behavior unchanged and correct only the contradictory
wording.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1caed471-be01-4752-938c-b50dd58da449

📥 Commits

Reviewing files that changed from the base of the PR and between df390a2 and 61f4df7.

📒 Files selected for processing (4)
  • modes/preclinical-4d/src/initToolGroups.tsx
  • modes/segmentation/src/initToolGroups.ts
  • modes/usAnnotation/src/initToolGroups.js
  • platform/app/src/routes/WorkList/WorkList.tsx

Comment thread platform/app/src/routes/WorkList/WorkList.tsx Outdated
@cypress

cypress Bot commented Jul 17, 2026

Copy link
Copy Markdown

Viewers    Run #6569

Run Properties:  status check passed Passed #6569  •  git commit 782cb3a5ba: fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR
Project Viewers
Branch Review fix/worklist-search-and-mpr-crosshairs
Run status status check passed Passed #6569
Run duration 01m 46s
Commit git commit 782cb3a5ba: fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR
Committer Alireza
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 28
View all changes introduced in this branch ↗︎

In the segmentation, usAnnotation and preclinical-4d modes the MPR Crosshairs
tool was configured with no mouse binding and disableOnPassive: true. Without a
binding it activates on plain Primary, so the moment another Primary tool in the
same `mpr` group (brush, zoom, pan) is activated from the toolbar, the
setToolActive handler evicts Crosshairs and — because of disableOnPassive —
fully disables it, making them mutually exclusive. Measurement tools were
unaffected only because they are not members of the `mpr` group.

Bind Crosshairs to Primary+Shift (matching the longitudinal/tmtv modes) so it
lives on its own binding and stays active alongside brush/zoom/pan.

Fixes #6152
@sedghi
sedghi force-pushed the fix/worklist-search-and-mpr-crosshairs branch from 61f4df7 to 782cb3a Compare July 17, 2026 15:52
@sedghi
sedghi temporarily deployed to fork-pr-approval July 17, 2026 15:52 — with GitHub Actions Inactive
@sedghi sedghi changed the title fix: study-list patient search and MPR crosshairs coexistence (#6151, #6152) fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR (#6152) Jul 17, 2026
@sedghi sedghi changed the title fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR (#6152) fix(crosshairs): keep Crosshairs active alongside brush/zoom/pan in MPR Jul 17, 2026
@sedghi

sedghi commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

@jbocce i made it crosshair only

@jbocce

jbocce commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

I just tried the deploy preview and this is still broken.

ScreenHunter.Jul.17.18.01.mp4

@sedghi
sedghi merged commit 7c176d3 into master Jul 17, 2026
12 checks passed
@sedghi
sedghi deleted the fix/worklist-search-and-mpr-crosshairs branch July 17, 2026 16:46
dchansen06 pushed a commit to iCRcompany/OHIFViewer that referenced this pull request Jul 27, 2026
…PR (OHIF#6157)

In the segmentation, usAnnotation and preclinical-4d modes the MPR Crosshairs
tool was configured with no mouse binding and disableOnPassive: true. Without a
binding it activates on plain Primary, so the moment another Primary tool in the
same `mpr` group (brush, zoom, pan) is activated from the toolbar, the
setToolActive handler evicts Crosshairs and — because of disableOnPassive —
fully disables it, making them mutually exclusive. Measurement tools were
unaffected only because they are not members of the `mpr` group.

Bind Crosshairs to Primary+Shift (matching the longitudinal/tmtv modes) so it
lives on its own binding and stays active alongside brush/zoom/pan.

Fixes OHIF#6152
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Crosshairs tool does not stay on while segmentation/zoom/pan tool is active

2 participants