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.

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

Jump to bottom

fix(aria-allowed-role): add gridcell, separator, slider and treeitem to allowed roles of button element #4398

Merged
merged 1 commit into from
Apr 5, 2024
Merged

fix(aria-allowed-role): add gridcell, separator, slider and treeitem to allowed roles of button element #4398

merged 1 commit into from
Apr 5, 2024

Conversation

msereniti
Copy link
Contributor

Closes: #4397

@msereniti msereniti requested a review from a team as a code owner April 4, 2024 14:18
@CLAassistant
Copy link

CLAassistant commented Apr 4, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

straker
straker previously approved these changes Apr 4, 2024
View reviewed changes
@straker
Copy link
Contributor

straker commented Apr 4, 2024

Thanks for the pr. I noticed we're also missing roles of gridcell, separator, and treeitem. It makes sense to update them in one go, so if you're willing we could add them to this pr (with an update to the pr title to reflect that).

If not though I've gone ahead and approved the pr and we can merge as is as well

@straker
Copy link
Contributor

straker commented Apr 4, 2024 •
edited
Loading

Awesome, thanks for updating those. There is a single failing test in https://github.com/dequelabs/axe-core/blob/develop/test/integration/rules/aria-allowed-role/aria-allowed-role.html#L222-L227 where we were failing a button with role=gridcell but that's allowed now. The fix would be to update the id to pass-button-role-gridcell and then update the companion json file to remove [#fail-button-role-gridcell] from the violations list and add the new [#pass-button-role-gridcell] to the passes list.

@msereniti
fix(aria-allowed-role): added gridcell, separator, slider and `…
9e9ac2a 
…treeitem` to allowed roles of button element
@msereniti
Copy link
Contributor Author

Looks like all the tests are passing now 👀

straker
straker approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Awesome!

Reviewed for security

@straker straker changed the title fix(aria-allowed-role): added slider to allowed roles of button element fix(aria-allowed-role): add gridcell, separator, slider and treeitem to allowed roles of button element Apr 5, 2024
@straker straker merged commit 4788bf8 into dequelabs:develop Apr 5, 2024
20 of 22 checks passed
@msereniti msereniti deleted the patch-1 branch April 8, 2024 15:09
@attest-team-ci attest-team-ci mentioned this pull request May 6, 2024
Merged
WilcoFiers added a commit that referenced this pull request May 6, 2024
@WilcoFiers
chore(release): v4.9.1 (#4446)
fff9b9d 
###
[4.9.1](v4.9.0...v4.9.1)
(2024-05-06)

### Bug Fixes

- Prevent errors when loading axe in a page with prototype.js
- **aria-allowed-attr:** allow meter role allowed aria-\* attributes on
meter element
([#4435](#4435))
([7ac6392](7ac6392))
- **aria-allowed-role:** add gridcell, separator, slider and treeitem to
allowed roles of button element
([#4398](#4398))
([4788bf8](4788bf8))
- **aria-roles:** correct abstract roles (types) for
aria-roles([#4421](#4421))
- **aria-valid-attr-value:** aria-controls & aria-haspopup incomplete
([#4418](#4418))
- fix building axe-core translation files with region locales
([#4396](#4396))
([5c318f3](5c318f3)),
closes [#4388](#4388)
- **invalidrole:** allow upper and mixed case role names
([#4358](#4358))
([105016c](105016c)),
closes [#2695](#2695)
- **isVisibleOnScreen:** account for position: absolute elements inside
overflow container
([#4405](#4405))
([2940f6e](2940f6e)),
closes [#4016](#4016)
- **label-content-name-mismatch:** better dismiss and wysiwyg symbolic
text characters
([#4402](#4402))
- **region:** Decorative images ignored by region rule
([#4412](#4412))
- **target-size:** ignore descendant elements in shadow dom
([#4410](#4410))
([6091367](6091367))
- **target-size:** pass for element that has nearby elements that are
obscured ([#4422](#4422))
([3a90bb7](3a90bb7)),
closes [#4387](#4387)


This PR was opened by a robot 🤖 🎉 (And updated by @WilcoFiers
)
@nerdyman nerdyman mentioned this pull request May 29, 2024
Closed
@data-sync-user data-sync-user mentioned this pull request Jun 3, 2024
Merged
@wingn8t wingn8t mentioned this pull request Jun 14, 2024
Open
@gurudeep9 gurudeep9 mentioned this pull request Jun 23, 2024
Open
@abdulrahman305 abdulrahman305 mentioned this pull request Jul 3, 2024
Open
@Stanislav1975 Stanislav1975 mentioned this pull request Aug 9, 2024
Open
@Stanislav1975 Stanislav1975 mentioned this pull request Aug 19, 2024
Open
@Stanislav1975 Stanislav1975 mentioned this pull request Sep 9, 2024
Open
@organich organich mentioned this pull request Sep 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@straker straker straker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should slider role be allowed for button?
3 participants
@msereniti @CLAassistant @straker