-
Notifications
You must be signed in to change notification settings - Fork 791
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): correctly compute background color for elements with opacity #3944
Merged
Merged
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
81e2a01
fix(create-grid): correctly compute stack order for non-positioned st…
straker 46d3832
chore: Refactor createStackingOrder (#3932)
WilcoFiers e269217
use treewalker nodeIndex
straker b8bfb29
no floatpoint precision errors
straker f40d21a
pass nodeIndex to createGrid for shadowDom
straker b160c3d
do it properly
straker 0cd2b0d
remove whitespace
straker 08c7b09
fix(color-contrast): correctly handle opacity on parent elements
straker 2137075
working
straker 4678676
idea finalized
straker ea6bb04
remove comments
straker 186723c
move function
straker e71dbb0
Merge branch 'develop' into color-contrast-opacity
straker 2e43a29
tests!
straker 5ad2ea3
fix comments
straker d29a302
fix stuff
straker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import flattenShadowColors from './flatten-shadow-colors'; | |
import getTextShadowColors from './get-text-shadow-colors'; | ||
import getVisibleChildTextRects from '../dom/get-visible-child-text-rects'; | ||
import { getNodeFromTree } from '../../core/utils'; | ||
import { getStackingContext, stackingContextToColor } from './stacking-context'; | ||
|
||
/** | ||
* Returns background color for element | ||
|
@@ -47,59 +48,57 @@ export default function getBackgroundColor( | |
} | ||
|
||
function _getBackgroundColor(elm, bgElms, shadowOutlineEmMax) { | ||
const elmStack = getBackgroundStack(elm); | ||
if (!elmStack) { | ||
return null; | ||
} | ||
|
||
const textRects = getVisibleChildTextRects(elm); | ||
let bgColors = getTextShadowColors(elm, { minRatio: shadowOutlineEmMax }); | ||
if (bgColors.length) { | ||
bgColors = [{ color: bgColors.reduce(flattenShadowColors) }]; | ||
} | ||
|
||
const elmStack = getBackgroundStack(elm); | ||
const textRects = getVisibleChildTextRects(elm); | ||
|
||
// Search the stack until we have an alpha === 1 background | ||
(elmStack || []).some(bgElm => { | ||
for (let i = 0; i < elmStack.length; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By turning this into a loop instead of a |
||
const bgElm = elmStack[i]; | ||
const bgElmStyle = window.getComputedStyle(bgElm); | ||
|
||
if (elementHasImage(bgElm, bgElmStyle)) { | ||
bgColors = null; | ||
bgElms.push(bgElm); | ||
|
||
return true; | ||
return null; | ||
} | ||
|
||
// Get the background color | ||
const bgColor = getOwnBackgroundColor(bgElmStyle); | ||
if (bgColor.alpha === 0) { | ||
return false; | ||
continue; | ||
} | ||
|
||
// abort if a node is partially obscured and obscuring element has a background | ||
if ( | ||
bgElmStyle.getPropertyValue('display') !== 'inline' && | ||
!fullyEncompasses(bgElm, textRects) | ||
) { | ||
bgColors = null; | ||
bgElms.push(bgElm); | ||
incompleteData.set('bgColor', 'elmPartiallyObscured'); | ||
|
||
return true; | ||
return null; | ||
} | ||
|
||
// store elements contributing to the bg color. | ||
bgElms.push(bgElm); | ||
const blendMode = bgElmStyle.getPropertyValue('mix-blend-mode'); | ||
bgColors.unshift({ | ||
color: bgColor, | ||
blendMode: normalizeBlendMode(blendMode) | ||
}); | ||
|
||
// Exit if the background is opaque | ||
return bgColor.alpha === 1; | ||
}); | ||
|
||
if (bgColors === null || elmStack === null) { | ||
return null; | ||
if (bgColor.alpha === 1) { | ||
break; | ||
} | ||
} | ||
|
||
const stackingContext = getStackingContext(elm, elmStack); | ||
bgColors = stackingContext.map(stackingContextToColor).concat(bgColors); | ||
|
||
const pageBgs = getPageBackgroundColors( | ||
elm, | ||
elmStack.includes(document.body) | ||
|
@@ -166,6 +165,7 @@ function fullyEncompasses(node, rects) { | |
function normalizeBlendMode(blendmode) { | ||
return !!blendmode ? blendmode : undefined; | ||
} | ||
|
||
/** | ||
* Get the page background color. | ||
* @private | ||
|
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took this opportunity to do something I've been wanting to do for awhile now which is convert the
Color
constructor function into a true class. The advantage to this was that I was able to dodeepEquals
on two Color objects whereas before that was not possible (thus greatly simplifying the tests). Another advantage is that all Color objects now share memory for all their functions thanks to the prototype whereas before each Color object had their own memory instance for each function.