-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Try] Font Library: register default fonts #54888
base: trunk
Are you sure you want to change the base?
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/experimental/fonts/font-library/class-wp-font-collection.php ❔ lib/experimental/fonts/font-library/class-wp-font-library.php ❔ lib/experimental/fonts/font-library/font-library.php |
Size Change: +541 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
This looks great. I think it is the way. What I understand this is doing: Allows us to register a special collection of fonts that are always activated and always available to the user. Can we do this with the existing API? Rather than adding a new 'register_default_fonts' can we instead allow for that as a configuration option on a collection?
|
Thank you so much @pbking for going over the PR again and for the helpful discussions around this.
Yes, that's a possibility, too, but I think it isn't the best alternative regarding performance, mental models around what a font collection is, and the future development of the font collection functionality. It can be problematic in terms of performance because the data from font collections are read from a JSON file or a remote URL, so to print the CSS, we should wait to read the remote URL, parse it, and after that, print the font faces to the page, which can be expensive. We could restrict the font collection In terms of mental models of what a font collection is, I think we should keep the font collection's scopes and the default fonts' register separated. Font collections are a way for extenders to propose a set of fonts to the user that can be installed on their site. It's a good practice and gives end users full UI control of what's the active and active fonts without touching anything related to what's printed on the site. On the other hand, registering default fonts is like a highway to the frontend because it's in charge of injecting things into the theme.json, and their use should be minimal. Mixing both functionalities below the same font collection umbrella could add confusion about each thing. About the development of the font collection functionality, this option isn't optimal either. We would need to add extra logic to the font collection codebase in the current codebase. For example, in the backend, restrict the |
…ion (to avoid confusion with the 'default' font idea).
I've made the changes you mentioned would be necessary; restricted 'default' font collection to local-file only and filtered default font collections from the user's view. I believe that the expense of loading a local file for that font data is minimal. I definitely agree that fetching that from anywhere shouldn't be an available option, but I think this mitigates that issue. From a user's perspective this seems pretty straightforward. These are fonts that are installed at the system layer and I can't remove them and always have access to them. What is available this way can be a decision made by system administration, not users. From an integration standpoint this is also most convenient. In the same way I can make collections of fonts available to a user to CHOOSE to use I can also, with the same and familiar API, make a collection of fonts ALWAYS available to a user. I believe this would completely close the gap on the functionality that Jetpack Google Fonts is currently offering. Please let me know your thoughts with that change. |
Flaky tests detected in 3bdc2c2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6341792087
|
Now we try to use the default collection file to register the default fonts, so the developer cannot control which fonts should be removed from the default collection dynamically due to the duplication. Do we handle the duplication between the default fonts and theme fonts (or even installed fonts)? Additionally, I'm curious about how you get the asset URL like I'd prefer to support loading the fonts via the Google API, |
I want to mention that this feature can change and even be removed. Sorry for not adding more details right now. |
This is an experiment designed in conjunction with @pbking and @jeffikus 🙌
What?
Experiment with registering fixed default fonts.
Registered default fonts:
Calling
wp_register_default_fonts( $fonts )
, you can add default fonts imperatively. The parameter$fonts
should be an array of font families in theme.json fashion format.Calling
wp_unregister_default_fonts( $fonts )
you can remove font imperatively added bywp_register_default_fonts( $fonts )
;Why?
To facilitate the migration from plugins or extenders code that want to migrate to the font library to enable the users to manage their typographies.
How?
With the implementation of
wp_register_default_fonts( $fonts )
andwp_unregister_default_fonts( $fonts )
, extenders can add and remove font families programmatically in PHP.Example:
Testing Instructions
Try to add fonts programmatically following the example.
Screenshots or screencast
2023-09-27.16-12-07.mp4
Co-authored-by: Jason Crist [email protected]
Co-authored-by: Jeffrey Pearce [email protected]