fix(color-contrast): properly blend multiple alpha colors#3193
Merged
fix(color-contrast): properly blend multiple alpha colors#3193
Conversation
WilcoFiers
requested changes
Oct 8, 2021
WilcoFiers
approved these changes
Oct 8, 2021
Contributor
WilcoFiers
left a comment
There was a problem hiding this comment.
I'm good with that, assuming you do create that tech debt ticket before we merge.
Contributor
Author
|
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
This was referenced Jan 29, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I refactored the
flattenColorsfunction 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 frommix-blend-modeCSS property if we ever decide to support that.Changing
flattenColorsadversely 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 oldflattenColorscode and named itflattenShadowColorsas @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
dataproperty 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.
With the changes to reverse the blending order we now produce the correct colors.
Closes issue: #2924