Prototype: look Appearance font faces up from the family a block's text is drawn in - #83462

🤖 PR meta 🤖

🏷️ Labels

This pull request needs exactly one label indicating its type, and has 0.

• Required: any label starting with [Type].
• Found: none.

Read more about Type labels in Gutenberg. If you cannot add labels yourself, a reviewer can do it for you.

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Jiwoon-Kim <kimjiwoon@git.wordpress.org>
Co-authored-by: jahirulwp <jahirulwp@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Thanks for picking this up! I tested this PR, and I have one suggestion.

Testing

• Checked out a215bb9f (Node 24.21). The new typography-attributes-to-style tests pass. The related suites also pass: the block-editor global-styles tests (including the Typography panel browser tests) and all of global-styles-ui. That's 21 files and 2409 tests.
• I also wrote a browser test that renders ScreenBlock with a config modeled on Twenty Twenty-Four: body text in Inter (300 900), and headings in Cardo (400, 400 italic, 700) set on styles.elements.heading. That's how TT4 ships its fonts.

So Paragraph is fixed, but Heading still gets the wrong list. screen-block.tsx only reads the root typography.fontFamily, while a Heading's text is painted by elements.heading. Based on getElementLayers() in inherited-value-context.js, the same problem should affect Button (elements.button), the other heading-like blocks (Site Title, Post Title, Query Title, …), and the whole-link blocks (elements.link).

Suggestion

The block inspector already resolves this cascade with resolveStyle() plus getElementLayers(). Using both on the Blocks screen would keep the two in sync:

const capabilityFontFamily = useMemo(
	() =>
		resolveStyle( mergedConfig, {
			blockName: name,
			variationName: variation ?? null,
			elements: getElementLayers( name ),
		} ).value?.typography?.fontFamily,
	[ mergedConfig, name, variation ]
);

This needs getElementLayers to be exported from inherited-value-context.js and added to the block-editor private APIs (through components/global-styles/index.js). With that change, both cases in my test pass, all related suites still pass (22 files and 2411 tests, including the new one), and npm run lint:js is clean on the changed files. Passing variationName should also fix the block style variation limitation you mention in the description, but I haven't tested that case. Since you left the browser test out until the model is settled, I can share the patch and the test, or open a PR against your branch, whichever you prefer.

I think this also answers the A/B/C question: go with B (the family the text is actually drawn in), resolved the same way the inspector does it. That way the Blocks screen and the block inspector offer the same weights for the same block.

A smaller note (this bug already exists; this PR didn't cause it): styleToAttributes() in hooks/typography.jsx always removes typography.fontFamily from style and only keeps a preset slug. After the attributesToStyle() change, the panel now shows an inline, non-preset family. From reading the code, I think changing another typography control (for example, font size) still drops that family. Might be worth a follow-up.

Not tested yet: a manual check in the editor.

Thank you — this is the case I could not find, and the Twenty Twenty-Four setup is what makes it obvious. It also answers the A/B/C question, in favour of B: the family the text is actually drawn in, resolved the same way the inspector resolves it, so that the Blocks screen and the block inspector offer the same weights for the same block.

Could you open a pull request against this branch with the resolveStyle() and getElementLayers() change and the browser test? I would rather review yours than rewrite what you have already tested. I will run the manual editor checks alongside it.

One thing worth a case, since your test models Twenty Twenty-Four rather than running it: the shipped themes do not agree on how a family is written into styles. Twenty Twenty-Five uses var:preset|font-family|manrope, while Twenty Twenty-Four and Twenty Twenty-Three use var(--wp--preset--font-family--heading). It would be worth having both forms in the test rather than whichever one the modeled config happens to use.

That the element screen already handles the second form is a point for resolveStyle(): on a real Twenty Twenty-Four, Styles > Typography > Headings shows Cardo and offers its Regular and Bold and the italic it has a face for.

One note for whoever reads the diff after us: passing only the block name to getElementLayers() is deliberate rather than an omission. Its second parameter is a resolved heading level, and leaving it out returns [ 'heading' ], which is what the Blocks screen wants, since that screen is about a block type rather than a particular heading in a document.

On the styleToAttributes() note: I agree, and I would keep it out of this pull request. It predates this branch and it is a different write path, so it deserves its own issue rather than being folded into a change about what the panel reads.