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

fix(audit): mark widget roles as interactive #9825

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

tugrulates
Copy link
Contributor

Changes

  • passes valid element/role combinations like button/switch (example)
  • adds all subclasses of widget to the interactive roles (by exclusion from non-interactive roles in code), including composite widgets
  • I used the list from https://www.w3.org/TR/wai-aria/#widget_roles
  • a programmatic subclass query, using aria-query is possible but omitted in this PR (please let me know if this would be preferred)
  • looks like the a11y_non_interactive_element_to_interactive_role_exceptions list was setup to handle composite widgets, but it was defunct before
  • this reverts behavior for all of these roles, i.e., now the audit will fail if they are on non-interactive elements

Testing

Manually on minimal example:

<button role="switch" aria-checked="false">Switch</button>

Audit fails before, passes after.

Also tested on blog, framework-react and with-mdx examples to make sure new false positives are not introduced.

Docs

No mention of this audit in the docs (as far as I could find).

Copy link

changeset-bot bot commented Jan 25, 2024

🦋 Changeset detected

Latest commit: 841bc63

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 25, 2024
@tugrulates tugrulates marked this pull request as ready for review January 25, 2024 11:11
Copy link
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

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

This looks great to me!

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I wonder we could add a new test to avoid possible future regressions

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@natemoo-re natemoo-re merged commit e4370e9 into withastro:main Jan 26, 2024
13 of 14 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Jan 26, 2024
@tugrulates tugrulates deleted the fix/audit-widget-roles branch February 15, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants