Skip to content

feat(ui-next): Adds toggle state for ToolButton and Crosshair example #5914

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.

Sign up for GitHub

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

Open
dan-rukas wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat(ui-next): Adds toggle state for ToolButton and Crosshair example #5914

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
487d9d2
Adds toolbutton toggle state for crosshairs
dan-rukas Mar 23, 2026
cbf93a9
Added correct disabled text
dan-rukas Mar 23, 2026
66fe808
Added some comment details
dan-rukas Mar 23, 2026
882ed3b
Rename toggleCrosshairsToolbar to togglePassiveDisabledToolbar for co…
sedghi Mar 24, 2026
9dd75d4
Implement mouse shortcut customization for crosshairs tool, allowing …
sedghi Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions extensions/cornerstone/src/commandsModule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,28 @@ function commandsModule({
}
}
},
toggleCrosshairsToolbar({ value, itemId, toolGroupIds = ['mpr'] }) {
const toolName = itemId || value;

toolGroupIds.forEach(toolGroupId => {
const toolGroup = toolGroupService.getToolGroup(toolGroupId);
if (!toolGroup || !toolGroup.hasTool(toolName)) {
return;
}

const currentMode = toolGroup.getToolOptions(toolName).mode;
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled;
Comment on lines +1054 to +1055
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P1 Active mode excluded from isOn check (same as evaluator)

This mirrors the omission in the evaluator: if Crosshairs is somehow in Active mode (keyboard shortcut or another command), isOn will be false, and clicking the toggle button will call setToolPassive instead of setToolDisabled, leaving crosshairs active without feedback.

The same fix applies here:

Suggested change
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled;
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled ||
currentMode === Enums.ToolModes.Active;

Copy link
Copy Markdown
Member Author

@dan-rukas dan-rukas Mar 23, 2026

Choose a reason for hiding this comment

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

Same reasoning as above. Active mode is out of scope for this PR and will be handled in follow-up work.


if (isOn) {
// On → turn OFF
toolGroup.setToolDisabled(toolName);
} else {
// Off → turn ON (Passive — visible + repositionable via handles)
toolGroup.setToolPassive(toolName);
}
});
},
setToolActiveToolbar: ({ value, itemId, toolName, toolGroupIds = [], bindings }) => {
// Sometimes it is passed as value (tools with options), sometimes as itemId (toolbar buttons)
toolName = toolName || itemId || value;
Expand Down Expand Up @@ -2639,6 +2661,9 @@ function commandsModule({
toggleActiveDisabledToolbar: {
commandFn: actions.toggleActiveDisabledToolbar,
},
toggleCrosshairsToolbar: {
commandFn: actions.toggleCrosshairsToolbar,
},
updateStoredPositionPresentation: {
commandFn: actions.updateStoredPositionPresentation,
},
Expand Down
25 changes: 25 additions & 0 deletions extensions/cornerstone/src/getToolbarModule.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,31 @@ export default function getToolbarModule({ servicesManager, extensionManager }:
};
},
},
{
name: 'evaluate.cornerstoneTool.crosshairToggle',
evaluate: ({ viewportId, button, disabledText }) => {
const toolGroup = toolGroupService.getToolGroupForViewport(viewportId);
if (!toolGroup) {
return;
}

const toolName = toolbarService.getToolNameForButton(button);
if (!toolGroup.hasTool(toolName)) {
return getDisabledState(disabledText);
}

const currentMode = toolGroup.getToolOptions(toolName).mode;
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled;
Comment on lines +453 to +454
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P1 Active mode excluded from isOn check

The isOn detection uses Passive || Enabled but omits Active. If Crosshairs is activated through an alternative path (e.g., a keyboard binding or another command that calls setToolActive), the button will appear un-toggled even though the tool is visually active in the viewports. The corresponding command logic (line 1054–1055 in commandsModule.ts) shares the same omission, so clicking the button when Crosshairs is already Active would try to enable Passive mode again instead of disabling it, leaving the crosshairs on.

For consistency with toggleActiveDisabledToolbar, consider including Active:

Suggested change
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled;
const isOn = currentMode === Enums.ToolModes.Passive ||
currentMode === Enums.ToolModes.Enabled ||
currentMode === Enums.ToolModes.Active;

Copy link
Copy Markdown
Member Author

@dan-rukas dan-rukas Mar 23, 2026

Choose a reason for hiding this comment

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

The omission of Active is intentional.

See the PR message above - this implementation scopes the toolbar button to a pure on/off toggle (Passive ↔ Disabled). The crosshair tool is never set to Active through the toolbar. Active mode will be handled separately at the viewport level in @sedghi's upcoming work via a modifier key interaction. The toolbar button intentionally stays in its toggled appearance during that interaction. Any changes to support Active mode would be part of that follow-up work.


return {
disabled: false,
isActive: false,
isToggled: isOn,
icon: isOn ? 'tool-crosshair-checked' : 'tool-crosshair',
};
},
Comment on lines +436 to +477
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2 Duplicated isOn detection logic

The isOn evaluation (Passive || Enabled) is copy-pasted identically into both evaluate.cornerstoneTool.crosshairToggle (here) and toggleCrosshairsToolbar in commandsModule.ts. When one changes (e.g., adding Active to the check), the other must be updated manually.

Consider extracting this into a shared helper, or at minimum leaving a comment cross-referencing the two sites so they stay in sync.

Copy link
Copy Markdown
Member Author

@dan-rukas dan-rukas Mar 23, 2026

Choose a reason for hiding this comment

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

The two checks should stay in sync. I added cross-reference comments in both locations explaining that they must agree on which modes count as "on," and noting that both need updating if Active mode support is added.

},
{
name: 'evaluate.action',
evaluate: () => {
Expand Down
2 changes: 1 addition & 1 deletion modes/basic/src/initToolGroups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ function initMPRToolGroup(extensionManager, toolGroupService, commandsManager) {
xOffset: 0.95,
yOffset: 0.05,
},
disableOnPassive: true,
disableOnPassive: false,
autoPan: {
enabled: false,
panSize: 10,
Expand Down
5 changes: 3 additions & 2 deletions modes/basic/src/toolbarButtons.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,14 +647,15 @@ const toolbarButtons: Button[] = [
type: 'tool',
icon: 'tool-crosshair',
label: i18n.t('Buttons:Crosshairs'),
tooltip: i18n.t('Buttons:Click to toggle on or off'),
commands: {
commandName: 'setToolActiveToolbar',
commandName: 'toggleCrosshairsToolbar',
commandOptions: {
toolGroupIds: ['mpr'],
},
},
evaluate: {
name: 'evaluate.cornerstoneTool',
name: 'evaluate.cornerstoneTool.crosshairToggle',
disabledText: i18n.t('Buttons:Select an MPR viewport to enable this tool'),
},
},
Expand Down
3 changes: 3 additions & 0 deletions platform/ui-next/src/components/Icons/Icons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ import {
ToolCobbAngle,
ToolCreateThreshold,
ToolCrosshair,
ToolCrosshairChecked,
ToolDicomTagBrowser,
ToolFlipHorizontal,
ToolFreehandPolygon,
Expand Down Expand Up @@ -436,6 +437,7 @@ export const Icons = {
ToolCobbAngle,
ToolCreateThreshold,
ToolCrosshair,
ToolCrosshairChecked,
ToolDicomTagBrowser,
ToolFlipHorizontal,
ToolFreehandPolygon,
Expand Down Expand Up @@ -703,6 +705,7 @@ export const Icons = {
'tool-cobb-angle': (props: IconProps) => ToolCobbAngle(props),
'tool-create-threshold': (props: IconProps) => ToolCreateThreshold(props),
'tool-crosshair': (props: IconProps) => ToolCrosshair(props),
'tool-crosshair-checked': (props: IconProps) => ToolCrosshairChecked(props),
'dicom-tag-browser': (props: IconProps) => ToolDicomTagBrowser(props),
'tool-flip-horizontal': (props: IconProps) => ToolFlipHorizontal(props),
'tool-freehand-polygon': (props: IconProps) => ToolFreehandPolygon(props),
Expand Down
62 changes: 62 additions & 0 deletions platform/ui-next/src/components/Icons/Sources/Tools.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,68 @@ export const ToolCrosshair = (props: IconProps) => (
</svg>
);

export const ToolCrosshairChecked = (props: IconProps) => (
<svg
width="28"
height="28"
viewBox="0 0 28 28"
fill="none"
xmlns="http://www.w3.org/2000/svg"
{...props}
>
<g clipPath="url(#clip0_tool_crosshair_checked)">
<path
d="M14 3V9"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M3 14H9"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M14 25V19"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M25 14H19"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M14.001 16.001C15.1056 16.001 16.001 15.1056 16.001 14.001C16.001 12.8964 15.1056 12.001 14.001 12.001C12.8964 12.001 12.001 12.8964 12.001 14.001C12.001 15.1056 12.8964 16.001 14.001 16.001Z"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
/>
<path
d="M22 17C24.7614 17 27 19.2386 27 22C27 24.7614 24.7614 27 22 27C19.2386 27 17 24.7614 17 22C17 19.2386 19.2386 17 22 17ZM24.7998 19.7998C24.5789 19.6344 24.2652 19.6796 24.0996 19.9004L21.4453 23.4385L19.8535 21.8467C19.6583 21.6514 19.3417 21.6514 19.1465 21.8467C18.9513 22.0419 18.9513 22.3585 19.1465 22.5537L21.1465 24.5537C21.2489 24.6561 21.3907 24.7094 21.5352 24.6992C21.6797 24.6889 21.8134 24.6159 21.9004 24.5L24.9004 20.5C25.0658 20.2791 25.0205 19.9654 24.7998 19.7998Z"
fill="currentColor"
/>
</g>
<defs>
<clipPath id="clip0_tool_crosshair_checked">
<rect
width="28"
height="28"
fill="white"
/>
</clipPath>
</defs>
</svg>
);

export const ToolDicomTagBrowser = (props: IconProps) => (
<svg
width="28px"
Expand Down
6 changes: 5 additions & 1 deletion platform/ui-next/src/components/ToolButton/ToolButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useIconPresentation } from '../../contextProviders/IconPresentationProv
const baseClasses = '!rounded-lg inline-flex items-center justify-center';
const defaultClasses = 'bg-transparent text-foreground/80 hover:bg-background hover:text-highlight';
const activeClasses = 'bg-highlight text-background hover:!bg-highlight/80';
const toggledClasses = 'bg-transparent text-highlight hover:bg-muted';
const disabledClasses =
'text-foreground hover:bg-muted hover:text-highlight opacity-40 cursor-not-allowed';

Expand All @@ -33,6 +34,7 @@ interface ToolButtonProps {
tooltip?: string;
size?: 'default' | 'small';
isActive?: boolean;
isToggled?: boolean;
disabled?: boolean;
disabledText?: string;
commands?: Record<string, unknown>;
Expand All @@ -50,6 +52,7 @@ function ToolButton(props: ToolButtonProps) {
size = 'default',
disabled = false,
isActive = false,
isToggled = false,
disabledText,
commands,
onInteraction,
Expand All @@ -63,7 +66,7 @@ function ToolButton(props: ToolButtonProps) {
const buttonClasses = cn(
baseClasses,
buttonSizeClass,
disabled ? disabledClasses : isActive ? activeClasses : defaultClasses,
disabled ? disabledClasses : isActive ? activeClasses : isToggled ? toggledClasses : defaultClasses,
className
);

Expand All @@ -85,6 +88,7 @@ function ToolButton(props: ToolButtonProps) {
data-cy={id}
data-tool={id}
data-active={isActive}
data-toggled={isToggled}
>
<Button
className={buttonClasses}
Expand Down
Loading