Skip to content
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

Add: Font Family picking mechanism #24868

Merged
merged 2 commits into from Nov 4, 2020

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Aug 27, 2020

This PR adds a font family picking mechanism to the global styles similar to line-height and adds the mechanism at the block level to the paragraph block.

By default, we present as options the list of web-safe fonts https://www.w3schools.com/cssref/css_Websafe_fonts.asp. Themes are able to overwrite the font families available and even use custom ones e.g: google, adobe, etc provided they are enqueued.

image

Related: #23204

How has this been tested?

I verified I'm able to change the font family in a paragraph.
I verified I can change the font families of all paragraphs globally using the following theme.json:

	"core/paragraph": {
		"styles": {
			"typography": {
				"fontFamily": "var(--wp--preset--font-family--comic-sans-ms)"
			}
		}
	}

@jorgefilipecosta jorgefilipecosta added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Aug 27, 2020
@github-actions
Copy link

github-actions bot commented Aug 27, 2020

Size Change: +477 B (0%)

Total Size: 1.21 MB

Filename Size Change
build/block-editor/index.js 131 kB +388 B (0%)
build/block-library/index.js 147 kB +23 B (0%)
build/blocks/index.js 48.1 kB +11 B (0%)
build/edit-site/index.js 22.5 kB +55 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/api-fetch/index.js 3.45 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 9.06 kB 0 B
build/block-library/editor.css 9.06 kB 0 B
build/block-library/style-rtl.css 7.9 kB 0 B
build/block-library/style.css 7.9 kB 0 B
build/block-library/theme-rtl.css 837 B 0 B
build/block-library/theme.css 838 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.2 kB 0 B
build/components/style.css 15.2 kB 0 B
build/compose/index.js 9.81 kB 0 B
build/core-data/index.js 12.5 kB 0 B
build/data-controls/index.js 772 B 0 B
build/data/index.js 8.77 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.46 kB 0 B
build/edit-navigation/index.js 11.2 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.41 kB 0 B
build/edit-post/style.css 6.39 kB 0 B
build/edit-site/style-rtl.css 3.88 kB 0 B
build/edit-site/style.css 3.88 kB 0 B
build/edit-widgets/index.js 26.4 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 43.1 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 7.7 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.16 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13.2 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@aristath
Copy link
Member

Should the select control show plain - unstyled - text instead of the font previews?
It's OK when using system fonts, but if a theme/plugin adds google-fonts for example, then in order to preview a font it will have to be downloaded client-side. That's OK if there's a few of them, but if they add all google-fonts it will get messy real quick

@mtias mtias added the Needs Design Feedback Needs general design feedback. label Aug 28, 2020
@ZebulanStanphill ZebulanStanphill added the Needs Accessibility Feedback Need input from accessibility label Aug 28, 2020
@jorgefilipecosta
Copy link
Member Author

It's OK when using system fonts, but if a theme/plugin adds google-fonts for example, then in order to preview a font it will have to be downloaded client-side. That's OK if there's a few of them, but if they add all google-fonts it will get messy real quick

Hi @aristath,

That's a good point. But if one makes a set of google font available will not the fonts need to be enqueued all the time anyway? Because a font may be used in a paragraph somewhere so the assets need to be loaded so the font displays correctly when used in a random paragraph.
So I guess all the fonts that are available to show in the drop-down will be loaded anyway all the time.

@aristath
Copy link
Member

It depends on how that font-selector is used...
Will the theme show a dropdown with pre-defined sets of font-families? Or will themes allow users to choose any google-font?

It's pretty common practice for current (legacy/non-FSE) themes to add a dropdown in the customizer for example where the user can select any google-font for their body, headers etc. Imagine they want to port that functionality to global-styles... it's a common and valid scenario, as for example some locales require specific fonts that the theme can't fit in a list of hand-picked fonts.

The way things are currently handled in the customizer in themes is that the <select> is a plain select (or select2-like that allows search), and then as soon as the user picks a font, a hook gets triggered and the <link> gets added to the previewer, and the css gets applied.

Font-family selectors will get abused real fast... It would just be safer if it was a plain unstyled <select>, and onChange an action can run so authors can hook in there and add the font's <link> so the family gets loaded.

@aristath
Copy link
Member

aristath commented Aug 31, 2020

As for when the fonts get loaded, I can easily picture themes adding a filter on render_block, and only print the <link> for the gfont if there's a block present that uses a google-font in its font-family attributes... So I wouldn't expect them to be always available

@jasmussen
Copy link
Contributor

We should use this opportunity to provide some beautiful and slighty more modern font stacks! What you can do across Windows, Mac and Linux now, is a fair bit better than it was back in the comic-sans days 🙈 😅

@jorgefilipecosta
Copy link
Member Author

We should use this opportunity to provide some beautiful and slighty more modern font stacks! What you can do across Windows, Mac and Linux now, is a fair bit better than it was back in the comic-sans days 🙈 😅

Hi @jasmussen, the list I used seems to be what people normally call web-safe fonts. I guess currently we have other options and the list may be outdated. Do you know any resource that contains an updated list of fonts we can safely use on the web? Or are there any specific fonts you would like to include?

@jorgefilipecosta
Copy link
Member Author

As for when the fonts get loaded, I can easily picture themes adding a filter on render_block, and only print the for the gfont if there's a block present that uses a google-font in its font-family attributes... So I wouldn't expect them to be always available

Hi @aristath, this solution may work but has some costs e.g: iterate on all blocks to generate the list of fonts that should be enqueued depending on the fonts used on the blocks. But in scenarios where one wants to provide a very large set of fonts, I guess there is no other way.

@jorgefilipecosta
Copy link
Member Author

for > It depends on how that font-selector is used...
Will the theme show a dropdown with pre-defined sets of font-families? Or will themes allow users to choose any google-font?

It's pretty common practice for current (legacy/non-FSE) themes to add a dropdown in the customizer, for example, where the user can select any google-font for their body, headers etc. Imagine they want to port that functionality to global-styles... it's a common and valid scenario, as for example some locales require specific fonts that the theme can't fit in a list of hand-picked fonts.

The way things are currently handled in the customizer in themes is that the is a plain select (or select2-like that allows search), and then as soon as the user picks a font, a hook gets triggered and the gets added to the previewer, and the css gets applied. Font-family selectors will get abused real fast... It would just be safer if it was a plain unstyled , and onChange an action can run so authors can hook in there and add the font's so the family gets loaded.

The problem of loading the fonts is very complex; on one side, we want to be very unopinionated so our mechanism can be used by everyone, including to display a list of fonts from a third-party provider.
So an option would be what @aristath said, simply have events to say a given font family and weight is being used, and themes or plugins, etc, would be responsible for reacting to the events and making sure the assets needed are loaded.
The problem is that an approach like that may not work on mobile.

@pinarol are there plans to support themes loading third party fonts and have them displayed correctly on mobile? E.g: google fonts https://fonts.google.com/specimen/Oswald?standard-styles=&sidebar.open=true&selection.family=Oswald:wght@600;691
Are we able to on mobile download an arbitrary font available on the internet, e.g: a WOFF file, and use in some part of the text?

If the plan is to support mobile and given that we may not be easily able to run arbitrary code there, I guess we may need a declarative mechanism for font loading.

One simple option would be on theme JSON provide something like:

fontFamilies: [
	fontFamily: "'Oswald'",
	cssSrc: "https://fonts.googleapis.com/css2?family=Oswald:wght@200;300;400&display=swap"
]

That declaration would make sure the editor and frontend always enqueue the stylesheet.
"https://fonts.googleapis.com/css2?family=Oswald:wght@200;300;400&display=swap" when the Oswald font family is used.
One mobile, we would need to parse the stylesheet to retrieve the font URL.
It follows the following syntax:

/* latin */
@font-face {
  font-family: 'Oswald';
  font-style: normal;
  font-weight: 200;
  font-display: swap;
  src: url(https://fonts.gstatic.com/s/oswald/v35/TK3iWkUHHAIjg752GT8Gl-1PKw.woff2) format('woff2');
  unicode-range: U+0000-00FF, U+0131, U+0152-0153, U+02BB-02BC, U+02C6, U+02DA, U+02DC, U+2000-206F, U+2074, U+20AC, U+2122, U+2191, U+2193, U+2212, U+2215, U+FEFF, U+FFFD;
}

So parsing it and retrieving the font URL https://fonts.gstatic.com/s/oswald/v35/TK3iWkUHHAIjg752GT8Gl-1PKw.woff2 (for 200 font-weight) is not very hard. We would only parse the font-face declarations on that URL.

The ways fonts are set are complex. We may have different font files depending on weight and even Unicode ranges. So on the same line of text, to render some Unicode character, one should use a font file to render another Unicode character; one may need to use another font file. One the web browsers deal with using the correct files etc., so we get that for free I'm not sure how this would work on mobile.

This simple loading mechanism assumes the cssSrc links to a file that provides all the font weights available in that font-family as font-weights are not being taken into account in this simple mechanism.

I guess if we want to take into account font weights, a possibility would be to say that the WordPress font loading mechanism follows a specific syntax: E.g. "example.com/font-loader.php?font-family="Oswald"&font-weights=200,400". Where we would append "?font-family="Oswald"&font-weights=200,400" to the font loader resource.
To support a third party font loading mechanism following a different syntax, one could easily create a PHP script that receives the font-family and font-weights from the get parameters and does a permanent HTTP redirect to the way fonts are loaded in a specific font system.
E.g: example.com/google-font-loader.php?font-family="Oswald"&font-weights=200,400 may permanently redirect to https://fonts.googleapis.com/css2?family=Oswald:wght@200;300;400&display=swap.

Any thoughts on these approaches to load fonts?

@jorgefilipecosta
Copy link
Member Author

jorgefilipecosta commented Sep 1, 2020

Regarding mobile, this link says woff files (seems the most common format on the web) are not supported on react native android https://docs.expo.io/guides/using-custom-fonts/. I'm not sure if that is accurate.
The link also contains a sample loading fonts from an HTTP URL that may be useful cc: @pinarol.
This link is from a specific framework but I guess the platform limitations are independent of the framework.

@richtabor
Copy link
Member

A few thoughts here, pulled from our discussion in today's design meeting:

  1. Why are we showing the full font family tail? Is that necessary? It complicates the UX here quite a bit and causes a good bit of disorganization in the list of fonts.

  2. Perhaps a font, and a fallback "serif" or "sans-serif" (how Google Fonts does it) is all we need. I still don't think we need to show that fallback stack within the UI here though. Perhaps serif/sans is declared in the global style.

  3. If there's an advanced component to this control, where you can manipulate the stack, then I'd say surfacing the full stack in a text field would be ideal of course.

@jrtashjian
Copy link
Contributor

I think the idea of an advanced component to manipulate the stack is a good compromise which is similar to the idea of padding/margin controls displaying S, M, L, and custom.

The fallbacks should be handled by the theme in the case of a simpler control where a single preferred font is selected from this dropdown as opposed to a full fallback stack. Regardless, the end-user will always have full control of the fonts used through their browser settings.

@jorgefilipecosta
Copy link
Member Author

Hi @richtabor, @jrtashjian,
The mechanism we are proposing has a name field that can be used to provide a more human-friendly display name for the font. Themes can configure the names they want and the name is independent from what appear in the font-family CSS. That idea is that the display name is also translatable but for now, that work is blocked because we don't have yet translations for JSON files.
I guess we can create more readable human names for the defaults using the name field even if the name is not yet translatable.

@pinarol
Copy link
Contributor

pinarol commented Sep 1, 2020

@pinarol are there plans to support themes loading third party fonts and have them displayed correctly on mobile? E.g: google fonts https://fonts.google.com/specimen/Oswald?standard-styles=&sidebar.open=true&selection.family=Oswald:wght@600;691
Are we able to on mobile download an arbitrary font available on the internet, e.g: a WOFF file, and use in some part of the text?

Not really. We are not interested in loading 3rd party fonts on native mobile side. More context in these comments: #24165 (comment) #24165 (comment)

We are interested in allowing to change size related things, like font size, line height etc. But not changing the font family.

@jasmussen
Copy link
Contributor

jasmussen commented Sep 2, 2020

the list I used seems to be what people normally call web-safe fonts

Absolutely, I recognize it. So long as the list is eventually extensible, I think we have an opportunity to do a bit of curation of the default offering. This also touches on what @richtabor asks:

Perhaps a font, and a fallback "serif" or "sans-serif" (how Google Fonts does it) is all we need. I still don't think we need to show that fallback stack within the UI here though. Perhaps serif/sans is declared in the global style.

One of the options I'd personally love to have on this list, is a "system font" stack:

-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,Oxygen-Sans,Ubuntu,Cantarell,"Helvetica Neue",sans-serif

☝️ that is intrinsically a very long list because it needs to include fonts from a multitude of operating systems to intentionally fall back on the one that is available. Much as I'd like it, there may not be a place in WordPress for a "system fonts" stack, perhaps it's too high concept. But it illustrates the point Rich outlines, that the list can get unwieldy. For that reason, I like the suggestion of a human-friendly field to describe a stack.

Perhaps a font, and a fallback "serif" or "sans-serif" (how Google Fonts does it) is all we need.

That works for Google because the font is expected to work across platforms. But for the non-Google stacks, unless we include cross-platform alternatives, we'd very quickly skew the aesthetics towards a particular platform. Here's a chart of which fonts are installed on which platforms, and it becomes quite a puzzle.

At a very quick glance, this resource looks like a decent starting point, insofar as it offers exactly that, font stacks with human readable names, such as "Georgia-based", "Times New Roman-based" (but with better labels).

From that list, I'd start with these:

/* Times New Roman-based stack */
/* Modern Georgia-based serif stack */
/* Helvetica/Arial-based sans serif stack */
/* Verdana-based sans serif stack */
/* Monospace stack */

That's a decent base spread of fonts, and it's always easy to expand the range later on. As a next step I'd reduce that stack to at most 4 or 5 in the stack. @kjellr do you have any insights to help guide how to reduce those stacks? Or any other thoughts?

@aristath
Copy link
Member

aristath commented Sep 2, 2020

One of the options I'd personally love to have on this list, is a "system font" stack:

-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,Oxygen-Sans,Ubuntu,Cantarell,"Helvetica Neue",sans-serif

That would be brilliant... in most cases a carefully curated list of system fonts works wonders and works across the board. Taking this a step further, the ideal solution for me would consist of options like these:

  • Serif
  • Sans-Serif
  • Monospace
  • Handwritten/Decoration

They are abstract enough to convey the message that these are system fonts that may vary depending on the platform, and they're specific enough to convey what they are.
If themes/plugins need to add to that list with google-fonts, adobe-fonts or whatever other option exists out there then they can add their filters & actions to do what they need to do.
But a clear & minimal starting point for fonts would be great.

We also need to think of cross-platform... I'm on Ubuntu so most of the font-families that by default exist in Windows & OSX don't exist on my laptop. If the list consists of multiple sans-serif variants (Arial, Helvetica, Tahoma, Geneva and the likes all as different options) then they're all going to look the same to me 'cause they'll fallback to sans-serif.

@jasmussen
Copy link
Contributor

a carefully curated list of system fonts works wonders

I think I agree with your general sentiment, though perhaps I'd go a little bit less abstract than just serif and sans-serif. But I wanted to clarify that by using the term "system fonts", in this case I was referring to a so called system font stack whose purpose is to emulate the same font your operating system uses. I.e. SF for Mac, Segoe UI for Windows.

@BinaryMoon
Copy link

I have a list of modern font stacks here if that helps:
https://prothemedesign.com/tools/websafe-fonts/

I would also like a system font added to the options.

@jasmussen
Copy link
Contributor

Thank you Ben, that is an excellent resource! Shorter stacks, human readable names (though I'd skew the sans serif name in favor of Helvetica ;) ) and useful.

I would still start smaller, though, because it's trivial to expand the list, but it's VERY hard to reduce the list. If we were to start with, say, 2 sans serif, 2 serifs and 1 monospace font, which stacks would you choose?

@aristath
Copy link
Member

aristath commented Sep 23, 2020

Whether core should enqueue fonts or not. I feel this is still undecided. It looks like core should either enqueue a good curated list of fonts or don't enqueue any. Given the feedback I read by Matías and Joen it seems to me that the defaults provided are sub-optimal and it'd be best not to ship any.

+1 to not enqueing any. There are many ways to enqueue a font, and there are dozens of webfont providers out there. Core shouldn't enqueue anything here, if a theme or plugin adds 3rd-party fonts it's up to them to implement their enqueing. Google Fonts don't work the same as Adobe fonts or locally-hosted webfonts, or any other provider... each one needs separate implementations and all core can do is say "you need to get this font". How that font is added in the browser is outside the scope of Gutenberg. For system fonts that don't need enqueueing there is no issue.

I don't think we need the group/fontFamilyGroups feature, as nobody is using it. If proved necessary, it could be added later.

I'll have to disagree with that... If on my theme I have 3 system fonts, 10 google-fonts and 5 adobe fonts I'd like them in separate groups. System fonts will be picked by those who want something native & fast, gfonts by those who don't have issues with privacy and performance, adobe fonts by those who have an API key.
Some other theme might group them as Serif/Sans-Serif/Monospace and have a collection of fonts that will be neatly categorized so users can find what they're looking for a lot easier...

@jorgefilipecosta
Copy link
Member Author

+1 to not enqueing any. There are many ways to enqueue a font, and there are dozens of webfont providers out there.

Hi @nosolosw, @aristath I agree core should not enqueue any font. This PR is not enqueuing any font. We allow the user to select between 5 fonts + the system font. We don't enqueue these fonts. We choose these five fonts because they were all very safe, and we are assuming users will have these fonts on their system. The users having some choice, even if very limited, is an excellent way to get attention to the feature; otherwise, the future would not be noticeable unless the time provides some fonts.

@jorgefilipecosta
Copy link
Member Author

I don't think we need the group/fontFamilyGroups feature, as nobody is using it. If proved necessary, it could be added later. However! This is a strong opinion weakly held, so is non-blocking from my POV.

I tend to agree with @aristath and I think having groups is helpful because fonts are typically grouped. We don't force the group usage in this PR it is something themes and plugins can provide or not.

@jorgefilipecosta
Copy link
Member Author

jorgefilipecosta commented Sep 24, 2020

We're attaching a CSS variable to the style attribute: font-family: var(--wp--preset--font-family--);, which doesn't work for users without unfiltered_html capabilities. Can we instead make it work like the other presets (attach a class named like has--font-family)? This is somehow related to the link color issues at #25411 although I think it's different in a fundamental way: in the case of font families, the user can only select one of the presets, they can't use custom values, which enables us to use the classes approach that works for everyone.

Regarding the css vars and unfiltered_html. This problem is a little different from the link color. On the link color, we need to use a CSS variable here, we just want to use a CSS variable.
E.g:
<p style="font-family:Consolas;">AAAAAAAAAA</p> is allowed by kses while <p style="font-family:var(--wp--preset--font-family--consolas)">AAAAAAAAAA</p> is not.

But we could still expand #25411 to allow all css variables and that would fix the issue.
The issue would also be fixed by any solution that adds the styles on the server.

Regarding the usage of classes, I did not follow that option because I thought we had a decision to not use classes for presets and instead use CSS variables. We should revert that decision and just use classes for presets here like we currently do for colors? cc: @youknowriad

@aristath aristath added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Sep 28, 2020
@aristath
Copy link
Member

@jorgefilipecosta It was no longer possible to test this PR because it was too far behind the master branch, and FSE themes that worked on the master branch no longer worked with this one.
I merged master, resolved conflicts and this is now up to date again.

@aristath
Copy link
Member

aristath commented Oct 2, 2020

Merged master here again, keeping this branch sync'd 'cause it's important.
With font-size and line-height marked as stable there were a lot of conflicts, they were all resolved and things are working again.

@jorgefilipecosta what is blocking us here?

@jorgefilipecosta
Copy link
Member Author

I think this PR should be ready to merge I applied the feedback and right now core does not expose any font family preset so by default the system is disabled.
To test one needs to add something like the following sample to theme.json:

				"fontFamilies": [
					{
						"fontFamily": "-apple-system,BlinkMacSystemFont,\"Segoe UI\",Roboto,Oxygen-Sans,Ubuntu,Cantarell,\"Helvetica Neue\",sans-serif",
						"slug": "system-font",
						"name": "System Font",
						"group": "web-safe"
					},
					{
						"fontFamily": "Helvetica Neue, Helvetica, Arial, sans-serif",
						"slug": "helvetica-arial",
						"group": "web-safe"
					},
					{
						"fontFamily": "Geneva, Tahoma, Verdana, sans-serif",
						"slug": "geneva-verdana",
						"group": "web-safe"
					},
					{
						"fontFamily": "Cambria, Georgia, serif",
						"slug": "cambria-georgia",
						"group": "web-safe"
					},
					{
						"fontFamily": "Hoefler Text, Baskerville Old Face, Garamond, Times New Roman, serif",
						"slug": "hoefler-times-new-roman",
						"group": "web-safe"
					},
					{
						"fontFamily": "Consolas, monaco, monospace",
						"slug": "consolas",
						"group": "web-safe"
					}
				],
				"fontFamilyGroups": [
					{ "name": "Web safe", "slug": "web-safe"}
				]

One thing that we need to clarify before the merge is if we should expose the font family picker on the post editor. Right now we do that as we do for the other UI parts, but if I'm not in error I think @mtias referred that for font-family we should not expose it to individual blocks. @mtias would you be able to confirm if that's the case? If yes I will update the PR to remove the font family from individual blocks if not I guess we can merge.

@mtias
Copy link
Member

mtias commented Oct 29, 2020

Let's start with this only in the global context and for specific blocks that want it (through theme.json) like "navigation". Do we need the fontFamilyGroups spec? I think we should be flatter initially. Also don't quite like the font definition itself specifying which group it belongs.

@jorgefilipecosta
Copy link
Member Author

Hi @mtias, your feedback was applied:

  • The shape is flat without any grouping for now. We can analyze grouping after font families get a wider usage.
  • Font family picker is available at the global level and the block level for blocks I thought it would make sense: Navigation, Site Title, Site tagline, Post Title.

We can test the font family mechanism by adding the following data under typography to theme.json.


				"fontFamilies": [
					{
						"fontFamily": "-apple-system,BlinkMacSystemFont,\"Segoe UI\",Roboto,Oxygen-Sans,Ubuntu,Cantarell,\"Helvetica Neue\",sans-serif",
						"slug": "system-font",
						"name": "System Font"
					},
					{
						"fontFamily": "Helvetica Neue, Helvetica, Arial, sans-serif",
						"slug": "helvetica-arial"
					},
					{
						"fontFamily": "Geneva, Tahoma, Verdana, sans-serif",
						"slug": "geneva-verdana"
					},
					{
						"fontFamily": "Cambria, Georgia, serif",
						"slug": "cambria-georgia"
					},
					{
						"fontFamily": "Hoefler Text, Baskerville Old Face, Garamond, Times New Roman, serif",
						"slug": "hoefler-times-new-roman"
					},
					{
						"fontFamily": "Consolas, monaco, monospace",
						"slug": "consolas"
					}
				],

Themes can disable font family picking for the global level and for each block level by simply setting font families options to empty:

	"core/site-title": {
		"settings": {
			"typography": {
				"fontFamilies": []
			}
		}
	}

Following the same approach, we have for the other blocks.

@aristath
Copy link
Member

aristath commented Nov 4, 2020

Merged master branch here, resolving some conflicts that existed due to recent PRs that got merged.

@jorgefilipecosta jorgefilipecosta dismissed oandregal’s stale review November 4, 2020 14:21

The review was addressed.

@jorgefilipecosta jorgefilipecosta merged commit aeef540 into master Nov 4, 2020
@jorgefilipecosta jorgefilipecosta deleted the add/font-family-picking-mechanism branch November 4, 2020 15:20
@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 4, 2020
@oandregal
Copy link
Member

@jorgefilipecosta @aristath This PR landed without documentation. I've prepared a fix for it at #26891

$font_family_name = substr( $font_family, $index_to_splice );
$styles[] = sprintf( 'font-family: var(--wp--preset--font-family--%s);', $font_family_name );
} else {
$styles[] = sprintf( 'font-family: %s;', $block_attributes['style']['color']['fontFamily'] );
Copy link
Member

@oandregal oandregal Jul 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under which situations a font-family value is not in the form of var:preset|font-family|etc?

Unlike other style properties, users can't provide custom values, so I don't see how this path is reached. I presume we can remove it. The fact that this has been using style.COLOR.fontFamily without a bug being reported seems to confirm this is a code path we don't need. cc @jorgefilipecosta @aristath

For context, this question comes from a PR I'm working on at #31910 to use classes instead of CSS Custom Properties for font-family. I'm looking at removing this code path there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess a situation where font-family may not be preset is in the case of a block pattern where the pattern is excitedly using a specific font family in a block.

Regarding the usage of classes for font-family presets I think we can use variables for now it is not clear classes bring any advantage in that use case. For the colors, we are outputting CSS vars anyway so a block could use the vars directly and we need to support that. For the font family, I guess we can keep with just vars for now and see what feedback we get.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess a situation where font-family may not be preset is in the case of a block pattern

Nice thinking, thanks! I'll keep that use case then.

Regarding the usage of classes for font-family presets I think we can use variables for now it is not clear classes bring any advantage in that use case

I'd think we want to do it the other way around: what's the advantage of font-family using custom properties instead of classes, as the other style properties do?

@StefanXhunga
Copy link

Thank you for your assistance!

@ethanclevenger91
Copy link

@jorgefilipecosta is there updated documentation for this? You used this example of theme.json contents to opt into the font picker:

"core/site-title": {
	"settings": {
		"typography": {
			"fontFamilies": []
		}
	}
}

But that structure (block > settings) isn't documented in any way I can find here, let alone in relation to enabling the font picker.

@jorgefilipecosta
Copy link
Member Author

Hi Ethan Clevenger,

One can control settings per block, now the syntax is not that one but there are updated samples of using settings per block at https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-json/#settings-examples e.g: "Disable border radius for the button block:".
Something like this should work well:

{
    "version": 1,
    "settings": {
        "blocks": {
            "core/site-title": {
                "typography": {
                    "fontFamilies":[]
                }
            }
        }
    }
}

Any setting can be set globally or for a specific block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet