fix(color-contrast): improve speed and accuracy of code blocks with syntax highlighting #2003
Conversation
This file contains 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
straker
changed the title
straker
commented
Jan 29, 2020
| @@ -14,8 +14,8 @@ describe('color.getForegroundColor', function() { | |||
|
|
|||
| it('should return the blended color if it has alpha set', function() { | |||
| fixture.innerHTML = | |||
| '<div style="height: 40px; width: 30px; background-color: #800000;">' + | |||
| '<div id="target" style="height: 20px; width: 15px; color: rgba(0, 0, 128, 0.5);' + | |||
| '<div style="height: 40px; background-color: #800000;">' + | |||
There was a problem hiding this comment.
I had to update these tests as my code changes made them more accurate than before. Before my changes, we would test only the first line of this fixture. However, if we look at the result of the fixture, we can see that there should be 3 background colors and 3 foreground colors, but we only calculate 1.
So I updated the tests to just focus on the single color with alpha/opacity.
WilcoFiers
requested changes
Jan 31, 2020
WilcoFiers
approved these changes
Jan 31, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Code blocks with syntax highlighting could have large amounts of client rects, which would then cause it to take a few seconds each to process. Code blocks could also cause us lots of incomplete results with "partially obscured by another element."
The code changes fix the speed by focusing only on the client rects of the text nodes of the element rather than all client rects. Since each of the child nodes that have text nodes will be checked individually, there is no need for the main parent to also check those nodes as well. This reduced the large code block from 2000+ client rects to just <20, taking color contrast from running in 10+ seconds to <1 seconds.
It also adjusts the collision check to account for elements which lie exactly on the middle of the element being checked. This solved all the incomplete issues and they now return as proper pass/fail.
Closes issue: #1985
Reviewer checks
Required fields, to be filled out by PR reviewer(s)