test(segmentation): add labelmap segment color change E2E tests. - #6054
test(segmentation): add labelmap segment color change E2E tests.#6054diattamo wants to merge 7 commits into
Conversation
Moves the labelmap segmentation color-change E2E test and its screenshots onto a dedicated branch.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR adds comprehensive test infrastructure for the labelmap segmentation color-picker dialog. UI components receive ChangesLabelmap Segmentation Color Change Test Suite
Sequence Diagram(s)sequenceDiagram
participant Test
participant RightPanel
participant Dialog
participant DOMOverlay
Test->>RightPanel: openChangeColor()
RightPanel->>Dialog: click "Change Color" action
Test->>DOMOverlay: colorPicker.fillHex(hex)
DOMOverlay->>Dialog: fill hex input + press Enter
Test->>DOMOverlay: colorPicker.save()
DOMOverlay->>Dialog: click save button
Test->>RightPanel: rowDataColorHex
RightPanel->>Test: return color indicator CSS value
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?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 `@tests/pages/DOMOverlayPageObject.ts`:
- Around line 64-65: The hex input selection and assertions are brittle: replace
locator.getByLabel('hex') with a scoped query for the actual input element
inside the color picker (e.g., use the locator from
getByTestId('color-picker-dialog') and then find the input element via
locator.locator('input') or getByRole('textbox') scoped to that dialog) so you
reliably target the ChromePicker EditableInput, and when checking its value
normalize case and/or perform a case-insensitive match and ensure the string
contains the leading '#' (e.g., compare lowercased value or use a regex like
/^`#/i`) instead of asserting a specific uppercase hex string.
🪄 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: c3454003-0f70-4bf6-a421-d5920f8a6aef
⛔ Files ignored due to path filters (4)
tests/screenshots/chromium/LabelMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-afterCancel.pngis excluded by!**/*.pngtests/screenshots/chromium/LabelMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-afterChange.pngis excluded by!**/*.pngtests/screenshots/chromium/LabelMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-beforeCancel.pngis excluded by!**/*.pngtests/screenshots/chromium/LabelMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-beforeChange.pngis excluded by!**/*.png
📒 Files selected for processing (6)
extensions/default/src/utils/colorPickerDialog.tsxplatform/ui-next/src/components/DataRow/DataRow.tsxtests/LabelMapSegmentationColorChange.spec.tstests/pages/DOMOverlayPageObject.tstests/pages/RightPanelPageObject.tstests/utils/screenShotPaths.ts
| const locator = page.getByTestId('color-picker-dialog'); | ||
| const hexInput = locator.getByLabel('hex'); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In react-color 2.19.3, does the ChromePicker "hex" EditableInput render a <label> associated with its <input> (so a by-label query matches it), and is the displayed value uppercased and prefixed with "#"?
💡 Result:
Answer: No. In react-color 2.19.3, the ChromePicker “hex” field uses EditableInput that renders an without an associated (so a by-label query for the label text should not match the input). The displayed hex value is rendered as a plain hex string coming from tinycolor’s toHexString, which includes the “#” prefix but does not guarantee uppercase (it reflects tinycolor’s output/casing for the given color). Evidence: 1) The EditableInput implementation in react-color renders a wrapper
Citations:
Fix hexInput locator/value expectations for react-color ChromePicker
locator.getByLabel('hex') is unreliable with react-color 2.19.3: the ChromePicker “hex” EditableInput renders an <input> without a label/htmlFor association, so getByLabel('hex') may not resolve. The displayed value comes from tinycolor(...).toHexString (includes #), but casing isn’t guaranteed—avoid asserting a specific uppercase hex string (use a case-insensitive match or normalize case).
🤖 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 `@tests/pages/DOMOverlayPageObject.ts` around lines 64 - 65, The hex input
selection and assertions are brittle: replace locator.getByLabel('hex') with a
scoped query for the actual input element inside the color picker (e.g., use the
locator from getByTestId('color-picker-dialog') and then find the input element
via locator.locator('input') or getByRole('textbox') scoped to that dialog) so
you reliably target the ChromePicker EditableInput, and when checking its value
normalize case and/or perform a case-insensitive match and ensure the string
contains the leading '#' (e.g., compare lowercased value or use a regex like
/^`#/i`) instead of asserting a specific uppercase hex string.
…-change # Conflicts: # tests/pages/DOMOverlayPageObject.ts # tests/pages/RightPanelPageObject.ts
GitHub Actions
Error
…amo/Viewers into feat/labelmap-color-change
GitHub Actions
Inactive
diattamo
changed the title
1 participant
Context
Add coverage for changing the color of labelmaps
Changes & Results
tests/utils/screenShotPaths.ts— registered newlabelMapSegmentationColorChangebaseline keys.tests/LabelMapSegmentationColorChange.spec.ts— three testscovering: opening the dialog with the current hex prefilled, saving
a new color, and cancelling the dialog. Verification uses
checkForScreenshotscoped to the default viewport for thecanvas-side check; four baseline screenshots are added under
tests/screenshots/chromium/LabelMapSegmentationColorChange.spec.ts/.Testing
From the repo root, run both new specs:
yarn playwright test tests/LabelMapSegmentationColorChange.spec.tsTo regenerate the labelmap baselines after an intentional rendering
change:
yarn playwright test tests/LabelMapSegmentationColorChange.spec.ts --update-snapshotsChecklist
PR
semantic-release format and guidelines.
Code
Public Documentation Updates
Tested Environment
Summary by CodeRabbit
Greptile Summary
This PR adds Playwright end-to-end test coverage for the labelmap segmentation color-change workflow, wiring up three tests (dialog opens with prefilled hex, save changes the color, cancel does not). Supporting changes add
data-cyattributes toColorPickerDialogand theDataRowcolor swatch, and extend theRightPanelPageObject/DOMOverlayPageObjectwith color-picker helpers.data-cyattributes are added to the dialog wrapper and the Save/Cancel buttons incolorPickerDialog.tsx, and to the color swatch span inDataRow.tsx.Confidence Score: 3/5
The production code changes (data-cy additions) are safe, but the new test infrastructure has three areas that may make the tests unreliable or always-failing on CI without further verification.
The hexInput locator uses getByLabel('hex') against ChromePicker, which does not expose a proper accessibility label — the locator may silently return no element. The hex value assertions and fill calls pass a '#'-prefixed string that ChromePicker's raw input may not accept or return. Finally, waitForViewportRenderCycle is captured before the multi-step changeColor action, so an intermediate render during the action sequence could let the screenshot race ahead of the actual color-change render.
tests/pages/DOMOverlayPageObject.ts (hexInput locator) and tests/LabelMapSegmentationColorChange.spec.ts (hex value format, render-cycle timing)
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge branch 'master' into feat/labelmap..." | Re-trigger Greptile