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

Moves font directory into uploads to match WP 6.5 #60354

Merged
merged 5 commits into from
Jun 30, 2024

Conversation

creativecoder
Copy link
Contributor

What?

Why?

  • Keep Gutenberg + WP 6.4 font library behavior consistent with WP 6.5
  • Ensure font assets uploaded before the directory change are deleted with their associated font face

How?

  • Updates the default font directory location returned from wp_font_dir/wp_get_font_dir
  • Adds a function on the before_delete_post hook that checks for font files in the old location and deletes them, if needed

Testing Instructions

  • Using trunk branch of Gutenberg with WP 6.4, install fonts from the font library in the site editor
  • See that those font files are installed to wp-content/fonts
  • Switch to this branch
  • Install another font and see that the fonts are uploaded to wp-content/uploads/fonts
  • Delete both sets of fonts you just installed
  • The font files from both wp-content/fonts and wp-content/uploads/fonts should be deleted

Fixes #59417

@creativecoder creativecoder added Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Feature] Font Library labels Apr 2, 2024
@creativecoder creativecoder self-assigned this Apr 2, 2024
@creativecoder creativecoder requested review from matiasbenedetto, peterwilsoncc and mikachan and removed request for spacedmonkey April 2, 2024 01:48
Copy link

github-actions bot commented Apr 2, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @joehoyle, @Zealth57.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: joehoyle, Zealth57.

Co-authored-by: creativecoder <[email protected]>
Co-authored-by: peterwilsoncc <[email protected]>
Co-authored-by: azaozz <[email protected]>
Co-authored-by: priethor <[email protected]>
Co-authored-by: hellofromtonya <[email protected]>
Co-authored-by: costdev <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: andreilupu <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: aaronjorbin <[email protected]>
Co-authored-by: cbirdsong <[email protected]>
Co-authored-by: mcsf <[email protected]>
Co-authored-by: jazzsequence <[email protected]>
Co-authored-by: joemcgill <[email protected]>
Co-authored-by: pwtyler <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: mtias <[email protected]>
Co-authored-by: justlevine <[email protected]>
Co-authored-by: samuelsidler <[email protected]>
Co-authored-by: johnbillion <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

github-actions bot commented Apr 2, 2024

Flaky tests detected in abbbb28.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9733811493
📝 Reported issues:

Base automatically changed from do/wp-dev-r57868-backport to trunk April 2, 2024 05:32
@creativecoder creativecoder force-pushed the update/move-font-directory-into-uploads branch from 89cad11 to 2303f63 Compare April 2, 2024 12:54
@azaozz
Copy link
Contributor

azaozz commented Apr 2, 2024

Thinking it may be better to hold on to this for couple of weeks (this change seems only relevant to new Gutenberg installs in WP 6.4). Lets see if there are any bugs caused by the changed location that would need fixing in a 6.5.1?

Also planning to work on implementing Josepha's decision from https://make.wordpress.org/core/2024/03/07/unblocking-wp6-5-font-library-and-synced-pattern-overrides/ in WP 6.6. This will set the default location back to wp-content/fonts.

Copy link
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

This looks good to me and matches the changes in WordPress-Develop.

I've requested some docblocks but preapproving.

The PHP unit tests are failing due to an issue pulling docker images, have you managed to get them passing locally?

* @param int $post_id Post ID.
* @param WP_Post $post Post object.
*/
function gutenberg_before_delete_font_face( $post_id, $post ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch, I would have totally missed this.

*
* @covers ::gutenberg_before_delete_font_face
*/
class Tests_Font_Delete_Files_From_Wp_Content_Folder extends WP_UnitTestCase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you able to add docblocks for the methods and properties throughout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏼 done in abbbb28

Copy link

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
❔ phpunit/tests/fonts/font-library/fontDeleteFilesFromWpContentFolder.php
❔ lib/compat/wordpress-6.5/fonts/fonts.php

@creativecoder creativecoder force-pushed the update/move-font-directory-into-uploads branch from 9980d08 to abbbb28 Compare June 30, 2024 17:37
@creativecoder
Copy link
Contributor Author

I'm planning to go ahead with merging this. We can regroup as needed if/when the fonts folder is changed again.

@creativecoder creativecoder merged commit b76965d into trunk Jun 30, 2024
62 checks passed
@creativecoder creativecoder deleted the update/move-font-directory-into-uploads branch June 30, 2024 21:52
@github-actions github-actions bot added this to the Gutenberg 18.8 milestone Jun 30, 2024
@peterwilsoncc
Copy link
Contributor

Thanks Grant, I'll create a follow up PR to backport WordPress/wordpress-develop@a033cf1 now it is unblocked.

carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jul 18, 2024
* Updates default font directory location to wp-content/uploads/fonts when using Gutenberg with WP 6.5 and below
* Deletes font files from wp-content/fonts when font face posts are deleted

Unlinked contributors: joehoyle, Zealth57.

Co-authored-by: creativecoder <[email protected]>
Co-authored-by: peterwilsoncc <[email protected]>
Co-authored-by: azaozz <[email protected]>
Co-authored-by: priethor <[email protected]>
Co-authored-by: hellofromtonya <[email protected]>
Co-authored-by: costdev <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: andreilupu <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: aaronjorbin <[email protected]>
Co-authored-by: cbirdsong <[email protected]>
Co-authored-by: mcsf <[email protected]>
Co-authored-by: jazzsequence <[email protected]>
Co-authored-by: joemcgill <[email protected]>
Co-authored-by: pwtyler <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: mtias <[email protected]>
Co-authored-by: justlevine <[email protected]>
Co-authored-by: samuelsidler <[email protected]>
Co-authored-by: johnbillion <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Feature] Font Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reconsider placing uploaded fonts within the uploads directory
3 participants