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

Font Library: Add a way to disable it. #55275

Closed
matiasbenedetto opened this issue Oct 11, 2023 · 11 comments · Fixed by #57818
Closed

Font Library: Add a way to disable it. #55275

matiasbenedetto opened this issue Oct 11, 2023 · 11 comments · Fixed by #57818
Labels
[Feature] Typography Font and typography-related issues and PRs Needs Decision Needs a decision to be actionable or relevant [Type] Enhancement A suggestion for improvement.

Comments

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Oct 11, 2023

What?

Add a way to disable the Font Library.

When disabled:

  • The Font Management UI wouldn't be presented to the user
  • Activated fonts would not be made available to select from to the user or rendered in the view. The only fonts available to be used would be those provided by the active theme or those added by plugins without the use of the Font Library API.

Why?

To allow environments to disable the font library functionality.


Props to @spacedmonkey, who proposed this functionality.

@matiasbenedetto matiasbenedetto added [Type] Enhancement A suggestion for improvement. Needs Decision Needs a decision to be actionable or relevant [Feature] Typography Font and typography-related issues and PRs labels Oct 11, 2023
@matiasbenedetto
Copy link
Contributor Author

matiasbenedetto commented Jan 5, 2024

Currently, if you define the constant FONT_LIBRARY_DISABLED in wp-config.php like this

define( 'FONT_LIBRARY_DISABLED', true );

then the Font Library UI and the PHP code are not loaded.

Do you think that's enough @spacedmonkey ?

@mikachan
Copy link
Member

mikachan commented Jan 5, 2024

cc. @swissspidy @TimothyBJacobs @costdev, as you've helped out previously with the Font Library work 🙇

@swissspidy
Copy link
Member

We shouldn't really introduce new constants for this kind of stuff in WordPress. It's better to handle this with new capabilities and/or filters.

@TimothyBJacobs
Copy link
Member

Agreed we shouldn't be using a constant. I think it might make sense for this to be a theme support that can be removed.

@costdev
Copy link
Contributor

costdev commented Jan 6, 2024

@mikachan Thanks for the ping!

I agree that we should avoid a constant for this, and think we should consider this a theme support removal as @TimothyBJacobs said, via:

PHP:
remove_theme_support( 'font-library' );

theme.json:
settings.typography.fontLibrary = false
(this appears to be the appropriate path, but correct me if wrong)

Or if it's more about the user than the feature itself, capabilities as @swissspidy suggests.

@mikachan
Copy link
Member

mikachan commented Jan 8, 2024

Thanks all! This is really helpful.

It's better to handle this with new capabilities and/or filters.

We're discussing creating potential new capabilities in #55280, perhaps install_fonts, edit_fonts, and uninstall_fonts. Are these the types of capabilities you had in mind?

I think it might make sense for this to be a theme support that can be removed.

I think the ability to manage fonts is probably more about the individual user, however I like the idea of a theme support too, as this gives an additional level of control, especially for theme builders. Would it be appropriate to create both the new capabilities as well as the theme support removal?

@matiasbenedetto
Copy link
Contributor Author

matiasbenedetto commented Jan 8, 2024

it might make sense for this to be a theme support that can be removed

PHP:
remove_theme_support( 'font-library' );

theme.json:
settings.typography.fontLibrary = false

Turning off the Font Library at a theme level, in my opinion, doesn't make 100% sense because the feature to install and remove fonts is independent of the active theme. Only one part of the feature (activating and de-activating fonts) is theme-dependent via Global Styles.

with new capabilities

Roles and capabilities are tightly related to particular users. I think that the idea is to be able to turn off the feature entirely, system-wide, for all the users, so a new capability doesn't seem to be the best option to do that. I'm not saying we should not create a new user capability as we are discussing this issue, but turning off the feature system-wide seems a different requirement.

and/or filters

I think having a filter, as suggested by @swissspidy, seems to be the best way to turn off the feature globally. Having something similar to use_block_editor_for_post filter seems like a good option for this purpose due to its clarity.

Example: To disable the block editor for all post types, extenders can use the following:

add_filter('use_block_editor_for_post', '__return_false');

Following this prior art existing in core, could we disable the Font Library with something like the following line?

add_filter('use_font_library', '__return_false');

@hellofromtonya
Copy link
Contributor

IMO a filter is the preferred approach.

As @matiasbenedetto noted, a filter is available to disable the block editor. Following that approach keeps consistency while providing extenders and site owners a mechanism to turn it off.

What about capabilities?
Capabilities for managing user tasks within the Font Library is covered by #55280. I agree this is not the best way though to globally turn off the Font Library for the website and, thus, for all users.

What about a constant?
I also agree to not add a constant. While this approach is okay for initial set up and testing with the Gutenberg plugin, it's not the preferred approach for WP Core.

@hellofromtonya
Copy link
Contributor

Thinking more about this ...

A constant defined in a site's wp-config.php makes sense when functionality needs to be customized before plugins / themes get loaded, i.e. early and throughout boot, load, and processing.

When does font processing start? If it's before must-use and/or plugins load, then a constant may be needed.

@matiasbenedetto
Copy link
Contributor Author

Should the flag to disable the Font Library should disable only the UI to use it from the editor or it should disable the PHP and API rest functionality too?

@azaozz
Copy link
Contributor

azaozz commented Jan 11, 2024

Constants are generally used because it makes it pretty easy to set things when (auto)installing WordPress. These are generally for bigger parts of WP that would affect how it works to a large extend. For example post revisions can be disabled with a constant (because on some hosting accounts they may overwhelm the DB?).

On the other hand, seems disabling the Font Library is not one of these more critical parts, so I agree a constant is probably not needed.

I think having a filter, as suggested by @swissspidy, seems to be the best way

+1. A filter should work there. Also a lot easier to test than a constant :)

Should the flag to disable the Font Library should disable only the UI to use it from the editor or it should disable the PHP and API rest functionality too?

Thinking that it should disable only the UI. This would solve several problems:

  • What happens if the Font Library was enabled on a site for some time and then disabled? Not disabling the back-end of it would ensure the site continues to look the way it used to, and doesn't break.
  • When the library is disabled by a theme or a plugin that implements some other UI or means for the users to select fonts. Preferably such themes/plugins would still use the back-end functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Typography Font and typography-related issues and PRs Needs Decision Needs a decision to be actionable or relevant [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
7 participants