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(matches): use VirtualNode and deprecate HTMLElement #1988

Merged
merged 8 commits into from Jan 29, 2020
Merged

Conversation

straker
Copy link
Contributor

@straker straker commented Jan 17, 2020

Allow VirtualNode to be used in axe.commons.matches. I also added axe.utils.matches which is a VirtualNode implementation of Element.matches so we could handle CSS selectors in the matches function. It's mostly a copy paste of the QSA code with a slight modification to handle working up the parent tree for selectors. I thought about replacing our axe.utils.matchesSelector code with the new implementation, but it would technically be a breaking change as we restrict the allowed combinators to just and > as well as which pseudos we support to just :not.

axe.utils.convertExpression is just a copy paste from the QSA code to convert CSS selectors to the expression object we use in QSA and now utils.matches. I didn't want to add it or axe.utils.matchesExpression to the public API (since they are only used as shared code), but I was unsure how to have both QSA and matches share functions.

Closes issue: #1967

Reviewer checks

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

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

@straker straker requested a review from a team as a code owner January 17, 2020 15:25
@straker straker changed the title [WIP] feat(matches): use VirtualNode, add axe.utils.matches and convertSelector feat(matches): use VirtualNode, add axe.utils.matches and convertSelector Jan 20, 2020
@straker straker changed the title feat(matches): use VirtualNode, add axe.utils.matches and convertSelector feat(matches): use VirtualNode and deprecate HTMLElement Jan 20, 2020
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Couple of minor points.

lib/core/utils/matches.js Outdated Show resolved Hide resolved
assert.isTrue(
fromDefinition(node, {
fromDefinition(virtualNode.actualNode, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't you passing virtualNode itself in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the test suite is making sure it still works with actual nodes passed in (backwards compatibility)

test/commons/matches/node-name.js Show resolved Hide resolved
test/commons/matches/attributes.js Show resolved Hide resolved
@straker straker merged commit 2600a06 into develop Jan 29, 2020
@straker straker deleted the matchers branch January 29, 2020 17:43
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.

None yet

2 participants