Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.42k stars 1.99k forks source link

Global Styles: Font changes not saved #59259

Closed jerrysarcastic closed 2 years ago

jerrysarcastic commented 2 years ago

Quick summary

When using a theme with Global Styles, it seems that font selections are not saved. If you click the "Publish" button they appear in Gutenberg still, but if you reload the view it's returned to the default system fonts setting.

See video:

https://user-images.githubusercontent.com/1967448/146261112-2c2325e5-3200-4c28-9597-afe8ee5e3a7b.mp4

Note: publishing the page (in addition to publishing the style change) does not affect this issue. At no time does the live site show the font change. It's shown temporarily in the editor only, and reverts when you reload the screen.

Steps to reproduce

  1. start with test site using Global Styles for font selections
  2. open any page and change the fonts, then publish your font selection change
  3. observe that the new fonts are shown in Gutenberg
  4. observe fonts are unchanged on the live site
  5. reload Gutenberg and observe the fonts are set to system defaults

What you expected to happen

New font selections to be applied site-wide

What actually happened

No change to site fonts

Context

User report in https://wordpress.com/forums/topic/font-not-changing-3

Simple, Atomic or both?

Simple

Theme-specific issue?

Affects all Global Styles themes (I tested several) that I could see.

Browser, operating system and other notes

No response

Reproducibility

Consistent

Severity

Some (< 50%)

Available workarounds?

No response

Workaround details

No response

kosiew commented 2 years ago

Hi @jerrysarcastic ,

Thanks for reporting this and the detailed steps.

Triaging for developers' further investigation.

rinazrina commented 2 years ago

I got another report here: #4877016-zen

It's a simple site and it's using Rockfield theme.

After changing the Heading and Base Font via Global Styles, updating the page, and reloading, the fonts are back to the Theme Default.

I can reproduce this on the user's site, but not on my site with the same theme.

ai2n commented 2 years ago

Using Zoologist theme. Noticed the same thing is happening for Post/Comments/Query Title when you set the color: https://d.pr/v/1fpI1m

Might be related.

Edit: Nevermind, it's unrelated, opened a new issue here: https://github.com/Automattic/wp-calypso/issues/65398

dcoleonline commented 2 years ago

I think this is the same issue, but if not I'm happy to move it elsewhere

Theme: Payton

I could get the font to change for links from Styles > Typography > Links.

What failed was Styles > Typography > Text. When we tried to save a font there, it showed the change within the editor, but then it went back to the default option after saving.

I applied this CSS as a workaround:

body {
    font-family: "Overpass", sans-serif;
}
github-actions[bot] commented 2 years ago

Support References

This comment is automatically generated. Please do not edit it.

sudeepbaral commented 2 years ago

Another report where only the Heading Fonts were not changing. Shared CSS to force heading fonts as a workaround.

mpkelly commented 2 years ago

I am going to take a look now.

sudeepbaral commented 2 years ago

Another report where all fonts were not changing from Site Editor Theme: Meraki

wpshellbelle commented 2 years ago

36640861-hc

Another report. All font changes are not saving from the Site Editor. Provided CSS to help for now. If needed, I can create a new issue for Global Styles using the Site Editor.

ktyfuller604 commented 2 years ago

To add to the report above, the particular case wanted to use playfair-display as the font. However, that specific font wasn't loading, even with the CSS workaround. Suggested Baskerville instead.

edequalsawesome commented 2 years ago

Customer is still running into this issue, please follow-up with them through 5515832-zen

edequalsawesome commented 2 years ago

Also ran into this in 36593936-hc, requested follow-up through ticket:

5519249-zen

Robertght commented 2 years ago

After checking several themes, it looks like only these share this pattern:

https://wordpress.com/themes/filter/global-styles/

I looked around in Gutenberg's repo and I can see some other reports but nothing that resembles this.

@Automattic/cylon is this something your team can look into?

jamiepalatnik commented 2 years ago

Another report: 36751881-hc

Twenty Twenty-Two theme. Shared CSS as a workaround.

mpkelly commented 2 years ago

Hi @jerrysarcastic and @Robertght, I started looking at this again yesterday. I could reproduce it for Simple sites, but when I resumed work today, I could not. So, it was either fixed overnight, or maybe I forgot to hit the "Publish" button yesterday when testing it. The code for this feature lives in the editing-toolkit-plugin package, but there is no sign of a recent fix there. It could be that the fix was made elsewhere, and I will keep looking. In the meantime, can I ask one of you to try and reproduce the issue?

Here's a short video that shows this is working for a Simple site today: https://d.pr/v/g088xy

Robertght commented 2 years ago

Strange! That looks good here as well @mpkelly!

@jamiepalatnik or @edequalsawesome could you also check on your end, please?

Addison-Stavlo commented 2 years ago

I started looking at this again yesterday. I could reproduce it for Simple sites, but when I resumed work today, I could not.

I think @fullofcaffeine has been doing some related digging recently and implemented a fix for the headings and buttons block global styles, so that may be what you are seeing. D87291-code

edequalsawesome commented 2 years ago

@mpkelly I'm now able to save regular text styles on my simple test site using Blockbase (🎉 ), but Heading font styles are reset after saving -- you can see that in the GIF below:

2022-09-07 17 52 52

mpkelly commented 2 years ago

Hi @edequalsawesome, I can confirm there is still an issue as you found. There is a patch in progress that @fullofcaffeine is working on. I have just tested that and found that all text styles are saved correctly. So the issue should be resolved once that patch is released.

fullofcaffeine commented 2 years ago

Fixed in D87291-code. Feel free to re-open if the issue persists!

ClassicRKR27 commented 2 years ago

So we're still working with this user on the issue and I've found that there is still a stipulation on getting this working for them: 5510224-zd-woothemes

I found that when trying to change All of the Heading fonts, they are still not changing - with that said, if I select the specific header types (H1/H2/H3/etc...) it will change the font correctly as shown below:

https://user-images.githubusercontent.com/51870544/189484903-9f015a0a-a538-44ce-84f8-2f04fbc63d7e.mp4

Reopening for further investigation.

mpkelly commented 2 years ago

@ClassicRKR27, I was able to reproduce this using a Support Session. It only affects certain themes, like Pendant, which the customer is using, and not others, like Disco. The problem appears to be only for themes that include an option for "Headings (System Font)":

image

It looks like there is currently no way to inherit the "All" font for heading blocks on some themes. The solution should either be to make selecting "Default" clear the current selection and use the "All" value, or we need a clear button as we have on the Customizer Font Picker. You can see the problem and how the CSS is being applied here.

https://user-images.githubusercontent.com/6851384/189587538-b8b61215-8c2b-4408-ab43-064fcf624aab.mp4

As the default, and also if the user selects "Default" (or clears the set value somehow), I think the top CSS rule that is applying font-family: var(--wp--preset--font-family--heading-font); should not be output and the h1 should inherit the "All" font instead.

I am currently looking at a fix for this. While it is related, it is not the same issue fixed by @fullofcaffeine.

mpkelly commented 2 years ago

@ClassicRKR27, thanks for your bug report. I think this is a separate issue and looks to be related to how we handle the defaults, and also the default CSS variable names used across themes. I have created https://github.com/Automattic/wp-calypso/issues/67641 to track it. I will close this one down.