feat(measurement): implement Change Color for measurements - #6186
Open
awatson1978 wants to merge 1 commit into
Open
feat(measurement): implement Change Color for measurements#6186awatson1978 wants to merge 1 commit into
awatson1978 wants to merge 1 commit into
Conversation
The measurement panel's row menu has offered a Change Color action since the ui-next MeasurementTable landed, but the changeMeasurementColor command it runs was never implemented, so selecting it silently did nothing. Add the command to the cornerstone extension: it opens the shared color picker dialog seeded with the annotation's effective color from the style hierarchy, then applies the chosen color to the annotation and its text box through the existing updateMeasurement action, which sets per-annotation styles and triggers a re-render. Fixes OHIF#6059 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughChangesMeasurement color command
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
awatson1978
marked this pull request as ready for review
July 27, 2026 20:28
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@extensions/cornerstone/src/commandsModule.ts`:
- Around line 831-844: Update the onSave handler in the measurement color flow
to serialize newRgbaColor as rgba rather than rgb, preserving its alpha value in
the generated CSS color. Apply the alpha-preserving color consistently to all
style fields currently assigned cssColor, while retaining the existing RGB
components and updateMeasurement call.
🪄 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: 2e7eed58-1b61-45b5-a2b9-2860974e8f94
📒 Files selected for processing (1)
extensions/cornerstone/src/commandsModule.ts
Comment on lines
+831
to
+844
| onSave: newRgbaColor => { | ||
| const cssColor = `rgb(${newRgbaColor.r}, ${newRgbaColor.g}, ${newRgbaColor.b})`; | ||
| actions.updateMeasurement({ | ||
| uid, | ||
| style: { | ||
| color: cssColor, | ||
| colorHighlighted: cssColor, | ||
| colorSelected: cssColor, | ||
| colorLocked: cssColor, | ||
| textBoxColor: cssColor, | ||
| textBoxColorHighlighted: cssColor, | ||
| textBoxColorSelected: cssColor, | ||
| textBoxColorLocked: cssColor, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep 'extensions/cornerstone/src/commandsModule.ts' || true
echo "== outline relevant symbols =="
ast-grep outline extensions/cornerstone/src/commandsModule.ts --match changeMeasurementColor --view expanded || true
echo "== relevant lines 780-860 =="
sed -n '780,860p' extensions/cornerstone/src/commandsModule.ts
echo "== search color parsing/save usages =="
rg -n "_cssColorToRgba|changeMeasurementColor|colorHighlighted|cssColorTo|rgba|rgb\\(" extensions/cornerstone/src -S
echo "== parse/save color implementation candidates =="
python3 - <<'PY'
from pathlib import Path
p = Path('extensions/cornerstone/src/commandsModule.ts')
text = p.read_text()
for needle in ['function', 'const', 'let', 'var']:
pass
# Print lines mentioning rgbaColor or color style update
lines = text.splitlines()
for i,l in enumerate(lines,1):
if ('rgbaColor' in l or '_cssColorToRgba' in l or 'ColorPicker' in l or 'Save' in l or 'onSave:' in l):
print(f"{i:5}: {l}")
PYRepository: OHIF/Viewers
Length of output: 6155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== color parsing helper =="
sed -n '114,160p' extensions/cornerstone/src/commandsModule.ts
echo "== locate colorPickerDialog definition/import =="
rg -n "export .*colorPickerDialog|const colorPickerDialog|function colorPickerDialog|colorPickerDialog|ColorPicker|ColorPickerComponent" extensions/cornerstone/src -S
echo "== files with colorPickerDialog =="
git ls-files extensions/cornerstone/src | xargs rg -l "colorPickerDialog|ColorPickerComponent|ColorPicker" -S
echo "== inspect likely dialog file(s) =="
while IFS= read -r f; do
echo "--- $f"
sed -n '1,240p' "$f"
done < <(rg -l "colorPickerDialog|ColorPickerComponent|ColorPicker" extensions/cornerstone/src)
echo "== behavioral probe of parsing and save-format expressions =="
python3 - <<'PY'
import re
def rgba(css_color):
rgb_match = re.search(r'rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*([\d.]+)\s*)?\)', css_color)
if not rgb_match:
return None
r, g, b, a = rgb_match.groups()
return {'r': int(r), 'g': int(g), 'b': int(b), 'a': 1.0 if a is None else float(a)}
def save_rgb(rgba_color):
return f"rgb({rgba_color['r']}, {rgba_color['g']}, {rgba_color['b']})"
def save_rgba(rgba_color):
return f"rgba({rgba_color['r']}, {rgba_color['g']}, {rgba_color['b']}, {rgba_color['a']})"
cases = [
"rgba(255, 0, 0, 0.5)",
"rgba(0, 0, 255, 0)",
"rgba(0, 0, 255, 1)",
"rgb(0, 255, 0)",
]
for case in cases:
parsed = rgba(case)
print(case, "=>", save_rgb(parsed), "== opaque?", not parsed['a'] < 1.0, "=> rgba?", save_rgba(parsed))
PYRepository: OHIF/Viewers
Length of output: 1934
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== import section =="
sed -n '1,45p' extensions/cornerstone/src/commandsModule.ts
echo "== other usage around lines 1828-1856 =="
sed -n '1828,1856p' extensions/cornerstone/src/commandsModule.ts
echo "== all files mentioning colorPickerDialog with line numbers =="
rg -n "colorPickerDialog" extensions/corner -S || true
echo "== search color picker packages/components =="
rg -n "ColorPicker|color-picker|react-color|`@uiw/react-color`|`@uiw/color`" package.json pnpm-lock.yaml package-lock.json yarn.lock extensions -S || true
echo "== npm package availability probe =="
node - <<'JS'
const fs = require('fs');
for (const p of [
'package.json',
'pnpm-lock.yaml',
'package-lock.json',
'yarn.lock'
]) {
if (fs.existsSync(p)) {
console.log(p + ' exists');
const t = fs.readFileSync(p, 'utf8');
for (const term of ['`@uiw/react-color`', '`@uiw/color`', 'ColorPicker']) {
let found = t.includes(term);
if (p.endsWith('.json') && term === '`@uiw/react-color`') {
// more focused
const idx = t.indexOf('"`@uiw/react-color`"');
found = idx !== -1;
}
console.log(p + ': ' + term + ' => ' + found);
}
}
}
JS
echo "== behavioral probe of parsing and save-format expressions =="
python3 - <<'PY'
import re
def rgba(css_color):
rgb_match = re.search(r'rgba?\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*(?:,\s*([\d.]+)\s*)?\)', css_color)
if not rgb_match:
return None
r, g, b, a = rgb_match.groups()
return {'r': int(r), 'g': int(g), 'b': int(b), 'a': 1.0 if a is None else float(a)}
def save_rgb(rgba_color):
return f"rgb({rgba_color['r']}, {rgba_color['g']}, {rgba_color['b']})"
def save_rgba(rgba_color):
return f"rgba({rgba_color['r']}, {rgba_color['g']}, {rgba_color['b']}, {rgba_color['a']})"
cases = [
"rgba(255, 0, 0, 0.5)",
"rgba(0, 0, 255, 0)",
"rgba(0, 0, 255, 1)",
"rgb(0, 255, 0)",
]
for case in cases:
parsed = rgba(case)
print(case, "=>", save_rgb(parsed), "== opaque?", parsed['a'] >= 0.9999, "=> rgba?", save_rgba(parsed))
PYRepository: OHIF/Viewers
Length of output: 4652
Preserve the selected alpha value.
_cssColorToRgba parses alpha from values like rgba(..., 0.5), but the measurement color save path emits rgb(...), which discards non-opaque alpha and changes the displayed measurement.
Proposed fix
- const cssColor = `rgb(${newRgbaColor.r}, ${newRgbaColor.g}, ${newRgbaColor.b})`;
+ const cssColor = `rgba(${newRgbaColor.r}, ${newRgbaColor.g}, ${newRgbaColor.b}, ${newRgbaColor.a})`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onSave: newRgbaColor => { | |
| const cssColor = `rgb(${newRgbaColor.r}, ${newRgbaColor.g}, ${newRgbaColor.b})`; | |
| actions.updateMeasurement({ | |
| uid, | |
| style: { | |
| color: cssColor, | |
| colorHighlighted: cssColor, | |
| colorSelected: cssColor, | |
| colorLocked: cssColor, | |
| textBoxColor: cssColor, | |
| textBoxColorHighlighted: cssColor, | |
| textBoxColorSelected: cssColor, | |
| textBoxColorLocked: cssColor, | |
| }, | |
| onSave: newRgbaColor => { | |
| const cssColor = `rgba(${newRgbaColor.r}, ${newRgbaColor.g}, ${newRgbaColor.b}, ${newRgbaColor.a})`; | |
| actions.updateMeasurement({ | |
| uid, | |
| style: { | |
| color: cssColor, | |
| colorHighlighted: cssColor, | |
| colorSelected: cssColor, | |
| colorLocked: cssColor, | |
| textBoxColor: cssColor, | |
| textBoxColorHighlighted: cssColor, | |
| textBoxColorSelected: cssColor, | |
| textBoxColorLocked: cssColor, | |
| }, |
🤖 Prompt for 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.
In `@extensions/cornerstone/src/commandsModule.ts` around lines 831 - 844, Update
the onSave handler in the measurement color flow to serialize newRgbaColor as
rgba rather than rgb, preserving its alpha value in the generated CSS color.
Apply the alpha-preserving color consistently to all style fields currently
assigned cssColor, while retaining the existing RGB components and
updateMeasurement call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fixes #6059
Every measurement row in the panel has a "…" menu with a Change Color option — but clicking it does absolutely nothing. No dialog, no error, nothing. As the issue notes, it wasn't even clear whether the feature was ever implemented. It turns out: it wasn't.
Changes & Results
Why this happens: the ui-next
MeasurementTablewires its Change Color menu item to run a command calledchangeMeasurementColor— but no extension ever registered a command by that name, so the commands manager silently drops it.The change: implement
changeMeasurementColorin the cornerstone extension's commands module, reusing pieces that already exist:annotation.config.style.getStyleProperty)updateMeasurementaction, which already knows how to apply per-annotation styles (setAnnotationStyles) and trigger a viewport re-renderOn save, the chosen color is applied to the annotation line and its text box across all interaction states (default / highlighted / selected / locked), so the measurement keeps its color when hovered or selected.
Before (Change Color → nothing) / After (color picker opens; measurement and label re-render in the chosen color immediately):
Testing
Verified in Chromium via Playwright: dialog opens, per-annotation styles are set (checked via
annotation.config.style.getAnnotationToolStyles), and the viewport renders the new color. Also confirmed the pre-fix behavior (menu click is a silent no-op).Screenshots
6059-before-nothing-happens
6059-after-dialog-open
6059-after-annotation-recolored
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
Tested Environment
🤖 Generated with Claude Code
Summary by CodeRabbit