Skip to content

fix(color-contrast): check for size before ignoring pseudo elements#3097

Merged
straker merged 8 commits intodevelopfrom
pseudo-size-test
Jul 27, 2021
Merged

fix(color-contrast): check for size before ignoring pseudo elements#3097
straker merged 8 commits intodevelopfrom
pseudo-size-test

Conversation

@WilcoFiers
Copy link
Copy Markdown
Contributor

This PR adds a pseudoSizeThreshold options, which by default will ignore any pseudo element who's size is less than 25% the area of the element who's text is being tested. This should significantly reduce the number of incomplete results axe-core reports on color-contrast.

Closes issue: #3093

@WilcoFiers WilcoFiers requested a review from straker July 26, 2021 12:28
@WilcoFiers WilcoFiers requested a review from a team as a code owner July 26, 2021 12:28
Copy link
Copy Markdown
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.

Checking width/height isn't sufficient. Width and height can be defined with left, right, top, and bottom, as well as percentage values of width and height.

::before {
  content: '';
  position: absolute;
  left: 0;
  right: 0;
  top: 0;
  bottom: 0;
}

::before {
  content: '';
  position: absolute;
  left: 0;
  width: 100%;
  height: 50%;
}

To further complicate height, a percentage value depends on there being a defined height somewhere in the parent chain for it to even work. So you'd have to figure that out and then calculate all percentages down from there.

Also, we need to take into account scale transforms or anything else that can affect the size

Comment on lines +189 to +190
const pseudoWidth = parseInt(style.getPropertyValue('width'));
const pseudoHeight = parseInt(style.getPropertyValue('height'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TODO: Check the unit here. If this isn't 'px', return Invinity. IE doesn't normalise units. They'll just have to review them.

const beforeSize = getPseudoElementArea(vNode.actualNode, ':before')
const afterSize = getPseudoElementArea(vNode.actualNode, ':after')
if (beforeSize + afterSize > minimumSize) {
return vNode // Combined area of before and after exceeds the minimum size
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you rather comment on why you decided to do them both combined rather than separate checks against the minimumSize? That would be more helpful when trying to debug this later

WilcoFiers and others added 3 commits July 27, 2021 17:14
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
@straker straker dismissed their stale review July 27, 2021 16:55

Resolved

@straker straker merged commit e0f6c0c into develop Jul 27, 2021
@straker straker deleted the pseudo-size-test branch July 27, 2021 16: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.

2 participants