Skip to content

Tests/labelmap color change tests #6054

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

Draft
diattamo wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Tests/labelmap color change tests #6054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
363d681
test(seg): add labelmap segmentation color change coverage
diattamo Jun 2, 2026
aeed014
Merge branch 'master' into feat/labelmap-color-change
diattamo Jun 2, 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
10 changes: 8 additions & 2 deletions extensions/default/src/utils/colorPickerDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function ColorPickerDialog({ value, hide, onSave }) {
};

return (
<div>
<div data-cy="color-picker-dialog">
<ChromePicker
color={color}
onChange={handleChange}
Expand All @@ -21,8 +21,14 @@ function ColorPickerDialog({ value, hide, onSave }) {
/>
<FooterAction>
<FooterAction.Right>
<FooterAction.Secondary onClick={hide}>Cancel</FooterAction.Secondary>
<FooterAction.Secondary
dataCY="color-picker-cancel-btn"
onClick={hide}
>
Cancel
</FooterAction.Secondary>
<FooterAction.Primary
dataCY="color-picker-save-btn"
onClick={() => {
hide();
onSave(color);
Expand Down
1 change: 1 addition & 0 deletions platform/ui-next/src/components/DataRow/DataRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ const DataRowComponent = React.forwardRef<HTMLDivElement, DataRowProps>(
<span
className="ml-2 h-2 w-2 rounded-full"
style={{ backgroundColor: colorHex }}
data-cy="data-row-colorhex"
></span>
</div>
)}
Expand Down
125 changes: 125 additions & 0 deletions tests/LabelMapSegmentationColorChange.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import {
expect,
test,
visitStudy,
waitForViewportRenderCycle,
waitForViewportsRendered,
checkForScreenshot,
screenShotPaths,
} from './utils';

const NEW_LABELMAP_SEGMENT_HEX = '#FF00FF';
const NEW_LABELMAP_SEGMENT_HEX_CSS_RGB = 'rgb(255, 0, 255)';

// Default color of segment 0 in the labelmap SEG study.
const LABELMAP_SEGMENT_0_DEFAULT_HEX = '#9D6CA2';
const LABELMAP_SEGMENT_0_DEFAULT_CSS_RGB = 'rgb(157, 108, 162)';

const STUDY_UID = '1.3.6.1.4.1.14519.5.2.1.256467663913010332776401703474716742458';

test.beforeEach(async ({ page, leftPanelPageObject, DOMOverlayPageObject }) => {
await visitStudy(page, STUDY_UID, 'segmentation', 2000);

await leftPanelPageObject.loadSeriesByModality('SEG');
await waitForViewportsRendered(page);

await expect(DOMOverlayPageObject.viewport.segmentationHydration.locator).toBeVisible();
await DOMOverlayPageObject.viewport.segmentationHydration.yes.click();
});

test('opens the color edit popup when "Change Color" is clicked', async ({
rightPanelPageObject,
DOMOverlayPageObject,
}) => {
const segment = rightPanelPageObject.labelMapSegmentationPanel.panel.nthSegment(0);
await segment.toggleVisibility();

await segment.actions.openChangeColor();

await expect(DOMOverlayPageObject.dialog.colorPicker.locator).toBeVisible();
await expect(DOMOverlayPageObject.dialog.title).toHaveText('Segment Color');
await expect(DOMOverlayPageObject.dialog.colorPicker.saveButton).toBeVisible();
await expect(DOMOverlayPageObject.dialog.colorPicker.cancelButton).toBeVisible();

await expect(DOMOverlayPageObject.dialog.colorPicker.hexInput).toHaveValue(
LABELMAP_SEGMENT_0_DEFAULT_HEX
);
Comment on lines +44 to +46

@greptile-apps greptile-apps Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Hex value assertion may fail due to # prefix mismatch

ChromePicker typically stores and displays the hex value as the 6-character string without the # (e.g. 9D6CA2), while the # glyph is rendered as a static prefix outside the <input>. If that's the case here, toHaveValue('#9D6CA2') will always fail. The same concern applies to hexInput.fill('#FF00FF') in fillHexAndSave / fillHexAndCancel — passing the # prefix into an input that doesn't expect it may silently produce an unexpected color state.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tests/LabelMapSegmentationColorChange.spec.ts
Line: 44-46

Comment:
**Hex value assertion may fail due to `#` prefix mismatch**

`ChromePicker` typically stores and displays the hex value as the 6-character string without the `#` (e.g. `9D6CA2`), while the `#` glyph is rendered as a static prefix outside the `<input>`. If that's the case here, `toHaveValue('#9D6CA2')` will always fail. The same concern applies to `hexInput.fill('#FF00FF')` in `fillHexAndSave` / `fillHexAndCancel` — passing the `#` prefix into an input that doesn't expect it may silently produce an unexpected color state.

How can I resolve this? If you propose a fix, please make it concise.

});

test('changes the labelmap segment color when the user saves', async ({
page,
rightPanelPageObject,
DOMOverlayPageObject,
viewportPageObject,
}) => {
await rightPanelPageObject.labelMapSegmentationPanel.segmentsVisibilityToggle.click();
const segment = rightPanelPageObject.labelMapSegmentationPanel.panel.nthSegment(0);
await segment.toggleVisibility();
await segment.click();

await expect(segment.rowDataColorHex).toHaveCSS(
'background-color',
LABELMAP_SEGMENT_0_DEFAULT_CSS_RGB
);

const viewportPane = (await viewportPageObject.getById('default')).pane;
await checkForScreenshot({
page,
locator: viewportPane,
screenshotPath: screenShotPaths.labelMapSegmentationColorChange.colorBeforeChange
});

const colorChangeCycle = waitForViewportRenderCycle(page);
await segment.actions.changeColor(NEW_LABELMAP_SEGMENT_HEX);
await colorChangeCycle;
Comment on lines +72 to +74

@greptile-apps greptile-apps Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Render-cycle promise captured before the action that triggers the render

waitForViewportRenderCycle(page) is called before segment.actions.changeColor(...), which itself performs four sequential interactions (open menu → click "Change Color" → fill hex → click Save). If the viewport renders for any intermediate reason during those steps — such as the color picker opening or focus events — the promise resolves early and await colorChangeCycle becomes a no-op. The screenshot that follows may then be captured before the actual color-change render completes. Capturing the promise after changeColor resolves would remove this race.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tests/LabelMapSegmentationColorChange.spec.ts
Line: 72-74

Comment:
**Render-cycle promise captured before the action that triggers the render**

`waitForViewportRenderCycle(page)` is called before `segment.actions.changeColor(...)`, which itself performs four sequential interactions (open menu → click "Change Color" → fill hex → click Save). If the viewport renders for any intermediate reason during those steps — such as the color picker opening or focus events — the promise resolves early and `await colorChangeCycle` becomes a no-op. The screenshot that follows may then be captured before the actual color-change render completes. Capturing the promise _after_ `changeColor` resolves would remove this race.

How can I resolve this? If you propose a fix, please make it concise.


await expect(DOMOverlayPageObject.dialog.colorPicker.locator).toBeHidden();
await expect(segment.rowDataColorHex).toHaveCSS(
'background-color',
NEW_LABELMAP_SEGMENT_HEX_CSS_RGB
);

await checkForScreenshot({
page,
locator: viewportPane,
screenshotPath: screenShotPaths.labelMapSegmentationColorChange.colorAfterChange
});
});

test('does not change the labelmap segment color when the user cancels', async ({
page,
rightPanelPageObject,
DOMOverlayPageObject,
viewportPageObject,
}) => {
await rightPanelPageObject.labelMapSegmentationPanel.segmentsVisibilityToggle.click();
const segment = rightPanelPageObject.labelMapSegmentationPanel.panel.nthSegment(0);
await segment.toggleVisibility();
await segment.click();

await expect(segment.rowDataColorHex).toHaveCSS(
'background-color',
LABELMAP_SEGMENT_0_DEFAULT_CSS_RGB
);

const viewportPane = (await viewportPageObject.getById('default')).pane;
await checkForScreenshot({
page,
locator: viewportPane,
screenshotPath: screenShotPaths.labelMapSegmentationColorChange.colorBeforeCancel
});

await segment.actions.cancelChangeColor(NEW_LABELMAP_SEGMENT_HEX);

await expect(DOMOverlayPageObject.dialog.colorPicker.locator).toBeHidden();
await expect(segment.rowDataColorHex).toHaveCSS(
'background-color',
LABELMAP_SEGMENT_0_DEFAULT_CSS_RGB
);

await checkForScreenshot({
page,
locator: viewportPane,
screenshotPath: screenShotPaths.labelMapSegmentationColorChange.colorAfterCancel
});
});
33 changes: 33 additions & 0 deletions tests/pages/DOMOverlayPageObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,39 @@ export class DOMOverlayPageObject {
return new DicomTagBrowserPageObject(page);
},

get colorPicker() {
const locator = page.getByTestId('color-picker-dialog');
const hexInput = locator.getByLabel('hex');

@greptile-apps greptile-apps Bot Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 getByLabel('hex') may not resolve to the ChromePicker input

react-color's ChromePicker uses a <span> as a visual label below the input — it is not a <label> element and carries no for/aria-labelledby association. Playwright's getByLabel relies on the accessibility tree; without a proper label association, this locator will either return no element or throw at runtime. A more reliable selector would be something like locator.locator('input[id*="hex"], input[aria-label="hex"]') or querying by the label-companion pattern that ChromePicker actually renders.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tests/pages/DOMOverlayPageObject.ts
Line: 65

Comment:
**`getByLabel('hex')` may not resolve to the ChromePicker input**

`react-color`'s `ChromePicker` uses a `<span>` as a visual label below the input — it is not a `<label>` element and carries no `for`/`aria-labelledby` association. Playwright's `getByLabel` relies on the accessibility tree; without a proper label association, this locator will either return no element or throw at runtime. A more reliable selector would be something like `locator.locator('input[id*="hex"], input[aria-label="hex"]')` or querying by the label-companion pattern that ChromePicker actually renders.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +64 to +65

@coderabbitai coderabbitai Bot Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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

containing only an (with no element and no id/htmlFor linkage): the patched source shows render returning
<input is="input" value={ this.props.value } readOnly/>
.[1] 2) In the ChromePicker “hex” view, EditableInput is instantiated with label="hex" and value={ color.toHexString }, i.e., the input’s value comes from tinycolor toHexString and not from the label text.[2] So: - By-label query: should not match, because there is no associated element targeting the input.[1] - Value formatting: the value originates from toHexString (which includes the leading “#” prefix), but uppercase is not explicitly enforced in react-color; casing is determined by tinycolor.[2]

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.

const saveButton = page.getByTestId('color-picker-save-btn');
const cancelButton = page.getByTestId('color-picker-cancel-btn');
return {
locator,
hexInput,
saveButton,
cancelButton,
fillHex: async (hex: string) => {
await hexInput.fill(hex);
await hexInput.press('Enter');
},
save: async () => {
await saveButton.click();
},
cancel: async () => {
await cancelButton.click();
},
fillHexAndSave: async (hex: string) => {
await hexInput.fill(hex);
await hexInput.press('Enter');
await saveButton.click();
},
fillHexAndCancel: async (hex: string) => {
await hexInput.fill(hex);
await hexInput.press('Enter');
await cancelButton.click();
},
};
},

title: page.locator('[role="dialog"] h2'),
};
}
Expand Down
23 changes: 23 additions & 0 deletions tests/pages/RightPanelPageObject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@ export class RightPanelPageObject {
await actionsButton.click();
await this.page.getByTestId('Duplicate').click();
},
openChangeColor: async () => {
await actionsButton.click();
await this.page.getByTestId('Change Color').click();
},
changeColor: async (hex: string) => {
await actionsButton.click();
await this.page.getByTestId('Change Color').click();
await this.DOMOverlayPageObject.dialog.colorPicker.fillHexAndSave(hex);
},
cancelChangeColor: async (hex?: string) => {
await actionsButton.click();
await this.page.getByTestId('Change Color').click();
if (hex) {
await this.DOMOverlayPageObject.dialog.colorPicker.fillHexAndCancel(hex);
} else {
await this.DOMOverlayPageObject.dialog.colorPicker.cancel();
}
},
};
}

Expand All @@ -86,6 +104,9 @@ export class RightPanelPageObject {
get title() {
return row.getByTestId('data-row-title');
},
get rowDataColorHex() {
return row.getByTestId('data-row-colorhex');
},
click: async () => {
await row.getByTestId('data-row-title').click();
},
Expand Down Expand Up @@ -263,12 +284,14 @@ export class RightPanelPageObject {
const panel = this.getSegmentationPanel('Labelmap');
const menuButton = page.getByTestId('panelSegmentationWithToolsLabelMap-btn');
const segmentationSelect = this.getSegmentationSelect('Labelmap');
const segmentsVisibilityToggle = this.getSegmentsVisibilityToggle('Labelmap');

return {
addSegmentationButton,
menuButton,
panel,
segmentationSelect,
segmentsVisibilityToggle,
select: async () => {
await menuButton.click();
},
Expand Down
Binary file added ...belMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-afterCancel.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...belMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-afterChange.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...elMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-beforeCancel.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...elMapSegmentationColorChange.spec.ts/labelMapSegmentationColor-beforeChange.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions tests/utils/screenShotPaths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ const screenShotPaths = {
globalUnlockedSegPreEdit: 'unlockedSegPreEdit.png',
globalUnlockedSegPostEdit: 'unlockedSegPostEdit.png',
},
labelMapSegmentationColorChange: {
colorBeforeChange: 'labelMapSegmentationColor-beforeChange.png',
colorAfterChange: 'labelMapSegmentationColor-afterChange.png',
colorBeforeCancel: 'labelMapSegmentationColor-beforeCancel.png',
colorAfterCancel: 'labelMapSegmentationColor-afterCancel.png',
},
length: {
lengthDisplayedCorrectly: 'lengthDisplayedCorrectly.png',
},
Expand Down
Loading