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

Webfonts API #37140

Merged
merged 35 commits into from
Feb 28, 2022
Merged

Webfonts API #37140

merged 35 commits into from
Feb 28, 2022

Conversation

aristath
Copy link
Member

@aristath aristath commented Dec 6, 2021

Description

This is an alternative to #36394, backporting the API from WordPress/wordpress-develop#1736.
That patch was thoroughly discussed with core & theme developers and approved by core committers (@felixarntz, @hellofromtonya, @SergeyBiryukov), but the API was not merged in core on time for WP 5.9.
It was deemed a better choice to port the API to Gutenberg, where it can be implemented for theme.json and the font-family pickers (see comment on https://core.trac.wordpress.org/ticket/46370#comment:111)

The difference between this PR and #36394 is that this one removes the registry, controller, validator & provider implementations, simplifying the code structure, addressing all feedback from the initial PR, and making it easier to merge in WordPress.

The user-facing functions are exactly the same.

How has this been tested?

  • PHPUnit tests
  • Added a webfont in theme.json and confirmed that everything works as expected.
  • Added a webfont in functions.php and confirmed that it works as expected.

Testing a webfont loaded via theme.json:

In the twentytwentytwo theme, edit its theme.json file and replace fontFamilies with the following:

			"fontFamilies": [
				{
					"fontFamily": "-apple-system,BlinkMacSystemFont,\"Segoe UI\",Roboto,Oxygen-Sans,Ubuntu,Cantarell,\"Helvetica Neue\",sans-serif",
					"name": "System Font",
					"slug": "system-font"
				},
				{
					"fontFamily": "\"Source Serif Pero\", serif",
					"name": "Source Serif Pero",
					"slug": "source-serif-pero",
					"fontFace": [
						{
							"fontFamily": "Source Serif Pero",
							"fontWeight": "200 900",
							"fontStyle": "normal",
							"fontStretch": "normal",
							"src": [ "file:./assets/fonts/SourceSerif4Variable-Roman.ttf.woff2" ]
						},
						{
							"fontFamily": "Source Serif Pero",
							"fontWeight": "200 900",
							"fontStyle": "italic",
							"fontStretch": "normal",
							"src": [ "file:./assets/fonts/SourceSerif4Variable-Italic.ttf.woff2" ]
						}
					]
				}
			],

Then, in functions.php replace the twentytwentytwo_get_font_face_styles function with this:

function twentytwentytwo_get_font_face_styles() { return ''; }

The above will test the font-family registration using theme.json.

Next, to test the PHP registration using the wp_register_webfonts function, in functions.php add the following:

add_action( 'after_setup_theme', function() {
	if ( ! function_exists( 'wp_register_webfonts' ) ) {
		return;
	}
	wp_register_webfonts(
		array(
			array(
				'font-family'  => 'Random Font Name',
				'font-weight'  => '200 900',
				'font-style'   => 'normal',
				'font-stretch' => 'normal',
				'src'          => array( 'file:./assets/fonts/SourceSerif4Variable-Roman.ttf.woff2' ),
			),
			array(
				'font-family'  => 'Random Font Name',
				'font-weight'  => '200 900',
				'font-style'   => 'italic',
				'font-stretch' => 'normal',
				'src'          => array( 'file:./assets/fonts/SourceSerif4Variable-Italic.ttf.woff2' ),
			),
		)
	);
} );

The expected result is this:

  • The webfont gets properly loaded in the frontend and editor
  • The Source Serif Pero font is available in site editor -> global-styles -> typography
  • The Random Font Name font is available in the font-family picker too

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

lib/global-styles.php Outdated Show resolved Hide resolved
lib/class-wp-webfonts.php Outdated Show resolved Hide resolved
lib/class-wp-webfonts.php Outdated Show resolved Hide resolved
lib/class-wp-webfonts.php Outdated Show resolved Hide resolved
@azaozz
Copy link
Contributor

azaozz commented Dec 7, 2021

This is an alternative to #36394.

Yes, this is simpler and imho looks quite better. Still unsure about having the provider in separate files/classes but that's not a big deal.

@oandregal
Copy link
Member

I've pushed some changes to the theme.json classes. This PR depends on #38625

To test this, you need to have #38625 locally and do git merge --squash update/extend-theme-json. Updated the testing instructions in the issue description as well.

@oandregal
Copy link
Member

I've just merged #38625 so this needs a rebase to get those changes in.

@aristath aristath force-pushed the add/webfonts-simplified-api branch 2 times, most recently from bf51eae to 88d49be Compare February 9, 2022 11:53
@aristath
Copy link
Member Author

  • Feedback was addressed
  • Tests pass successfully
  • There are 2 ✅ already here

I think it's time we merge this PR, so I'll go ahead and do that. This API is something that we need for WP 6.0, so delaying it more will only result in blocking other PRs and features that depend on this.

We can create follow-up PRs for improvements and additions, but merging this will allow us to start using the API and iterate further as we go 👍

@aristath aristath changed the title Simplified Webfonts API Webfonts API Feb 28, 2022
@aristath aristath merged commit b6a787b into trunk Feb 28, 2022
@aristath aristath deleted the add/webfonts-simplified-api branch February 28, 2022 12:55
@github-actions github-actions bot added this to the Gutenberg 12.8 milestone Feb 28, 2022
@priethor priethor added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Mar 20, 2022
oandregal added a commit that referenced this pull request Dec 22, 2022
oandregal added a commit that referenced this pull request Dec 22, 2022
… inheriting per WordPress version (#46750)

* Backport WP_Theme_JSON_Resolver from core as it is

* Substitute WP_Theme_JSON by WP_Theme_JSON_Gutenberg

* Substitute WP_Theme_JSON_Data by WP_Theme_JSON_Data_Gutenberg

* Rename WP_Theme_JSON_Resolver to WP_Theme_JSON_Resolver_Base

The goal is to use it as base to inherit from, until we are able
to remove all the children.

* Make WP_Theme_JSON_Resolver_6_1 inherit from WP_Theme_JSON_Resolver_Base

* 6.1: remove field already defined in base class

* 6.1: remove translate, it is equal to base method

* 6.1: remove get_core_data, it does not have base changes

* 6.1: remove get_user_data

Missed core changes and does not do anything differently from core.

* 6.1: remove get_user_data_from_wp_global_styles

It misses core changes and does not do anything differently.

* 6.2: inherit from WP_Theme_JSON_Resolver_Base instead of 6.1

This makes the WP_Theme_JSON_Resover_6_1 class unused.

* 6.1: remove class no longer in use

* 6.2: deprecate theme_has_support

#45380

* 6.2: substitute WP_Theme_JSON_Resolver::theme_has_support

by wp_theme_has_theme_json()

#45168

* 6.2: port changes to get_user_data_from_wp_global_styles

#46043

* 6.2: update get_merged_data

#45969

* 6.2: remove get_user_data

Same code as core. There's a check for detecting whether the class
is an instance of WP_Theme_JSON_Gutenberg that was not ported.
This check was introduced to make sure the cache of the core class
didn't interfere with the cache of the Gutenberg class,
so it's no longer necessary.

See #42756

* experimental: make it inherit from WP_Theme_JSON_Resolver_Base

* 6.2: remove class no longer in use

* experimental: delete remove_json_comments

It's already part of the base class.

* experimental: remove get_block_data

It's already part of the base class and it didn't have
the changes from core.

* experimental: appearanceTools

Port changes to the base class that were meant to be part of 6.1.
See #43337

* experimental: port webfonts (experimental API)

see #37140

* experimental: use gutenberg_get_legacy_theme_supports_for_theme_json

#46112

* experimental: get_theme_data, all code is in the base class

* experimental: remove empty class

* Rename WP_Theme_JSON_Resolver_Base to WP_Theme_JSON_Resolver_Gutenberg

* Fix lint issue: rename class-wp-theme-json-resolver.php to class-wp-theme-json-resolver-gutenberg.php

* Move theme.json to unversioned lib/

* Move theme-i18n.json to unversioned lib/
noahtallen pushed a commit that referenced this pull request Dec 28, 2022
… inheriting per WordPress version (#46750)

* Backport WP_Theme_JSON_Resolver from core as it is

* Substitute WP_Theme_JSON by WP_Theme_JSON_Gutenberg

* Substitute WP_Theme_JSON_Data by WP_Theme_JSON_Data_Gutenberg

* Rename WP_Theme_JSON_Resolver to WP_Theme_JSON_Resolver_Base

The goal is to use it as base to inherit from, until we are able
to remove all the children.

* Make WP_Theme_JSON_Resolver_6_1 inherit from WP_Theme_JSON_Resolver_Base

* 6.1: remove field already defined in base class

* 6.1: remove translate, it is equal to base method

* 6.1: remove get_core_data, it does not have base changes

* 6.1: remove get_user_data

Missed core changes and does not do anything differently from core.

* 6.1: remove get_user_data_from_wp_global_styles

It misses core changes and does not do anything differently.

* 6.2: inherit from WP_Theme_JSON_Resolver_Base instead of 6.1

This makes the WP_Theme_JSON_Resover_6_1 class unused.

* 6.1: remove class no longer in use

* 6.2: deprecate theme_has_support

#45380

* 6.2: substitute WP_Theme_JSON_Resolver::theme_has_support

by wp_theme_has_theme_json()

#45168

* 6.2: port changes to get_user_data_from_wp_global_styles

#46043

* 6.2: update get_merged_data

#45969

* 6.2: remove get_user_data

Same code as core. There's a check for detecting whether the class
is an instance of WP_Theme_JSON_Gutenberg that was not ported.
This check was introduced to make sure the cache of the core class
didn't interfere with the cache of the Gutenberg class,
so it's no longer necessary.

See #42756

* experimental: make it inherit from WP_Theme_JSON_Resolver_Base

* 6.2: remove class no longer in use

* experimental: delete remove_json_comments

It's already part of the base class.

* experimental: remove get_block_data

It's already part of the base class and it didn't have
the changes from core.

* experimental: appearanceTools

Port changes to the base class that were meant to be part of 6.1.
See #43337

* experimental: port webfonts (experimental API)

see #37140

* experimental: use gutenberg_get_legacy_theme_supports_for_theme_json

#46112

* experimental: get_theme_data, all code is in the base class

* experimental: remove empty class

* Rename WP_Theme_JSON_Resolver_Base to WP_Theme_JSON_Resolver_Gutenberg

* Fix lint issue: rename class-wp-theme-json-resolver.php to class-wp-theme-json-resolver-gutenberg.php

* Move theme.json to unversioned lib/

* Move theme-i18n.json to unversioned lib/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Dev Note Requires a developer note for a major WordPress release cycle
Projects
Status: No status
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet