Use Colors: Enhance contrast checking API.

[epiqueras]

Follows #16781
Helps #18148

cc @youknowriad @jorgefilipecosta

Description

This PR enhances the contrast checker API in the __experimentalUseColors hook to fit all possible use cases pertaining a hook call's colors and refactors the heading block to take advantage of it.

See #18148 (comment) and #16781 (comment) for the discussions that led to this PR.

To summarize, the current API supports a contrastCheckerProps option that takes a set of props to render multiple identical contrast checkers for each color managed by the hook, where the text color is set to the edited color attribute and the background color is expected to be part of the passed in props. This is insufficient for a lot of use cases. What if you need the edited color attribute to be the background color instead? What if you need the background and text colors to both be edited color attributes? What if you don't want a contrast checker for every edited color attribute? All of this are common use cases, and furthermore, it will be pretty common, as was required by the heading block, to have the background color in a contrast checker be the actual DOM background color of the nearest parent element with a set background. This was also catered to.

This is the new API:

const { TextColor, InspectorControlsColorPanel, BackgroundColorDetector } = __experimentalUseColors(
	[ { name: 'textColor', property: 'color' } ],
	{
		contrastCheckers: contrastCheckersOptions,
	},
	[]
);

contrastCheckersOptions can be a single object or an array of objects. If it's an object, its values will be used to generate a contrast checker per edited color attribute as before, if it's an array of objects, a contrast checker will be created per item in the array.

The properties of the objects are the props for the contrast checker with backgroundColor and textColor handled a bit differently than any other props that will just get passed through as is.

Single Object Form

// You can pass in literals.
// `textColor` would default here to the edited color attribute for each color.
const contrastCheckersOptions = { backgroundColor: 'white', ...maybeOtherProps };

// You can pass in functions that take the color panel settings
// to get the color value of another edited color attribute.
const contrastCheckersOptions = {
	backgroundColor: ( { BackgroundColor } ) => BackgroundColor.value
};

// `true` means resolve to nearest parent background color.
const contrastCheckersOptions = { backgroundColor: true };

// You can also pass in literals or functions for `textColor` and
// have `backgroundColor` default to the edited color attribute for each color.
const contrastCheckersOptions = { textColor: 'white' };
const contrastCheckersOptions = {
	textColor: ( { TextColor } ) => TextColor.value,
};

Array of Objects Form

// You can pass in literals, functions, or `true`, but you need to specify both
// `backgroundColor` and `textColor` for each contrast checker as they
// are not mapped from edited color attributes here.
const contrastCheckersOptions = [
	{ backgroundColor: 'white', textColor: ( { TextColor } ) => TextColor.value },
	{ backgroundColor: true, textColor: ( { TextColor } ) => TextColor.value },
	// ...etc, etc.
];

Detecting Parent Background Color

To detect the background color for the true options, the hook now returns a BackgroundColorDetector component that you will need to render somewhere in the consuming component to get a handle to the right area of the DOM tree. See the heading block for an example of this.

How has this been tested?

The heading block was refactored to use the new API and it was verified that it works as expected.

Types of Changes

New Feature: The __experimentalUseColors contrast checking API now supports more use cases.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[jorgefilipecosta]

Hi @epiqueras, I did some tests and it looks like currently, we have a bug. If we nest the heading inside a group and set background for the group, then set a color for the heading without enough contrast the warning appears, if later we change the background color of the group to somethiong with enough contrast the warning of the heading does not disappear.

I think having a contrast checker that checks ancestors is complex.
In theory, an option would be to use getComputedStyle on every rerender (the background of an ancestor may have changed). I think doing that would have a noticeable performance impact, e.g: we would do that on every character typed.
Another option would be to on the useColors hook add background colors to a context passed to descendants. Then the contrast checker mechanism would retrieve fallback colors each time the non-undefined value of the background color of the closest ancestor changes.

But both approaches are not perfect, on the second if we have a simple parent block that has background colors but does not use the useColors e.g: it uses a simple toggle to switch the background between black and white our contrast checking mechanism would not update correctly for the blocks nested inside.

For the first idea, it also does not works in cases where gradients and images are used as backgrounds. Excluding that it works well, unfortunately, I think it is not an option because of needing to execute on every rerender.

I wonder if a contrast checking with multiple levels would be that useful and the additional complexity worths it? Maybe it is ok just to have a very simple mechanism that just checks one level, and executes each time the colors of the block change?

[epiqueras]

Yeah, neither of those solutions is acceptable. I made it a bit better by rerunning the detection when the attributes, and therefore the colors, of the block change.

I don't think this is a blocker for this PR since we already have this problem on master.

[jorgefilipecosta]

I'm noticing a bug. I did the following steps to reproduce it:
Added a heading block.
Selected a white background color.
A contrast warning appeared.
I saved the post and reloaded, selected the heading block, and verified the contrast warning disappeared.

[jorgefilipecosta]

Doing this, we are calling getComputedStyle on every attribute change, e.g., every character typed. Could we depend on the color-related attributes only?

[epiqueras]

Done!

[jorgefilipecosta]

Our background detector component is a sibling of the heading dom element, e.g., h2.
I expect that when themes set a style for a heading, they would do something like:

h2 {
	background-color: red;
}

Given that our background detector node is not the heading node, the background of the heading would not be detected, and we may be throwing a warning when we should not and vice versa.

In a block, it is hard to know the node from which fallback colors should be computed. withFallbackStyles passed a reference node, but allowed each block to specify from that node where would the fallback colors be retrieved. The paragraph block, for example, did the following to retrieve the paragraph node based on the reference node const editableNode = node.querySelector( '[contenteditable="true"]' );.
On the button block, the background and color nodes are even different from each other.

What if instead of returning a BackgroundColorDetector, we allow the parent to pass a reference to the component that should be used to detect the background of a given color?
I guess it would make this functionality more versatile -- If a block has two areas with different background colors, it is still possible to use the background detection. We can pass different refs on each contrast checker.

[epiqueras]

Good points here. We need to account for those use cases.

However, I don't like the idea of having users manage refs directly.

I solved it by adding a querySelector prop to the detector. This also fits all use cases.

[jorgefilipecosta]

We are calling withFallbackStyles, even if the block does not use the detection functionality or the contrast functionality at all. Given that the reference is undefined, the withFallbackStyles will not do much. But, I still wonder, could we make these calls only if the background detection functionality is used?
We would an auxiliary component given than hooks always need to be called in the same order.

[epiqueras]

Not in any straightforward way, because we want to return a component, not an element so that it can take props easily.

I made it so that withFallbackStyles is only called when needed so that we avoid the potentially expensive part which is creating the component.

[epiqueras]

I'm noticing a bug. I did the following steps to reproduce it:
Added a heading block.
Selected a white background color.
A contrast warning appeared.
I saved the post and reloaded, selected the heading block, and verified the contrast warning disappeared.

Great catch! It's fixed.

[jorgefilipecosta]

Hi, @epiqueras thank you for the fixes, I confirm the problem is fixed in my tests.
I rebased this PR, and I rechecked the changes trying a paragraph implementation. I noticed that in the paragraph, we also do the text color detection, I would assume detecting text color is equally important as the user may change the background without changing the text color. Using these hooks as it is now in the paragraph, we would lose the color detection functionality. Do you think it is possible to add color detection functionality now, or you would prefer to try that in a follow-up PR?

[epiqueras]

Let's do it in a follow up.

[jorgefilipecosta]

Nice to see useColors expanding into contrast checking I think we can merge this and iterate in follow up PR's.
Things I would like to see in the future are:

• Include color detection (a blocker to having the hook with contrast checking in the paragraph and button).
• Document the hook so when building new blocks people can try this experimental hook.
• Ideally, add some test cases.

The contrast checking checks ancestors background-color, but we did not found yet solution to know when we should retrieve again the fallback colors. I think if we don't find a good solution maybe we should not check ancestor colors by default and maybe have a flag that allows to do it.

But we can keep the follow-up tasks in separate PR's.
For now let's get this in, nice work 👍

[epiqueras]

Thanks for all the reviews here!

I think if we don't find a good solution maybe we should not check ancestor colors by default and maybe have a flag that allows to do it.

Do you mean making the true option more explicit? Something like detect.