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(accessible-name): Allow fallback labels when input has id #951

Merged
merged 1 commit into from
Jun 21, 2018

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented Jun 10, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

Description of the changes

I was poking around in the accessibleName method when I found/ noticed this little bug. It turns out that if you have an input element with an ID, Axe doesn't bother looking for an implicit label. That should obviously have been based on if there was actually an explicit label, not if the input has an ID.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@@ -778,6 +778,18 @@ describe('text.accessibleTextVirtual', function() {
});
});

it('should find implicit labels with id', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the point of this test that the ID doesn't match the label? Might be good to add "invalid ID" in the test name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about "with id that does not match to a label"?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would work!

@WilcoFiers WilcoFiers merged commit 54fa569 into develop Jun 21, 2018
@WilcoFiers WilcoFiers deleted the implicit-label-fix branch June 21, 2018 13:55
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