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

Replace font management with screen pointing to native font library #539

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

madhusudhand
Copy link
Contributor

This change replaces the font management screen with links pointing to native font library.

image

This is an immediate change for 2.0
Clean up of the font management code is being done in #528

@madhusudhand madhusudhand self-assigned this Apr 3, 2024
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tinkered a bit with the copy, and made it translatable. LGTM

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, too! Great, simple design 👏

I don't think we have a better link than the 6.5 release post, we can always update this later if needed.

@madhusudhand madhusudhand merged commit 0630bf2 into trunk Apr 4, 2024
2 checks passed
@madhusudhand madhusudhand deleted the remove/font-library branch April 4, 2024 05:23
@t-hamano
Copy link
Contributor

t-hamano commented Apr 9, 2024

If I understand correctly, this looks like a breaking change.

  • If the user has already installed fonts previously via this plugin's font management, will these fonts be inherited by the native font library?
  • If fonts are installed through the font management of this plugin, is there a way to delete it from the UI?
  • What happens if a user has already activated a font in this plugin's font management and then activates the same font in the native font library?

@pbking
Copy link
Contributor

pbking commented Apr 9, 2024

If I understand correctly, this looks like a breaking change.

I consider this a "breaking change" in that the functionality to manage fonts is now done differently. (Which is why we are releasing it with a major version change.) However, any theme previously managed fonts with CBT (or any other method) is completely compatible with this change and can continue to be managed with CBT.

If the user has already installed fonts previously via this plugin's font management, will these fonts be inherited by the native font library?

Yes. If a user installed fonts via CBT 1.x then those assets are included in the theme and configured via theme.json. The native Font Library management system will understand and use that with or without CBT. Those fonts can be activated/deactivated with that tool. If CBT is used to further manage those fonts (combined with Core's Font Library management) then the theme exported will include those assets (for any activated font) and apply the correct configuration in theme.json just as it had before.

If fonts are installed through the font management of this plugin, is there a way to delete it from the UI?

Yes, any font that is deactivated via the Font Library UI will be removed from the theme when the theme is saved via CBT. (See instructions on this issue where the functionality was migrated).

What happens if a user has already activated a font in this plugin's font management and then activates the same font in the native font library?

To clarify CBT installs fonts in a theme (copies the assets to the appropriate folder and includes the configuration to use those assets in theme.json). The ability to "activate and deactivate" fonts is something that Core introduced in the Font Library. Both 1.x and 2.x versions of CBT allow fonts to be installed and uninstalled. To further answer your question, if a user has installed a font via the 1.x version of this plugin (or any other method) and a user attempts to install and activate the same font in the native font library they will be combined in the theme such that only one of each Font Face should be installed.

@pbking
Copy link
Contributor

pbking commented Apr 9, 2024

I will add that additional instructions and documentation as to how to achieve all of the above is very necessary!

@t-hamano
Copy link
Contributor

@pbking Thank you for letting me know in detail. I didn't know that there was already an implementation for backward compatibility. If CBT's font management was fully compatible with the native font library, I think it would be perfectly fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants