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 Weight Conflicts Between Built-in and Installed Fonts #58764

Closed
okmttdhr opened this issue Feb 7, 2024 · 15 comments
Closed

Font Weight Conflicts Between Built-in and Installed Fonts #58764

okmttdhr opened this issue Feb 7, 2024 · 15 comments
Labels
[Feature] Font Library [Type] Bug An existing feature does not function as intended

Comments

@okmttdhr
Copy link
Contributor

okmttdhr commented Feb 7, 2024

Description

There was an issue (Automattic/wp-calypso#84590) related to the theme that provides built-in fonts with limited font weights. That issue seems to have been solved in Gutenberg ≥ 17.6, but new issues have arisen. These issues are observed when installing the same font as the built-in fonts provided by a theme.

Observed Issues:

  1. The built-in and installed fonts are treated as distinct fonts on the UI.
  2. Changes in font-weight are not immediately reflected on the canvas, requiring a page reload for updates to be visible. (It might be related to Uploaded Fonts Are Not Reflected in Editor Canvas Immediately #58765.)
  3. The default font-weight provided by the theme is altered upon installing a new font variant, which should not be changed.

Expected Behavior:

  • The UI should recognize and treat the built-in and installed fonts as the same.
  • Font updates, especially weight changes, should be immediately visible on the canvas without reloading the page.
  • Installing new font variants should not alter the default font-weight settings the theme provides.

Step-by-step reproduction instructions

  • Ensure the active theme is set to Adventurer.
  • Navigate to the Site Editor
  • Add an H1 block with “default” font-weight through the “Appearance” setting.
  • Add an H2 block with a “light” font-weight through the “Appearance” setting.
  • Initially, observe both H1 and H2 using the same font weight in the canvas.
  • Open the Font Library modal and install ALL font variants of the Rubik font.
  • Notice that the built-in font and the installed font are recognized as two separate entities in the Font Library modal. (Observed Issue 1)
  • Change the font weight again for H1 and H2 blocks and observe they still display the same font weight in the canvas. (Observed Issue 2)
  • Save changes and view the site front (outside the canvas) to see both H1 and H2 displaying the updated font-weight. (Observed Issue 3 for H1)
  • Reload the editor and observe both H1 and H2 now reflect the updated font weight on the canvas. (Observed Issue 3 for H1)
  • Open the Font Library modal and remove the Rubik font, noticing that the font weights for both H1 and H2 do not revert to their previous states. (issue 2)

Screenshots, screen recording, code snippet

Screen.Recording.2024-02-07.at.15.16.22.mov

Environment info

Gutenberg > 17.6

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

No

@arthur791004
Copy link
Contributor

arthur791004 commented Feb 7, 2024

The built-in and installed fonts are treated as distinct fonts on the UI.

It seems to be an existing issue as we never merge the same font family between different origins (defaults, themes, and custom) 🤔

But what it should be? The UI seems to be correct as one is from the theme and the other is installed by users 😂

Changes in font-weight are not immediately reflected on the canvas, requiring a page reload for updates to be visible.

The reason for this issue is the editor doesn't load the font variants that are being activated.

@okmttdhr
Copy link
Contributor Author

okmttdhr commented Feb 7, 2024

It seems to be an existing issue as we never merge the same font family between different origins (defaults, themes, and custom) 🤔
But what it should be? The UI seems to be correct as one is from the theme and the other is installed by users 😂

It makes sense to me 🙂 I'm starting to feel it's the correct behavior.

The reason for this issue is the editor doesn't load the font variants that are being activated.

This behavior appears to be exclusive to themes that provide built-in fonts with limited font weights. Given that, I suspect there may be an issue around activating fonts: https://github.com/okmttdhr/gutenberg/blob/30b434956321d95f2d4acef83605b85f7546d61e/packages/edit-site/src/components/global-styles/font-library-modal/context.js#L311-L312

@annezazu
Copy link
Contributor

annezazu commented Feb 7, 2024

Going to proactively add to the 6.5 board just to be safe. We'll have the beta period to address these and dig in more but I didn't want it to get lost in the shuffle!

@annezazu annezazu moved this from 📥 Todo to ❓ Triage in WordPress 6.5 Editor Tasks Feb 7, 2024
@getdave
Copy link
Contributor

getdave commented Feb 13, 2024

built-in fonts with limited font weights

Please could we define "built-in" fonts? Thank you.

@colorful-tones colorful-tones moved this from ❓ Triage to 🗣️ In Discussion / Needs Decision in WordPress 6.5 Editor Tasks Feb 13, 2024
@okmttdhr
Copy link
Contributor Author

built-in fonts with limited font weights

Please could we define "built-in" fonts? Thank you.

"built-in" fonts are the fonts that are provided by the theme, which is defined in theme.json. For instance, in the case of the Adventurer theme, the built-in fonts are defined in https://github.com/Automattic/themes/blob/631cca5/adventurer/theme.json#L52-L116. While these fonts are hosted and provided by the theme, the concept of "built-in" fonts isn't limited to that; it can also include fonts loaded from external sources, as specified by the theme's configuration.

The issue arises when a theme provides a limited selection of font variants (e.g., only certain weights of a font family) as "built-in". Users may attempt to enhance their font selection by installing additional font variants from Google Fonts that match the built-in font family, but they'll see this issue described above.

@arthur791004
Copy link
Contributor

Changes in font-weight are not immediately reflected on the canvas, requiring a page reload for updates to be visible.

I'm proposing #59066 to resolve the above issue 🙂

@arthur791004
Copy link
Contributor

I think the issue, Heading Font Weight Not Applying in Adventurer, is related to this one as well.

If a theme provides a font with ALL variants, and people install the same font with a specific variant, then only the installed variant will take effect as it overrides ALL variants provided by a theme.

font-library-installed-fonts-overrides-theme-fonts.mov

Reproduced steps:

  • Open a site with TT4
  • Go to the Editor
  • Create the following paragraph:
    • Cardo Normal. Set the Font Family to Cardo, and the Appearance to “Regular”
    • Cardo Bold. Set the Font Family to Cardo, and the Appearance to “Bold”
    • Cardo Normal Italic. Set the Font Family to Cardo, and the Appearance to “Regular Italic”
  • Install the Cardo font with 400 font variant from the Install Fonts tab of the Font Library Modal
  • Save changes and refresh the page
  • Make sure the rendered font of those paragraphs is Cardo-Regular

@arthur791004
Copy link
Contributor

#58764 (comment)

Proposing #59119 to resolve the above issue

@matiasbenedetto
Copy link
Contributor

Since this seems like it should be fixed in Wordpress core I opened a ticket in trac linking this issue: https://core.trac.wordpress.org/ticket/60605#ticket

@matiasbenedetto
Copy link
Contributor

Note: this issue doesn't seems to be related strictly with the Font Library. The font library made it visible because of the use of custom apart from theme theme.json data origin.

@matiasbenedetto
Copy link
Contributor

I'm proposing a fix to this issue with an alternative approach to #59119 directly in core repo to be able to leverage the unit tests for this functionality: WordPress/wordpress-develop#6161

@youknowriad
Copy link
Contributor

This has been addressed directly in Core.

@github-project-automation github-project-automation bot moved this from 🗣️ In Discussion / Needs Decision to ✅ Done in WordPress 6.5 Editor Tasks Feb 27, 2024
@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Feb 27, 2024

This has been addressed directly in Core.

@youknowriad I re-opened this issue because I think it still needs to be updated in Gutenberg for retro-compatibility (users running WordPress core < 6.4 with the latest Gutenberg release).

I'm moving the changes from core to compat/6.4 here:
#59376

@annezazu
Copy link
Contributor

Got it! Going to remove this from the 6.5 board in that case.

@matiasbenedetto
Copy link
Contributor

Closing this now because it was fixed in core by WordPress/wordpress-develop#6161 and the 6.4 proposed fix was closed following this rationale: #59376

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library [Type] Bug An existing feature does not function as intended
Projects
None yet
6 participants