Activate the fonts coming from the backend and not the data from the frontend #60093
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
matiasbenedetto
added
[Type] Bug
matiasbenedetto
requested review from
creativecoder,
pbking,
youknowriad,
getdave and
mikachan
|
Size Change: +973 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
mikachan
reviewed
Mar 22, 2024
There was a problem hiding this comment.
I'm not sure this fixes #60040, as I'm testing using the instructions from that PR, using the Heineken Serif 18 font, and I'm still seeing the missing quotes in the preset:
If I add the quotes in manually the font renders correctly.
|
I tested the PR with the same font files that I used when testing the issue, and the name is wrapped in double quotes and works in my test. One space: Two spaces: Number and space: (I am using Chrome, macOS, WordPress 6.5 RC3 nightly.) |
|
I'm about to test this with the fonts provided as well. |
getdave
approved these changes
Mar 25, 2024
packages/edit-site/src/components/global-styles/font-library-modal/context.js
Show resolved
Hide resolved
|
@mikachan Could you possibly re-test? It seems like your result is the outlier and it would be good to know whether this is a legitimate failure 🙏 |
Co-authored-by: Dave Smith <[email protected]>
mikachan
approved these changes
Mar 25, 2024
There was a problem hiding this comment.
Thanks for the ping! I've re-tested and this PR is fixing the issue now 🎉
Just noting that my first round of testing was done by adding a single font file rather than multiple at once. However, I've just tested both ways (with a single font and with multiple at the same time) and it's definitely working. This is good to bring in I think!
|
I found that the font name is enclosed in double quotes with this PR. It is functioning properly.
|
|
I just had the time to test it too. It now works correctly for me with the PR. |
|
@mcsf You raised some potential concerns about the code of this PR in WP Slack. Are you able to review? |
getdave
reviewed
Mar 26, 2024
| @@ -402,14 +404,29 @@ function FontLibraryProvider( { children } ) { | |||
| }; | |||
|
|
|||
| const activateCustomFontFamilies = ( fontsToAdd ) => { | |||
| // Merge the existing custom fonts with the new fonts. | |||
| // Removes the id from the families and faces to avoid saving that to global styles post content. | |||
There was a problem hiding this comment.
I think we could explain here why we want to avoid saving to global styles post content.
Presumably the reason this code has been added now is because we're now using the font data from the API we're getting an id field which wasn't present previously.
There was a problem hiding this comment.
Although I think we can do that in a follow up.
getdave
added
the
Backport to Gutenberg RC
|
@bph I've marked this to be cherry picked into Gutenberg 18.0.0 in order that it will be open for consideration for inclusion WP 6.5. @matiasbenedetto @creativecoder Let's be sure to test the Gutenberg RC when it's ready to check that including this PR doesn't cause any regressions elsewhere in the Plugin. |
mcsf
approved these changes
Mar 26, 2024
| @@ -402,14 +404,29 @@ function FontLibraryProvider( { children } ) { | |||
| }; | |||
|
|
|||
| const activateCustomFontFamilies = ( fontsToAdd ) => { | |||
| // Merge the existing custom fonts with the new fonts. | |||
| // Removes the id from the families and faces to avoid saving that to global styles post content. | |||
|
There have been multiple reviews from contributors here and they are all ✅ I've gone ahead and merged this. |
|
This PR was triaged by Editor release leads and contributors in WP Slack as part of the final WordPress 6.5 process. See https://wordpress.slack.com/archives/C02QB2JS7/p1711371204886059 (requires registration). The criteria used was:
Conclusion: we propose that this fix is included in WP 6.5. Reason: bug affects the (blessed) Font Library feature in a severe way and this PR resolves in a clear and traceable manner. We now await a final decision from the full release team. |
bph
pushed a commit
that referenced
this pull request
Mar 26, 2024
…frontend (#60093) * Activate the fonts coming from the backend and not the data from the frontend * add comment ----- Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: mikachan <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: mcsf <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: huzaifaalmesbah <[email protected]> Co-authored-by: srueegger <[email protected]> Co-authored-by: annezazu <[email protected]>
|
I just cherry-picked this PR to the release/18.0 branch to get it included in the next release: 5ed745a |
bph
removed
the
Backport to Gutenberg RC
|
@matiasbenedetto Please could we look into adding some test coverage for the changes in this PR. This will increase confidence of release team in including this in WP 6.5. See related Slack discussion. |
getdave
added a commit
that referenced
this pull request
Mar 27, 2024
…frontend (#60093) * Activate the fonts coming from the backend and not the data from the frontend * add comment ----- Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: mikachan <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: mcsf <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: huzaifaalmesbah <[email protected]> Co-authored-by: srueegger <[email protected]> Co-authored-by: annezazu <[email protected]>
|
e2e test coverage is being worked on in #60221. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…frontend (WordPress#60093) * Activate the fonts coming from the backend and not the data from the frontend * add comment ----- Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: mikachan <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: mcsf <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: huzaifaalmesbah <[email protected]> Co-authored-by: srueegger <[email protected]> Co-authored-by: annezazu <[email protected]>
getdave
added
Backported to WP Core
|
I just cherry picked this into WP 6.5 RC 4 branch d2191d4 |
getdave
added a commit
that referenced
this pull request
Mar 27, 2024
…frontend (#60093) * Activate the fonts coming from the backend and not the data from the frontend * add comment ----- Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: mikachan <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: mcsf <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: huzaifaalmesbah <[email protected]> Co-authored-by: srueegger <[email protected]> Co-authored-by: annezazu <[email protected]>
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. git-svn-id: https://develop.svn.wordpress.org/trunk@57888 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/trunk@57888 git-svn-id: http://core.svn.wordpress.org/trunk@57389 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to gilzow/wordpress-performance
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/trunk@57888 git-svn-id: https://core.svn.wordpress.org/trunk@57389 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Reviewed by jorbin, swissspidy. Merges [57888] to the 6.5 branch. Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. git-svn-id: https://develop.svn.wordpress.org/branches/6.5@57891 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Reviewed by jorbin, swissspidy. Merges [57888] to the 6.5 branch. Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/branches/6.5@57891 git-svn-id: http://core.svn.wordpress.org/branches/6.5@57392 1a063a9b-81f0-0310-95a4-ce76da25c4cd
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
…frontend (WordPress#60093) * Activate the fonts coming from the backend and not the data from the frontend * add comment ----- Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: mikachan <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: mcsf <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: huzaifaalmesbah <[email protected]> Co-authored-by: srueegger <[email protected]> Co-authored-by: annezazu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Activate the fonts coming from the backend and not the data from the frontend
Why?
In some cases, as in #60040, the data coming from the frontend can have non-properly formatted the
font-familyproperty. In the case of the issue linkedHeineken Serif 18instead of"Heineken Serif 18".Fixes: #60040
How?
In this PR we use the font family and font face data coming from the API to activate the fonts (add to the Global Styles).
Testing Instructions
Screenshots or screencast
2024-03-21.16-46-32.mp4