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(color-contrast): properly blend multiple alpha colors #3193

Merged
merged 7 commits into from
Oct 8, 2021

Conversation

straker
Copy link
Contributor

@straker straker commented Oct 6, 2021

I refactored the flattenColors function to use the exact algorithm found in the spec and left LOTS of comments so we wouldn't have to dig that up again and figure it out. I also have it set up to support blending modes from mix-blend-mode CSS property if we ever decide to support that.

Changing flattenColors adversely affected the text-shadow tests, so instead of trying to fix 100+ shadow tests (trying to determine if the new blending should pass/fail the color contrast) I just added back the old flattenColors code and named it flattenShadowColors as @WilcoFiers did all the testing for that one to make it work. This makes it so the shadow colors are flattened how they use to be. We can add a tech debt ticket to fix that if you think we need to.

Lastly, I created an integration file that verifies the blending is correct. The integration test sets up a stack of different color alpha layers and only runs color contrast on the last layer (which has the text). It then grabs the axe-core computed background color from the nodes data property and makes that the background color of the result node. This way we can compare how axe-core computes the color to what the browser does.

Before this change, our code would correctly blend 2 alpha layers (since order didn't matter), but would fail to blend correctly at 3 or more, as shown in the screenshot below.

Screen Shot 2021-10-06 at 2 06 12 PM

With the changes to reverse the blending order we now produce the correct colors.

image

Closes issue: #2924

@straker straker requested a review from a team as a code owner October 6, 2021 20:12
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.

I'm good with that, assuming you do create that tech debt ticket before we merge.

@straker straker merged commit e930a70 into develop Oct 8, 2021
@straker straker deleted the color-contrast-blending branch October 8, 2021 15:33
@straker
Copy link
Contributor Author

straker commented Oct 8, 2021

Opened #3199

straker added a commit that referenced this pull request Oct 18, 2021
* fix(color-contrast): properly blend multiple alpha colors

* revert shadow

* add back shadow color flatten

* fix

* fix ie11

* type

* typo
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