Block Editor: Implement new colors hook.

[epiqueras]

Relates to #15450

Description

This PR continues to explore a new way to share Common Block Code using React hooks, see #16229 for prior art.

More specifically, it combines the functionality of the withColors HOC and PanelColorSettings with InspectorControls into an easy to use useColors hook.

To be able to reference the block edit context from within hooks, e.g. attributes and setAttributes, a few changes were made in @wordpress/block-editor's block-edit and a useBlockEditContext hook was introduced.

The new colors hook takes an array of color configuration objects for its first parameter. These objects draw inspiration from colorTypes in the HOC counterpart, but are easier to reason about, and support a few extra properties:

• name: The name of the color.
  E.g. 'backgroundColor'.

• attribute: The name of the corresponding CSS attribute in camel-case. Defaults to name.
  E.g. 'backgroundColor'.

• panelLabel: The color setting's label in the inspector panel. Defaults to startCase( name ).
  E.g. 'Background Color'.

• componentName: The name of the returned component for wrapping children with color. Defaults to panelLabel.replace( /\s/g, '' ).
  E.g. 'BackgroundColor'.

• color: An initial value for the color when one isn't set.
  E.g. '#fcba03'.

The second parameter is an optional hooks dependency array for cases where you have closures in your configuration objects, and the third is an optional string to overwrite the default panel title, __( 'Color Settings' ).

The hook will return a color component for each configuration object and a single inspector controls panel preconfigured for all the configuration objects passed. If you want to break up your colors into multiple panels, you can just break up your array of configuration objects into different calls to the hook as each call will return a different inspector controls panel.

Color components merge all of their top level children's style props with the relevant attribute and its current value, giving preference to current props. E.g. style: { color: attributes.textColor, ...style }. They also have static properties color and setColor, for directly reading and setting the color's value respectively.

Here's a simple example:

const MyColorfulHeadings = ( { attributes: { textColor }, setAttributes } ) => {
	const { TextColor, InspectorControlsColorPanel } = useColors(
		[ { name: 'textColor', attribute: 'color' } ],
		[]
	);

	TextColor.color; // `textColor`
	TextColor.setColor; // `( newColor ) => setAttributes( { textColor: newColor } )`

	return (
		<>
			{ /* This sets up the panel in the inspector. */ }
			{ InspectorControlsColorPanel }

			<TextColor>
				{ /* Color is applied to all children here. */ }
				<h1>Heading</h1>
				<h2>Heading</h2>
				{ /* Custom components work too! */ }
				<RichText />
			</TextColor>
		</>
	);
};

A real life example can be seen in this commit, aaf3e64, where I swap the HOC usage for the new hook in the heading block's edit function. Of course, if we decide to land a change like that, we would need to add a block deprecated entry and modify the new block save appropriately. It might make more sense to just use the new hook going forwards, but we should also consider that the current HOC breaks validation in a couple of ways. Namely, when settings are not available during validation, #16489, and when settings change.

New tests and documentation should be written once the approach is validated.

How has this been tested?

The color functionality of the heading block's edit mode was tested and it worked as expected.

Once the approach is validated, proper tests need to be written.

Types of Changes

New Feature: Add a new useColors hook to @wordpress/block-editor for making it easier to build blocks with color functionality.

New Feature: Add a new useBlockEditContext hook to @wordpress/block-editor for enabling the development of hooks that access block edit context, e.g. attributes and setAttributes.

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, thank you for going deep into this task. It seems the most significant simplification this new API brings is that the users don't need to create their own PanelColorSettings component, blocks still need to repeat some code on the save function and on the attribute definitions do you think it is possible to simplify these areas?

[jorgefilipecosta]

I wonder if we should have a useColors or a useColor hook used multiple times.

[epiqueras]

Plural, because we want one panel inspector controls per list of colors, not per color.

[jorgefilipecosta]

I guess we may have a memory leak here, each time setAttributes changes, we memoize another object, never cleaning old references.
Do we really need this memoize? The setters are already being created inside a useMemo hook.

[epiqueras]

Yes, otherwise a single attribute change would recreate all setters. setAttributes shouldn't change often and we can always set a maxSize.

[jorgefilipecosta]

I guess we don't need to assume the panel is nested inside InspectorControls and we can left the choice for the user.

[epiqueras]

Where else would you render it?

[jorgefilipecosta]

In a placeholder for example as part of a flow to initialize a block.

[epiqueras]

I would rather expose that under InspectorControlsColorPanel.ColorPanel. I think the inspector usage is the most common.

[jorgefilipecosta]

I guess we have a memory leak each time the attributes change we memoize a different version, never cleaning old memoizations.

[epiqueras]

I'll set a maxSize.

[jorgefilipecosta]

We need to verify if we should set the named color attribute or the customColor attribute. Currently, there is a regression if I set the text color to a custom color I get the following markup:

<!-- wp:heading {"textColor":"#175d7d"} -->
<h2 class="has-175-d-7-d-color">fdgdfgdfgdf</h2>
<!-- /wp:heading -->

It is not possible to use a single attribute e.g: red is a valid custom color string but may also be a named color with a slightly different "red".

[epiqueras]

Why do we need a differentiation between custom and not custom colors? It just makes validation fail and makes the usage of the colors API more complicated.

[jorgefilipecosta]

I may set a custom color of "red" (is a valid CSS color), but also may have a named color "red" (or a previous theme may have had it in the past), how can I know if should set an inline style or use a class?

[epiqueras]

We should stick to inline styles for this. Otherwise changing themes/color settings invalidates blocks.

[jorgefilipecosta]

The reason the named colors were introduce was to avoid inline styles. We want to avoid styles as much as possible. Currently, changing themes/colors do not invalidate blocks, the named colors may be applied if the new theme does not define classes for them but that was something we wanted as the colors set when an old theme was used may not make sense in the new theme.

[jorgefilipecosta]

The Pullquote problem is an exception because even for some named colors we want to use inline styles (for border styles which theme may not have defined as class) and depending on the block style the same color may be a border style (inline) or a background style (class), so the block works in different ways, the problem does not happen in other blocks.

[jorgefilipecosta]

I guess the correct way to implement that functionality on pullquote would have been to have different attributes one for border and other for the background and depending on the block style we should options to pick one color or the other.
The problem with this other approach would have been that when changing styles the border-color would not be a background color right away and the previews also would not reflect the chosen colors.
We should think about ways to deal with the problem but it is a special situation and I'm not sure the API's we offer need to take that case in consideration.

[epiqueras]

Yeah, but why not prevent developers from shooting themselves in the foot if we can?

I think that the fact that this happens is a code smell.

[jorgefilipecosta]

The use of classes provides interesting use cases.
E.g.: if my site uses two colors primary and secondary right now I can change all the primary colors and secondary colors around my website including existing blocks by changing the color value in the class and in the palette.
How can we do something similar now and globally change colors on the websites?
People build interesting mechanisms, e.g., allow users to globally change block colors in all the blocks of the website from the customizer
https://richtabor.com/gutenberg-customizer-colors/.
Can we keep back-compatibility with these solutions when we use this hook in the paragraph and button blocks?

[epiqueras]

That behavior belongs in a different API. Certain things have to be done differently to avoid invalidations when you use classes and I'd rather not overcomplicate a single hook when we can have a separate useThemeColor or something like that.

[jorgefilipecosta]

We should allow the hook users to add children that appear inside PanelColorSettings and these children them selfs may depend on the color values. E.g: it is very common to add contrast checking UI inside this panel.

[epiqueras]

Good idea.

[youknowriad]

Can't we back contract checking into the hook somehow?

[epiqueras]

back contract checking

Not sure what that means.

[epiqueras]

Oh got it.

[epiqueras]

Yeah we could, make it an option and turn it on by default.

[epiqueras]

I'll add it to the "next steps".

[jorgefilipecosta]

It seems users of the hook can not use their own color palette, currently, our HOC's provide that option and table block uses it.

[epiqueras]

Yeah we can support that as well. I didn't want to make the initial prototype harder to read so I minimized functionality to the most common use case.

[jorgefilipecosta]

Should we implement this hook as experimental (at least for one release)?

[epiqueras]

Sure

[youknowriad]

Maybe instead of making it experimental it could be just a utility used in the block library module for now, we don't have to expose it?

[epiqueras]

Wouldn't it get more feedback as experimental?

[youknowriad]

Are we interested in third-party feedback now? I think we should first use it in our blocks (multiple ones). I don't feel strongly though.

[epiqueras]

Are we interested in third-party feedback now?

Why not?

[epiqueras]

Hi @jorgefilipecosta , thanks for the quick review! 😄

blocks still need to repeat some code on the save function and on the attribute definition do you think it is possible to simplify these areas?

There isn't much we can do for the save function, but we could look at hijacking the attributes definitions although I don't know if that would play nicely with server-side block registration.

Next Steps:

• Add has-x-color class names.
• Avoid memory leaks: Block Editor: Implement new colors hook. #16781 (comment).
• Bake in optional contrast checking.
• Explore hijacking attributes definitions.
• Expose InspectorControlsColorPanel.ColorPanel.
• Mark hook as experimental.
• Support children inside the panel.
• Support custom color palettes.

[youknowriad]

I feel deps should always be the last param (to match react) and maybe instead of paneTitle we could have a config object (as a way to add other customizations if needed)

[epiqueras]

I feel deps should always be the last param (to match react)

It's more about which parameters are used more often. Here you would have had people always passing undefined, because they wanted the default panelTitle.

we could have a config object (as a way to add other customizations if needed)

Now that we need children inside the panel and optional contrast checking, I will make it an object and make deps go last as some of the object's properties might also have dependencies.

[youknowriad]

I'd have thought that sometimes it should be classname for the color palette colors. Why it's just style?
Also as mentioned privately, I feel this component adds more complexity that it solves compared to just returning the props that should be applied to any wrapper the block author decides to apply the color for.

[epiqueras]

I'd have thought that sometimes it should be classname for the color palette colors. Why it's just style?

the current HOC breaks validation in a couple of ways. Namely, when settings are not available during validation, #16489, and when settings change.

This is because of using class names. We should avoid it.

Also as mentioned privately, I feel this component adds more complexity that it solves compared to just returning the props that should be applied to any wrapper the block author decides to apply the color for.

They can still do that by accessing .color or .setColor. This just saves a lot of code for the most common use case.

[youknowriad]

I'm not sure I understand properly here but using classNames is an important choice we did a long time ago and I don't think we can/should revert to inline styles here. That's one of the main purposes of the color palettes.

[epiqueras]

See #16489

[youknowriad]

That's not a reason to abandon classNames though? we should just make sure they're available right?

[epiqueras]

Why add that extra complexity and implicitly make the block editor depend on the editor which is something we wanted to avoid?

Styles are meant to be used for cases like this where values are dynamically calculated.

[jorgefilipecosta]

I provided more context to the pullquote problem in #16781 (comment).

[youknowriad]

Should this be added automatically by the hook?

[epiqueras]

Is it common? I haven't seen it much.

[jorgefilipecosta]

Not all the blocks may want these classes to be added, I think blocks should add them explicitly if they need them.

[youknowriad]

Consistency is good IMO. I saw some people asking for this kind of classes. Could be opt-out. At least we should think about it.

[epiqueras]

It doesn't hurt to add them.

[jorgefilipecosta]

Hi @epiqueras, the hook handles the addition of has-text-color class right? Can we remove it from the block?

[epiqueras]

Yes, good catch!

Done.

[jorgefilipecosta]

Using a maxSize is not a perfect solution if we have many blocks that need to be rerendered at the same our cache may be invalid when it should not (setAttributes is specific per block).
What if in our hook before the current useMemo we have another useMemo that depends on setAttributes and colorCofig and returns an object whose keys are the name and whose value is the setter function? Then when we need the setter we can just retrieve it from that object.

[epiqueras]

That would recreate all setters when only one needs to change.

I can just create the memoized function inside the hook so that every instance gets its own cache, with a maxSize of colorConfigs.length, and clear it on unmount.

[epiqueras]

• Add has-x-color class names.
• Avoid memory leaks: Block Editor: Implement new colors hook. #16781 (comment).
• Bake in optional contrast checking.
• Explore hijacking attributes definitions.
• Expose InspectorControlsColorPanel.ColorPanel.
• Mark hook as experimental.
• Support children inside the panel.
• Support custom color palettes.

Done! 🚀

[youknowriad]

How do you decide which color from the first argument to use in the color contrast checks?

[epiqueras]

It checks all of them.

[jorgefilipecosta]

I think we should not check the contrast of all the colors. Imagining a scenario where we use the hook with background color and text color, the contrast checked should not check the background color.
Another point is that I think we should not hardcode a color for contrast checking (currently we assume the background is always white). Themes may set the heading background to block, and in that case, white text color is perfectly fine.
I think users should be able to configure the color pairs used in contrast checking.
We currently also have some mechanisms to retrieve colors using browser computed styles functions when they are not set. This mechanism will need to be improved, e.g., it was created before nesting existed and didn't handle it. As an example of the imporvement needed for nesting, If we have a heading with black color inside a group block with block background color if the theme does not explicitly set a heading background color we should show a contrast warning.
Having all contrast functionality in this hook makes it very huge. I think the contrast checking should be outside of this hook, going in the direction of what @mcsf referred (multiple hooks) the developers use useColor for the color logic useContrast for the contrast logic.
The contrast hook can contain the logic of the fallback styles we currently have, and receive some color values retrieved from useColors and simply returns a warning message or null.

[epiqueras]

I think we should not check the contrast of all the colors. Imagining a scenario where we use the hook with background color and text color, the contrast checked should not check the background color.

They can separate it into two hook calls or provide their own custom ConstrastChecker child.

Another point is that I think we should not hardcode a color for contrast checking (currently we assume the background is always white). Themes may set the heading background to block, and in that case, white text color is perfectly fine.

Nothing stops that backgroundColor property from being a variable.

This mechanism will need to be improved, e.g., it was created before nesting existed and didn't handle it. As an example of the imporvement needed for nesting, If we have a heading with black color inside a group block with block background color if the theme does not explicitly set a heading background color we should show a contrast warning.

I think that sort of contrast checking behavior belongs in another hook in another PR.

[mcsf]

Good work here. I think these are some of the present challenges:

• Making sure we aren't getting lost in the web of memoised objects and dependencies. That is, in every useCallback, useMemo or memoize context, are we nesting too deeply? Can certain things move up in the context, so that less things are exposed in their closures?
• Trying to flatten the module a bit could help shed light on this and improve maintenance. In this process we could isolate these — if I may use a term — constructors (the functions passed to useMemo) as named functions.
• Bonus question: right now useColors is one large hook mostly only relying on useBlockEditContext and stock React hooks. Can it be broken up into more (for now private) Gutenberg hooks?

In terms of interface, I think we're in a better position already, but:

• I wonder if we should learn from React and prefer smaller surfaces, e.g. how useState encourages the separation of stateful objects by separately calling [ foo, setFoo ] = useState(…); [ bar, setBar ] = useState( … ); rather than operating on larger objects such as, hypothetically, { foo, setFoo, bar, setBar } = ...; .
• While I see the advantages in immediate ease of use, there's something that feels off about the fact that, in the Heading example, TextColor and friends are completely detached from the attributes and setAttributes which are arguments of the very function we are in! I realise that those same objects are used inside useColors through the BlockEdit context, but I wonder if working so hard to provide "magic" behaviour could harm the developer experience at some point.

[mcsf]

I take it that relying on colorConfigs.length was a trade-off between the convenience for the hook consumer of not having to cache colorConfigs, while retaining some sort of relationship between the input and the memo?

I can see why it works, but it's strictly speaking not correct. Just noting it here.

[epiqueras]

For each use of the hook we need a memoized function with a cache space for each item in colorConfigs. That's the usage of the length here. It's the only dependency, the array itself is not a dependency.

[mcsf]

useCallback is likely more adequate here and in similar places. Why useMemo?

[epiqueras]

To avoid calling memoize unnecessarily.

[epiqueras]

Thanks for the review.

Making sure we aren't getting lost in the web of memoised objects and dependencies. That is, in every useCallback, useMemo or memoize context, are we nesting too deeply? Can certain things move up in the context, so that less things are exposed in their closures?

Not without adding more parameters to functions and making things harder to read.

are exposed in their closures

Not sure what that means.

Bonus question: right now useColors is one large hook mostly only relying on useBlockEditContext and stock React hooks. Can it be broken up into more (for now private) Gutenberg hooks?

Yes, but I can't see them being used anywhere else so it seems a bit premature.

I wonder if we should learn from React and prefer smaller surfaces, e.g. how useState encourages the separation of stateful objects by separately calling [ foo, setFoo ] = useState(…); [ bar, setBar ] = useState( … ); rather than operating on larger objects such as, hypothetically, { foo, setFoo, bar, setBar } = ...;

We want to be able to group colors so that they are all under the same inspector panel.

While I see the advantages in immediate ease of use, there's something that feels off about the fact that, in the Heading example, TextColor and friends are completely detached from the attributes and setAttributes which are arguments of the very function we are in! I realise that those same objects are used inside useColors through the BlockEdit context, but I wonder if working so hard to provide "magic" behaviour could harm the developer experience at some point.

I don't think it's magic, the whole point of having this utility is to not have to use attributes directly.

[jorgefilipecosta]

The combination of memoize with useMemo works and should be performant, but it took me some time to understand it.
Can we extract the setters and components creation to its own useMemo?
e.g: something like:

const colorSetters = useMemo(
	() => {
	return reduce( colorNames, ( accumulator, colorName ) => {
		accumulator[ colorName ] = ( newColor ) => setAttributes( { [ colorName ]: newColor } 			);
	} ),
	[ colorNames, setAttributes ]
);
const colorComponents = useMemo(...

Color colorNames would porbably come from another small useMemo hook.

We then would have a final useMemo that depends on colorSetters, colorComponents and probably colorConfig and generates the final object.
This approach probably has the disadvantage that as the colorComponents will depend on color value if the value of color changes all the colorComponents will be regenerated instead of just the color component of the color that changed, but I don't think this has a noticeable performance impact and the code simplification it brings may worth it.

[epiqueras]

I don't see how that code would look simpler. If anything, it creates more code and a soup of hook dependencies which are already not the easiest to understand.

[youknowriad]

I don't think we need the changes in this file, since you can access clientId from the context, you can call updateBlockAttributes and getBlockAttributes without extra context values.

[epiqueras]

That wouldn't subscribe the consumer to changes though, right?

[youknowriad]

useSelect would

[epiqueras]

Oh right. I was thinking of the Block API functions. I'll change it.

[epiqueras]

It just feels a lot clunkier to have to use the context to get the id and then pass it to useSelect.

Don't you think a useBlockEditContext with attributes and the setter will be very useful for people building custom hooks that need access to attributes?

[youknowriad]

I don't think so personally, I feel like the context should contain the minimum possible things because

• these are new APIs
• semantically speaking, the context says, we're editing that block (id) and it should be enough for the consumer.

if we add these, the question becomes: Will we add a new value there each time we need another selector/action?

[epiqueras]

Will we add a new value there each time we need another selector/action?

I don't think we could, because it would be limited to the props that BlockEdit receives. I just tend to favor context over selector/subscription boilerplate, but I can see why this aligns more with the rest of the codebase and see the value in having a single way of doing things:

69865dd

[youknowriad]

Can we find a path forward here @jorgefilipecosta @mcsf @epiqueras ?

Since it's experimental, it doesn't have to be perfect from the start. I'd like us to follow-up on this, try to use it in several blocks and maybe expand to other properties after that (gradients, font sizes...)

[jorgefilipecosta]

Hi @epiqueras I did a new set of tests, and I noticed the following bugs:

When I select a color, the color selected UI (with a right mark) does not appear above the selected color.

When I select a preset color and then open the custom color panel, the value by default is not the one I select from the preset. The value is a random one (making small color adjustments much harder). Maybe related to the previous bug, where it assumes there is no color selection.

When I select a preset color, the color indication always says I selected the
white color:

When I select a custom color, there is no color indication at all.
It also may be related to the first bug.

The contrast checker only triggers warnings for preset colors, if I select a custom color without enough contrast the warning does not appear:

[jorgefilipecosta]

Should on the heading we use a hardcoded value for background on contracts checking of white? By default, the default WordPress theme does not contain a white background.

[epiqueras]

Right, I changed it to detect the background of the nearest parent that has one.

[jorgefilipecosta]

We have a small function that computes the class name from the color slug getColorClassName could we use it here?

[epiqueras]

That one works slightly differently. It takes a kebab cased context name and the slug. Here we have both the color and attribute name in camel case and need to convert one or both depending on whether it's a custom color or not.

[jorgefilipecosta]

Should color config be a dependency here? colorConfig may totally change the expected output of the component. I guess we did not added it as a dependency because if users pass an array inline on each re-render a new reference is passed, could we automatically generate the dependencies from the config e.g: generate an array of [ name, attribute, name, attribute ...], If we also pass all properties of panel setting props we could avoid the need for dependencies.

Currently, we are relying on devs passing the dependencies and I think it will be something developers will easily miss.

[epiqueras]

colorConfig shouldn't be dynamic in most cases. If it is, they can use deps to invalidate it. This is a better DX than having to memoize colorConfig for all simple use cases.

[jorgefilipecosta]

At the start, our colors API explicitly made block developers identify what was the attribute and what was the CSS property:

gutenberg/core-blocks/button/index.js

Lines 93 to 94 in b8ee131

	colorContext: 'color',
	colorAttribute: 'textColor',

People provided feedback in reviews that maybe we could just use something in this format { textColor: 'color' }, where the key represents the Gutenberg attribute and the value of the CSS context/property where the color is used. Would not this format fit here? What were the reasons for the change and go back to explicitly identify the Gutenberg attribute and CSS property?

We have functions and docs where we call the CSS property 'color' the color context. And currently, some code that exists outside of the hook calls 'textColor' colorAttributeName. I think naming 'color' attribute may be confusing. Could we keep the current name 'context'?

[epiqueras]

{ textColor: 'color' } is super confusing and not self explanatory.

color here is an attribute, it's not context. I think it makes a lot more sense to stick to the terminology that the web platform uses.

[jorgefilipecosta]

Hi @epiqueras, The problem is that the term attribute in Gutenberg has a very specific meaning. I think people will get confused.
On the web, we normally call "color", "background-color" CSS properties. I think if we call "property" we still stick to the terminology that the web platform uses while removing a source of possible confusion for developers.

[epiqueras]

You're right, property makes more sense.

[jorgefilipecosta]

It seems this component would also be useful in the save function as the classes/styles we should add follow the same logic. Do you think it would be possible to have something identical there?

[epiqueras]

Yeah, that would be nice, but first we need to mock the React dispatcher in our serializer so that hooks don't throw errors.

[epiqueras]

I did a new set of tests, and I noticed the following bugs:

All fixed now 😄!

[jorgefilipecosta]

Should we add this export also to the native version of the file packages/block-editor/src/components/index.native.js?

Otherwise, headings on mobile will break.

[epiqueras]

Yes, good catch!

[jorgefilipecosta]

Thank you for iterating and fixing all issues 👍 I confirm that everything seems to be working as expected now.
I left two suggestions one related to mobile, and one related to calling property instead of attribute to decrease the chances of confusion with Gutenberg attributes. I leave the final decision on the suggestions to you.
I think this is ready to merge. Awesome work during all these iterations, and thank you for bringing this to the finish line!

[jorgefilipecosta]

Nice improvement 👍

[epiqueras]

I left two suggestions one related to mobile, and one related to calling property instead of attribute to decrease the chances of confusion with Gutenberg attributes. I leave the final decision on the suggestions to you.

I've implemented both.

I think this is ready to merge. Awesome work during all these iterations, and thank you for bringing this to the finish line!

Thanks for all the reviews!

[youknowriad]

Good work, let's try to open follow-up PRs to use in other blocks to see how it behaves, if it's good enough.

[ellatrix]

What does <TextColor> do? This doesn't seem to be documented anywhere.

[youknowriad]

It applies the text color as inline style to the wrapped component.

[ellatrix]

Why not pass the style directly to the element?

[epiqueras]

It's less verbose this way.

[youknowriad]

I'm on the fence personally, if it's just about "style", I guess being explicit is better (passing styles). But I believe it also handles classNames and merging these props which ultiimately can result in a lot of duplicated code.

[ellatrix]

It looks a bit like magic right now. It's not very obvious what it does. I'll look around sometime to see how it works.

[epiqueras]

But I believe it also handles classNames and merging these props which ultiimately can result in a lot of duplicated code.

Yes

[youknowriad]

Can you explain Why did we add this new behavior? (or where was it handled before?
also, is this something that can be absorbed by the hook somehow? Is this specific to the heading block.

[epiqueras]

Can you explain Why did we add this new behavior?

For the background value of the contrast checker.

or where was it handled before?

The Heading block did not have a contrast checker.

is this something that can be absorbed by the hook somehow? Is this specific to the heading block.

No it's not specific, it could potentially be useful for any block that changes text colors, but uses a parent background.

I think that if the contrast checker config is provided to useColors without a background value, the hook should somehow default to doing this parent traversal, what do you think?

[youknowriad]

I think that if the contrast checker config is provided to useColors without a background value, the hook should somehow default to doing this parent traversal, what do you think?

This means that it will be automatically handled by the hook right? I think that's better. I wonder if we need a way to disable it if needed?

[epiqueras]

Yes, what do you think about this proposal?

#18148 (comment)

[epiqueras]

See #18237.