Skip to content

Conversation

@pbking
Copy link
Contributor

@pbking pbking commented Mar 13, 2024

Fixes: #451

Copy ACTIVATED USER FONTS to the THEME when SAVED.

REMOVE UNACTIVATED FONTS from the THEME when SAVED.

To test:

Install Fonts to Theme

  • Install and Activate fonts from Google Fonts
  • 'Save' the theme with Create Block Theme
  • Note that the font assets are copied to the theme (/assets/fonts) and added to the theme.json
  • Note that the INSTALLED fonts have been DEACTIVATED (and can be safely deleted if desired)

Uninstall Fonts from Theme

  • Deactivate fonts using the Font Library UI
  • 'Save' the theme with Create Block Theme
  • Note that the deactivated fonts are removed from theme.json and font assets are removed from the asset folder

@pbking pbking requested a review from vcanales March 13, 2024 18:04
@pbking pbking marked this pull request as draft March 13, 2024 18:04
@pbking pbking marked this pull request as ready for review March 15, 2024 12:45
Vicente Canales and others added 4 commits March 20, 2024 18:17
Adding this because tests in this PR depend on a function —
wp_get_font_dir —  that was
introduced in 6.5
It isn't really working anyway.
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.

This is working well for me. LGTM.

run: |
mysql -uroot -h127.0.0.1 -e 'SELECT version()' \
&& ./bin/install-wp-tests.sh --recreate-db wordpress_test root '' > /dev/null \
&& ./bin/install-wp-tests.sh --wp-version=trunk --recreate-db wordpress_test root '' > /dev/null \
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that it didn't work on nightly; it worked here 420ad20

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Very strange! I didn't expect it to, I just noticed the || in the code and gave it a whirl.

It made zero sense but I ran with it!

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.

Saving Font Assets from native WP Font Management tooling

3 participants