Skip to content

Comments

test(advanced-color-picker): refine testing suite#7772

Open
paulrobinson88 wants to merge 2 commits intomasterfrom
FE-6440
Open

test(advanced-color-picker): refine testing suite#7772
paulrobinson88 wants to merge 2 commits intomasterfrom
FE-6440

Conversation

@paulrobinson88
Copy link
Contributor

Proposed behaviour

Removed flaky and duplicate tests from the Playwright file to improve stability and run times for the Advanced Color Picker component.
Amended Interaction tests to cover off aspects which weren't covered in Jest or Playwright.

Current behaviour

The Playwright test file contains a number of tests which are covered in either Jest or Storybook interactions.

Checklist

  • Commits follow our style guide
  • Related issues linked in commit messages if required
  • Screenshots are included in the PR if useful
  • All themes are supported if required
  • Unit tests added or updated if required
  • Playwright automation tests added or updated if required
  • Storybook added or updated if required
  • Translations added or updated (including creating or amending translation keys table in storybook) if required
  • Typescript d.ts file added or updated if required
  • Related docs have been updated if required

QA

  • Tested in provided StackBlitz sandbox/Storybook
  • Add new Playwright test coverage if required
  • Carbon implementation matches Design System/designs
  • UI Tests GitHub check reviewed if required

Additional context

Testing instructions

Check Storybook interactions pass and work as intended.

test("renders with aria-describedby prop", () => {
render(<ControlledColorPicker open aria-describedby="test-id" />);

expect(screen.getByRole("dialog")).toHaveAttribute(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(screen.getByRole("dialog")).toHaveAttribute(
expect(screen.getByRole("dialog")).toHaveAccessibleDescription(

test("renders with aria-labelledby prop", () => {
render(<ControlledColorPicker open aria-labelledby="test-label" />);

expect(screen.getByRole("dialog")).toHaveAttribute(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(screen.getByRole("dialog")).toHaveAttribute(
expect(screen.getByRole("dialog")).toHaveAccessibleName(

edleeks87
edleeks87 previously approved these changes Feb 18, 2026
@DipperTheDan DipperTheDan self-requested a review February 19, 2026 10:41
DipperTheDan
DipperTheDan previously approved these changes Feb 19, 2026
@paulrobinson88 paulrobinson88 marked this pull request as ready for review February 19, 2026 13:50
@paulrobinson88 paulrobinson88 requested review from a team as code owners February 19, 2026 13:50
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not required as focus is set when the button is clicked.

@@ -273,4 +257,4 @@
});

purpleChecked.focus();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not required as focus is set when the call to action button was clicked.

Copy link
Contributor Author

@paulrobinson88 paulrobinson88 Feb 20, 2026

Choose a reason for hiding this comment

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

I initially removed this but the interaction test failed in the pipeline, possibly a timing issue since manually testing and closing it does focus the purple colour but specifically focusing does seem to resolve. The tests did pass in the published Chromatic build and locally though 🤷

});

purpleChecked.focus();
await waitFor(() => expect(purpleChecked).toHaveFocus());
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be the final assertion. The test is incomplete and does not test the prop. The restoreFocusOnClose prop should return focus to the call to action initial trigger button when the colour picker is closed. There should be a test here that checks that when the colour picker is closed, focus is returned to the initial call to action button.

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

Development

Successfully merging this pull request may close these issues.

4 participants