Skip to content
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.

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

test(overlays): add Axe tests for color contrast #28630

Merged
merged 14 commits into from
Dec 12, 2023
Merged

Conversation

amandaejohnston
Copy link
Contributor

@amandaejohnston amandaejohnston commented Dec 4, 2023

Issue number: Internal


What is the current behavior?

  • The action-sheet, alert, and toast components have tests for Axe violations, but the color-contrast rule is disabled.
  • The select-popover component does not test for Axe violations.
  • For all of the above plus loading, dark mode test for color contrast are not included.

What is the new behavior?

  • Existing Axe tests pulled out into separate configs blocks that test both light and dark themes. Markup has also been converted to use page.setContent instead of page.goto to support automatic theme switching.
  • color-contrast rule re-enabled on action-sheet, alert, and toast. The rule was disabled due to contrast issues with the page content covered up by the backdrop, but with the new setup, the overlay is the only thing on the page.
  • For select-popover, the Axe test has been limited to only color contrast testing for now. The component had several Axe failures that will be addressed separately to keep scope down. (We didn't previously have any Axe testing on this component, which is why this wasn't caught.)
  • HTML file for the loading a11y test removed, since we're no longer using it.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@amandaejohnston amandaejohnston changed the title test(overlays): add Axe tests (todo better title) test(overlays): add Axe tests for color contrast Dec 5, 2023
@github-actions github-actions bot added the package: core @ionic/core package label Dec 5, 2023
@amandaejohnston amandaejohnston marked this pull request as ready for review December 8, 2023 16:07
@amandaejohnston amandaejohnston requested a review from a team as a code owner December 8, 2023 16:07
@amandaejohnston amandaejohnston requested review from liamdebeasi and removed request for a team December 8, 2023 16:07
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Looks good! One non-blocking suggestion and one question.

core/src/components/toast/test/a11y/toast.e2e.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Great work!

@amandaejohnston amandaejohnston merged commit 886a3df into FW-5584 Dec 12, 2023
44 checks passed
@amandaejohnston amandaejohnston deleted the FW-5635 branch December 12, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants