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

Support "name" attribute in wp_register_webfonts #46398

Closed
simison opened this issue Dec 8, 2022 · 10 comments
Closed

Support "name" attribute in wp_register_webfonts #46398

simison opened this issue Dec 8, 2022 · 10 comments
Labels
[Type] Enhancement A suggestion for improvement.
Projects

Comments

@simison
Copy link
Member

simison commented Dec 8, 2022

What problem does this address?

When registering fonts from a plugin with wp_register_webfonts, we define a font-family that then becomes the "name" of the font in Style picker.

Screenshot 2022-12-08 at 12 02 29

When registering fonts with themes with theme.json, fontFamilies supports passing the name attribute that then gets picked by the UI as font label in the dropdown.

Using fontFamily as label works fine for most cases just fine. When a plugin provinding external font-provider (Google Fonts in our case), there isn't flexibility in pickign a nice font family name because the font family has to match the one passed to Google Fonts API.

For example, language-specific fonts in Google fonts don't have pretty names:

  • Alexandria
  • IBM Plex Sans Arabic
  • Noto Sans Hebrew
  • Noto Sans HK
  • Noto Sans JP
  • Noto Sans KR
  • Noto Sans SC
  • Noto Sans TC
  • Noto Sans Telugu
  • Noto Serif Hebrew
  • Noto Serif HK
  • Noto Serif JP
  • Noto Serif KR
  • Noto Serif SC
  • Noto Serif TC

Above list would be better served with human-readable labels. It would be great to be able to translate the parts inside brackets, too.

	'Alexandria'           => 'Alexandria (Arabic)',
	'IBM Plex Sans Arabic' => 'IBM Plex Sans (Arabic)',
	'Noto Sans Hebrew'     => 'Noto Sans (Hebrew)',
	'Noto Sans HK'         => 'Noto Sans (Hong Kong)',
	'Noto Sans JP'         => 'Noto Sans (Japanese)',
	'Noto Sans KR'         => 'Noto Sans (Korean)',
	'Noto Sans SC'         => 'Noto Sans (Simplified Chinese)',
	'Noto Sans TC'         => 'Noto Sans (Traditional Chinese)',
	'Noto Sans Telugu'     => 'Noto Sans (Telugu)',
	'Noto Serif Hebrew'    => 'Noto Serif (Hebrew)',
	'Noto Serif HK'        => 'Noto Serif (Hong Kong)',
	'Noto Serif JP'        => 'Noto Serif (Japanese)',
	'Noto Serif KR'        => 'Noto Serif (Korean)',
	'Noto Serif SC'        => 'Noto Serif (Simplified Chinese)',
	'Noto Serif TC'        => 'Noto Serif (Traditional Chinese)',

What is your proposed solution?

wp_register_webfonts should support name attribute that then gets added to Theme JSON object.

AlternativelyI looked altering theme json object with wp_theme_json_data_theme filter (as in example) but this felt rather messy for something as simple.

@simison
Copy link
Member Author

simison commented Dec 8, 2022

Not quite sure who could comment on this best and take a look at PR #46404@oandregal or @youknowriad you might know the best. :-)

@youknowriad
Copy link
Contributor

Maybe that's a good one for @hellofromtonya as it relates to the fonts API.

@hellofromtonya hellofromtonya added this to Backlog in Fonts API via automation Dec 8, 2022
@hellofromtonya
Copy link
Contributor

The Web Fonts API's architecture is currently being completely redesigned (see #41481, #41484, #42868). I'll add this issue to its Roadmap but also block its PR until the architecture redesign PRs are merged.

@hellofromtonya
Copy link
Contributor

This enhancement is now added to the Web Fonts API's Roadmap ✅ and the PR is blocked until the architectural work is merged ✅ .

@simison
Copy link
Member Author

simison commented Dec 21, 2022

@hellofromtonya is it a good time to rebase this PR now that the API redesign is merged, or do you plan to continue refactoring the API?

@hellofromtonya
Copy link
Contributor

@simison There are 2 more blocking architectural PRs, one of which will rename the entire API and all of its code. A status update is here #41479 (comment).

@hellofromtonya
Copy link
Contributor

@simison This issue is now unblocked. Please note, the API has been completely redesigned including a brand new architecture and renamed to Fonts API (including all of the public function names).

@hellofromtonya hellofromtonya added the [Type] Enhancement A suggestion for improvement. label Jan 18, 2023
@hellofromtonya hellofromtonya moved this from Backlog to In progress in Fonts API Jan 18, 2023
@hellofromtonya hellofromtonya added this to the Gutenberg 15.1 milestone Jan 18, 2023
@hellofromtonya
Copy link
Contributor

Some notes to help:

  • I closed PR Add name to wp_register_webfonts #46404. Why? Its implementation applied to the old API but will not work in the new Fonts API.
  • A PR will be needed to add the "name" attribute.
  • wp_register_webfonts() is now wp_register_fonts()
  • The fonts definition array structure changed.
    Old:
array(
    array( /* font 1 */ ),
    array( /* font 2 */ ),
)

new array structure:

array(
    'Lato' => array(
          array( /* variation 1 */ ), 
          array( /* variation 2 */ ),
    ),
)

from an array of fonts to an array of font-families with each of the variations as an array to its keyed font-family.

The font-family key can be its proper name. The API will automatically convert it into a handle. Thus, a 'name' attribute might not be needed.

@mburridge mburridge removed this from the Gutenberg 15.1 milestone Jan 31, 2023
@hellofromtonya hellofromtonya moved this from In progress to In Discussion in Fonts API Apr 25, 2023
@ironprogrammer
Copy link
Contributor

Following up with @hellofromtonya's comment above, I've verified that using the updated fonts definition (keyed array) uses the provided key name in font pickers.

These results support Tonya's assertion that a new name attribute is not required to display the friendly names, and that this ticket should be considered for close.

Steps to Test

  1. Install this Fonts API test plugin, which utilizes Jetpack's Google Fonts provider, also mentioned by the OP. Make sure to run composer install in the plugin's root to install the provider dependency.
  2. In webfonts.php, update the JETPACK_GOOGLE_FONTS_LIST constant with the font names to test (for instance, drop in a handful of Noto Sans font faces) using the names shown on the website.
  3. Activate Gutenberg if it isn't. Then activate the test plugin.
  4. In the site editor, navigate to Styles > Typography > Headings and observe the names displayed in the font picker dropdown.
  5. Navigate to a post, select a block, and enable the Typography Font option through the three vertical dots menu. Observe the names displayed in the dropdown.

Expected Results

  • ✅ The "friendly names" of fonts should be displayed in the pickers, for global Styles and the post editor.

Supplemental Artifacts

Figure 1: Friendly font names displayed in global Styles picker.
friendly font names displayed in global styles picker

Figure 2: Friendly font name in post editor.
friendly font name in post editor

@hellofromtonya
Copy link
Contributor

Update:

This issue no longer applies to the Fonts API once the Font Library is merged. I believe this issue can be closed as the "name" attribute is supported in theme.json.

Why?

As shared in Ongoing Roadmap update #41479 (comment), the Fonts API will no longer provide the means to register or enqueue fonts. Instead, it will pull / get the fonts from Theme JSON and then process each to server-side generate and print the @font-face styles.

With the coming Font Library, theme.json schema / data is used for data workflow. The theme.json fontFamilies already supports "name" attribute:

"fontFamily": "\"DM Sans\", sans-serif",
"name": "DM Sans",
"slug": "dm-sans"

Therefore, I think this issue can be closed.

Thank you everyone for your contributions!

Fonts API automation moved this from In Discussion to Done Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
Development

No branches or pull requests

5 participants