WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.5k stars 4.2k forks source link

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

Closed okmttdhr closed 8 months ago

okmttdhr commented 9 months ago

Description

There was an issue (https://github.com/Automattic/wp-calypso/issues/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 https://github.com/WordPress/gutenberg/issues/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:

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

https://github.com/WordPress/gutenberg/assets/5287479/5eea10dc-6a77-4b23-8a7a-3e3050689b22

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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!

getdave commented 8 months ago

built-in fonts with limited font weights

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

okmttdhr commented 8 months ago

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 commented 8 months ago

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

I'm proposing https://github.com/WordPress/gutenberg/pull/59066 to resolve the above issue 🙂

arthur791004 commented 8 months ago

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.

https://github.com/WordPress/gutenberg/assets/13596067/d43d6f91-c5bc-4b69-b727-e38911900d6b

Reproduced steps:

arthur791004 commented 8 months ago

https://github.com/WordPress/gutenberg/issues/58764#issuecomment-1947778291

Proposing https://github.com/WordPress/gutenberg/pull/59119 to resolve the above issue

matiasbenedetto commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

youknowriad commented 8 months ago

This has been addressed directly in Core.

matiasbenedetto commented 8 months ago

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: https://github.com/WordPress/gutenberg/pull/59376

annezazu commented 8 months ago

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

matiasbenedetto commented 8 months ago

Closing this now because it was fixed in core by https://github.com/WordPress/wordpress-develop/pull/6161 and the 6.4 proposed fix was closed following this rationale: https://github.com/WordPress/gutenberg/pull/59376