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 Architecture: Explore leveraging WP_Dependencies existing enqueuing, registration, and internal handling #41481

Closed
hellofromtonya opened this issue Jun 1, 2022 · 6 comments · Fixed by #43492
Assignees
Labels
[Priority] High Used to indicate top priority items that need quick attention

Comments

@hellofromtonya
Copy link
Contributor

Part of #41479.

Explore changing the current architecture to extend from the WP_Dependencies to reuse its implementations of enqueuing, registration, and internal handling.

Why?

The current implementation re-implements a lot of the concepts that are already handled by WP_Dependencies for the script and style loaders. The script loader classes already have the concepts of dependencies, enqueuing, and registering, as well as a way to "do_item()" (generate CSS or a tag with source, in this case).

Goals: reduce code, increase readability, and increase maintainability.

@hellofromtonya hellofromtonya changed the title Webfonts API: Explore changing the architecture to extend from WP_Dependencies Webfonts API: Explore changing the architecture to leverage WP_Dependencies existing enqueuing, registration, and internal handling Jun 1, 2022
@hellofromtonya hellofromtonya changed the title Webfonts API: Explore changing the architecture to leverage WP_Dependencies existing enqueuing, registration, and internal handling Webfonts API Architecture: Explore leveraging WP_Dependencies existing enqueuing, registration, and internal handling Jun 1, 2022
@hellofromtonya hellofromtonya added the [Priority] High Used to indicate top priority items that need quick attention label Jun 23, 2022
@hellofromtonya
Copy link
Contributor Author

hellofromtonya commented Jul 13, 2022

Thank you @desrosj for creating this POC in PR #40556.

A lot of the functionality that is in the POC and currently in Gutenberg's implementation will be removed and/or changed as part of the current vision in the ongoing roadmap.

I have concerns about extending from WP_Dependencies to make it work for the needs of font management.

tl;dr

I'm thinking Webfonts API doesn't really fit into the intent of WP_Dependencies.

  1. Webfonts API does not need font-family- to - font-family dependency management which is big part of script/script loading management.
  2. WP_Dependencies exposes functionality that will not be used in the Webfonts API.

The concept of dependency is different

The understanding of "dependencies" for font management is different than styles/scripts.

WP_Dependencies and style/script loading management focuses more on managing stylesheet-to-stylesheet or script-to-script dependencies, i.e. one stylesheet is dependent upon one or other stylesheets or one script file is dependent upon one or more other script files.

Dependencies for styles and scripts have a 1:many relationship where it's clear the intent is to manage those resource-to-resource relationships to ensure a stylesheet or script files dependencies are also loaded into memory.

Font management is different. It doesn't have font-family to font-family dependencies. For example, Source Serif Pro is not dependent upon Lato. A font is also not dependent upon other scripts or stylesheets.

Unneeded WP_Dependencies public functionality

The following list of WP_Dependencies functionality would need to be disabled, meaning the publicly expose methods are still there but are not used or functional.

IMO having the publicly exposed methods that aren't usable will be confusing to extenders and contributors.

Enqueue

Enqueuing will not be publicly exposed, but rather will be private inner-workings of the API (please refer to the Vision > Enqueuing section).

WP_Dependencies publicly exposes the ability to enqueue and dequeue. Overloading could make the enqueue() and dequeue methods not do anything, ie empty methods.

Remove

Removing a font-family and/or a font-family's font-face (variant) are not planned functionality due to concerns of plugins removing theme defined font-families from registration.

Current vision:

  • the theme sets the fonts
  • plugins can also register additional fonts, but cannot override theme defined fonts. Note: These registered fonts will be available for users to select through the Global Styles > Typography pickers.

WP_Dependencies publicly exposes the ability to remove a handle, which in this case would be a font-family. Overloading could make the remove() method not do anything, ie an empty method.

@desrosj @azaozz @felixarntz What do you all think?

@desrosj
Copy link
Contributor

desrosj commented Jul 14, 2022

Thanks for looking at my proof of concept! I have some follow up questions to get some clarifications.

Font management is different. It doesn't have font-family to font-family dependencies. For example, Source Serif Pro is not dependent upon Lato. A font is also not dependent upon other scripts or stylesheets.

It's true that font faces do not depend on one another, but that's not how my proof of concept works. A font family gets registered with no source URL (which means it is an alias), and then the variations of each font are registered as dependencies of that alias. This practice has been in place for some time with scripts like jQuery, mediaelement, and a handful others. With this approach, it is a 1:many relationship. A font family to many variations.

I agree that WP_Dependencies does not feel like a perfect match, but recreating registering and enqueuing feels like we're reinventing the wheel. I also worry about confusion if there's a separate Web Fonts API

Enqueuing will not be publicly exposed, but rather will be private inner-workings of the API

I'm on board with this for the fonts in theme.json and within the content through the editor, but what if someone wants a font to load that is not detectable through these means? For example, maybe content is being generated and output somewhere with PHP through a shortcode block.

Removing a font-family and/or a font-family's font-face (variant) are not planned functionality due to concerns of plugins removing theme defined font-families from registration

How is this any different than plugins and themes removing Core defined scripts?

plugins can also register additional fonts, but cannot override theme defined fonts.

What if a theme specifies fonts to load from Google Fonts, but the site owner wants to include the fonts on their server for loading instead? Would that be possible without this ability? Or what if a company wants to prevent a certain font family from ever appearing as an option due to branding concerns?

@hellofromtonya
Copy link
Contributor Author

Thanks for the feedback @desrosj. I'll answer this part today and will think on the others.

What if a theme specifies fonts to load from Google Fonts, but the site owner wants to include the fonts on their server for loading instead? Would that be possible without this ability? Or what if a company wants to prevent a certain font family from ever appearing as an option due to branding concerns?

Theme specified fonts can be changed through the Global Styles Typography interface. For both of scenarios, a company or site admin could customize through a plugin and then select which to apply to the different typography needs.

@hellofromtonya
Copy link
Contributor Author

Removing a font-family and/or a font-family's font-face (variant) are not planned functionality due to concerns of plugins removing theme defined font-families from registration

How is this any different than plugins and themes removing Core defined scripts?

Great question @desrosj. How is it different?

tl;dr Users decide, rather than having plugins programmatically override the theme.

It's different because global styling typography is defined by the theme. A user can override the theme through the Global Styles UI using any of the additional web fonts defined by plugins. Plugins can offer their web fonts for selection, but users decide whether and where to use any of those plugin-defined fonts.

@hellofromtonya
Copy link
Contributor Author

I'm on board with this for the fonts in theme.json and within the content through the editor, but what if someone wants a font to load that is not detectable through these means? For example, maybe content is being generated and output somewhere with PHP through a shortcode block.

I'm not sure I follow. Are you thinking a web font could be programmatically enqueued through a shortcode?

Web fonts are for global styling, rather than individual blocks.

A shortcode block that needs a different family-font or variation could register its web font for user selection.

@hellofromtonya
Copy link
Contributor Author

I withdraw my objections to this proposal. Let me explain.

Initially the plan was to move enqueuing to an internal only inner-workings. After discussing with @mtias in light of @desrosj well-argued points and when considering classic and block themes, the vision changed and was updated in the ongoing roadmap. Now enqueue and remove will be publicly exposed.

I now agree with @desrosj that it makes sense to move forward with this architectural proposal. Why? It will remove the coupling between the 2 APIs while leveraging inheritance to reuse the functionality and concepts.

What about the intent of what dependencies means in the Script/Style Loader component? As Jon points out, there is precedence to bend what a dependency is. In the case of fonts, the variations are dependent upon the font-family and a font-family is dependent upon its variations.

I vote to change the architecture based on Jon's POC as a starting point 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment