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

Newsletter's editor-font-sizes overrides non-FSE theme editor-font-sizes in WP 5.9.1 #755

Closed
laurelfulford opened this issue Mar 2, 2022 · 6 comments · Fixed by #1144
Closed
Assignees
Labels

Comments

@laurelfulford
Copy link
Contributor

laurelfulford commented Mar 2, 2022

This issue was originally reported here: Automattic/newspack-theme#1718

In WordPress 5.9.1, changes were made to make sure non-FSE themes would still be applying the correct editor-font-sizes coming from the theme, and not be overwritten by the new global styles and the !importants they include. The Newspack theme sets these font sizes here.

Related WP change: WordPress/wordpress-develop#2233

The Newsletter plugin also sets editor-font-sizes; when this plugin is activated, it looks like those editor-font-sizes are now being applied to the theme and post/page editor, rather than the ones coming from the theme.

In the theme, the editor-font-sizes are:

small - 16px
normal - 20px
large - 36px
huge - 44px

In the Newsletters plugin, the editor-font-sizes are:

small - 12px
normal - 16px
large - 24px
x-large - 36px

Where the size names overlap is where the problem happens -- in the theme, small goes from 16px to 12px, and though normal looks good on the front-end, it goes from 20px to 16px in the editor preview. large goes from 36px to 24px; the only one left alone is the largest size, because the slugs --- huge and x-large --- are different.

This is also happening with other non-FSE Gutenberg-supporting themes where the font size names overlap -- for example, in Twenty Twenty One, the Small font size goes from 16px to 12px when the Newsletter plugin is activated.

Steps to recreate:

  1. Start with a test site running WordPress 5.9.1, the Newspack theme and the Newspack Newsletters plugin.
  2. Deactivate the Newsletters plugin.
  3. Create a post or page, and add some heading blocks, and set to each of your preset font options, which will be listed as Small (16px), Normal (20px), Large (36px), and Huge (44px):

image

  1. Publish your post/page, and view on the front-end; confirm that your font sizes are being applied:

image

  1. Activate the Newsletters plugin.
  2. Refresh your post; note that the small and large headings are now smaller:

image

  1. Edit your post; note that the normal heading previews smaller in the editor, and that the font sizes have changed in the Typography panel:

image

  1. The Huge font size will seem unaffected, but if you click on it, it says it's using a Custom font -- this is because the CSS class in the theme (.has-huge-font-size) doesn't line up with the one in the plugin (.has-x-large-font-size).

image

Additional information

I have tried overriding the styles on the theme side by changing the CSS variable values; I can get it to work on the front-end, but in the editor it's a bit muddled -- the controls still show the smaller sizes (12px, 16px, 24px...) even though I can get the blocks displaying the correct theme sizes (16px, 20px, 36px...).

There also seem to be some wires crossed for specific font sizes: if you try to add a custom 12px font size to a block, it will display as 16px in the editor and on the front-end; ditto for 24px, it will jump up to 36px. My best guess is there's some "smart" functionality switching custom font sizes to existing preset font sizes, and since they don't match under the hood, it's messing up the font size:

font-resizing

I'm not sure if there is something we can change in the Newsletters plugin to un-mix these custom font sizes, or if it goes beyond what the plugin can reasonably control?

@dkoo
Copy link
Contributor

dkoo commented Mar 9, 2022

Is there a particular reason the font sizes in Newsletters are smaller? Why not update the sizes to match the theme?

@thomasguillot
Copy link
Contributor

Is there a particular reason the font sizes in Newsletters are smaller? Why not update the sizes to match the theme?

The default/normal (desktop) font-size for the theme is 20px.

Whereas the default/normal font-size for the newsletters is 16px. Unless we increase the font-size to 20px, it doesn't make sense to have a "small" setting set to the default font-size.

@laurelfulford
Copy link
Contributor Author

I wonder if it would work to rename the overlapping font slugs either in the theme or newsletters, and set up some CSS so the old class names styles would be maintained? I'm not sure how this would work on the newsletters side, but on the theme side, with the Global Styles dequeued, I think the only place the styles might still be mismatched is for the old classes is in the editor preview. I can experiment with this a bit to confirm.

This is probably getting ahead of things, but it looks like this will also be an issue when the theme is switched to a FSE theme, but flipped: the newsletter font sizes get overridden by the ones set in theme.json. Here's a screenshot of the font settings when using the newsletters plugin and Blockbase:

image

In this case it doesn't look like the font size names matter -- when I change them in the theme, the newsletters editor still picks them up. I'm guessing because there's a theme.json, the fallback that picks up the sizes set in editor-font-sizes is no longer run. I'm not sure if we want to "go with the flow" with this change, and just make sure the new theme includes a range of sizes that will work for both newsletters and the website, or try to figure out of there's another way to maintain a separate font size set when we switch to FSE. There isn't a rush to resolve this bit, but something to think about going forward!

@laurelfulford
Copy link
Contributor Author

I wonder if it would work to rename the overlapping font slugs either in the theme or newsletters, and set up some CSS so the old class names styles would be maintained?

A bit late on following up on this, but no, this doesn't do it 😐 Regardless if I change the theme or newsletter font names, I still end up with the newsletter font sizes in the theme editor.

It seems to have to do with the newsletter's editor fonts having a higher priority than the theme's -- if I remove the 11 here, the font sizes are correct in the editor for the theme, but are overriding the newsletter instead, and are wrong there.

@matticbot
Copy link
Contributor

🎉 This issue has been resolved in version 1.63.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This issue has been resolved in version 1.63.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants