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

Add missing elements that have a factor on contrast #514

Merged
merged 1 commit into from
Sep 7, 2017

Conversation

mfairchild365
Copy link
Contributor

document.elementsFromPoint does not include the TBODY and THEAD elements, so contrast was not being calculated correctly under some circumstances involving those elements. See added tests for examples.

Copy link
Contributor

@dylanb dylanb 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 to me - thanks for the contribution!!!

@dylanb
Copy link
Contributor

dylanb commented Sep 7, 2017

@mfairchild365 the changes are good but I just noticed that the commit messages do not match our required style (which allows us to generate our release notes). Could you fix this using rebase?

@mfairchild365
Copy link
Contributor Author

@dylanb rebase was giving me grief, so I just deleted the branch, added the changes back, and made a new commit.

@dylanb
Copy link
Contributor

dylanb commented Sep 7, 2017

Awesome!! @WilcoFiers @marcysutton any further comments?

@marcysutton
Copy link
Contributor

Thanks so much for the contribution @mfairchild365! I ran the tests on this branch since Circle still isn't running on forks (#497) and they look all good.

It seems like a good addition to me; I wish we didn't have to do so much to shim support for document.elementsFromPoint but I'm glad we already had logic in place to expand upon. I didn't even know you could style those elements, TIL! :)

@marcysutton marcysutton merged commit f98f8bd into dequelabs:develop Sep 7, 2017
@marcysutton
Copy link
Contributor

I'll make a few changes so this can go into develop-2x as well.

mrtnvh pushed a commit to mrtnvh/axe-core that referenced this pull request Nov 24, 2023
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