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: make font collection fields translatable #59256

Merged
merged 4 commits into from Feb 22, 2024

Conversation

matiasbenedetto
Copy link
Contributor

What?

Porting changes back from WordPress core PR already merged: WordPress/wordpress-develop#6130

Copy link

github-actions bot commented Feb 21, 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.

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

Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
Co-authored-by: vcanales <vcanales@git.wordpress.org>
Co-authored-by: creativecoder <grantmkin@git.wordpress.org>
Co-authored-by: pbking <pbking@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>

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

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
❔ lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php
❔ lib/compat/wordpress-6.5/fonts/class-wp-font-library.php
❔ lib/compat/wordpress-6.5/fonts/class-wp-rest-font-collections-controller.php
❔ lib/compat/wordpress-6.5/fonts/fonts.php

Copy link

github-actions bot commented Feb 21, 2024

Flaky tests detected in c79602f.
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/8002415091
📝 Reported issues:

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.

LGTM

@matiasbenedetto matiasbenedetto enabled auto-merge (squash) February 21, 2024 20:57
wp_register_font_collection(
'google-fonts',
array(
'name' => _x( 'Google Fonts', 'font collection name' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these strings need the 'gutenberg' text domain as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added here: 450f9f5

@pbking
Copy link
Contributor

pbking commented Feb 21, 2024

image

If I pull out the category from the collection it's missing from the drop down. (Above Sans Serif was removed.) I suspect that's as-expected, and that's similar behavior to before this change. But with the category specifics now being provided separately from the font face assignment to those collections I just wanted to make sure.

If the above is as expected then this tests well for me.

@matiasbenedetto
Copy link
Contributor Author

If I pull out the category from the collection it's missing from the drop down.

@pbking yes, that's expected.

Copy link
Contributor

@creativecoder creativecoder left a comment

Choose a reason for hiding this comment

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

Thanks for adding the domain to the new text strings.

@getdave getdave added the Backport from WordPress Core Pull request that needs to be backported to the a Gutenberg release from WordPress Core label Feb 22, 2024
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: [Type] Enhancement, Backport from WordPress Core, [Feature] Font Library.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

@getdave getdave force-pushed the port/translatable-font-collection branch from 450f9f5 to c79602f Compare February 22, 2024 09:41
@getdave
Copy link
Contributor

getdave commented Feb 22, 2024

I rebased this and now hopefully all the tests will be ✅ and auto merge will handle.

@matiasbenedetto matiasbenedetto merged commit 8f1466f into trunk Feb 22, 2024
56 checks passed
@matiasbenedetto matiasbenedetto deleted the port/translatable-font-collection branch February 22, 2024 10:13
@github-actions github-actions bot added this to the Gutenberg 17.9 milestone Feb 22, 2024
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 the a Gutenberg release from WordPress Core [Feature] Font Library [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants