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

feat(aria-role): Remove non-existing "text" role #1069

Merged
merged 4 commits into from
Aug 22, 2018
Merged

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented Aug 15, 2018

Remove role=text was proposed for ARIA 1.1 but never made it in. It shouldn't be allowed by axe-core. Here's a list of all valid roles: https://www.w3.org/TR/wai-aria-1.1/#role_definitions

Closes #1048

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: Jey (@JKODU)

@WilcoFiers WilcoFiers requested a review from a team as a code owner August 15, 2018 10:05
jeeyyy
jeeyyy previously requested changes Aug 16, 2018
Copy link
Contributor

@jeeyyy jeeyyy left a comment

Choose a reason for hiding this comment

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

@WilcoFiers

  • Please resolve conflicts.
  • And add a prefix to PR message (eg: fix/ chore) & description if any.

@WilcoFiers WilcoFiers changed the title Remove text role feat(aria-role): Remove non-existing "text" role Aug 17, 2018
@jeeyyy
Copy link
Contributor

jeeyyy commented Aug 21, 2018

@WilcoFiers

Because of the way axe-codeowners is set up, if a PR is approved by anyone other than a codeowner, the status does not change in the list of PR's.

image

image

This is very misleading, or is this intentional? Also means for PR from yourself, you cannot merge until one of the other code owner(s) have approved it.

@WilcoFiers
Copy link
Contributor Author

@JKODU This is intentional. Because of the complexity of axe-core we want at least one of the code owners to approve PRs to develop.

@WilcoFiers WilcoFiers merged commit 67ec1f5 into develop Aug 22, 2018
@WilcoFiers WilcoFiers deleted the text-role branch August 22, 2018 13:42
stephenmathieson added a commit that referenced this pull request Aug 22, 2018
…re into circleci-releases

* 'circleci-releases' of ssh://github.com/dequelabs/axe-core:
  feat(aria-role): Remove non-existing "text" role (#1069)
@WilcoFiers WilcoFiers mentioned this pull request Aug 27, 2018
8 tasks
WilcoFiers added a commit that referenced this pull request Aug 27, 2018
Review 10% (50 max) of the following (Product manager only):

- [x] feat: Update FR (french) translation file for 3.1 release (#1089) (4a5cad0)
- [x] chore: Fix example dependency mistake (#1094) (6af5733)
- [x] fix: Do not require media captions / descriptions (#1075) (289f623)
- [x] feat(aria-role): Remove non-existing "text" role (#1069) (67ec1f5)
- [x] Merge pull request #1017 from dequelabs/aria-getrole (f64ff10)
- [x] chore: remove `.jshintrc` files (#1028) (03b3327)
- [x] chore: ability to add external libs (axios) (0957dab)
- [x] Merge pull request #985 from dequelabs/git-update (54b0b60)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants