Make WordPress Core

Opened 2 weeks ago

Last modified 5 days ago

#54782 reopened defect (bug)

Default presets in use by default themes need to be updated

Reported by: oandregal Owned by: jorgefilipecosta
Milestone: 5.9.1 Priority: normal
Severity: normal Version: 5.9
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Description

Themes that use the same preset slugs as WordPress uses need to be updated.

From the devnote https://make.wordpress.org/core/2022/01/08/updates-for-settings-styles-and-theme-json/

The CSS for some of the presets defined by WordPress (font sizes, colors, and gradients) was loaded twice for most themes in WordPress 5.8: in the block-library stylesheet plus in the global stylesheet. Additionally, there were slight differences in the CSS in both places.

In WordPress 5.9 those were consolidated into a single place, the global stylesheet whose name is global-styles-inline-css that is now loaded for all themes in the front-end.

Each preset value generates a single CSS Custom Property and a class, as in:

/* CSS Custom Properties for the preset values */
body {
  --wp--preset--<PRESET_TYPE>--<PRESET_SLUG>: <DEFAULT_VALUE>;
  --wp--preset--color--pale-pink: #f78da7;
  --wp--preset--font-size--large: 36px;
  /* etc. */
}

/* CSS classes for the preset values */
.has-<PRESET_SLUG>-<PRESET_TYPE> { ... }
.has-pale-pink-color { color: var(--wp--preset--color--pale-pink) !important; } 
.has-large-font-size { font-size: var(--wp--preset--font-size--large) !important; }

For themes to override the default values they can use the theme.json and provide the same slug. Themes that do not use a theme.json can still override the default values by enqueuing some CSS that sets the corresponding CSS Custom Property.

Example (sets a new value for the default large font size):

:root {
 --wp--preset--font-size--large: <NEW_VALUE>;
}

Change History (37)

This ticket was mentioned in PR #2130 on WordPress/wordpress-develop by oandregal.


2 weeks ago

  • Keywords has-patch added

Part of fixing https://core.trac.wordpress.org/ticket/54782
Depends on https://github.com/WordPress/wordpress-develop/pull/2129 and https://core.trac.wordpress.org/ticket/54781

## How to test

  • Apply on this branch the PR at https://github.com/WordPress/wordpress-develop/pull/2129
  • Use the TwentyTwenty theme.
  • Create a post that contains 5 paragraphs with the following content:
    • Small (18)
    • Normal (21)
    • Default
    • Large (26.25)
    • Larger (32)
  • To each paragraph apply the font size that corresponds to its content: apply small to the 1st paragraph, normal to the 2nd, nothing to the third, large to the 4th, and larger to the 5th.

The expected result is that the font size for each paragraph has the assigned value, both in the editor and front-end.

#2 @oandregal
2 weeks ago

As far as I could test, there are three themes affected: TwentyNineteen, TwentyTwenty, and TwentyTwentyOne.

#3 @jorgefilipecosta
2 weeks ago

  • Owner set to jorgefilipecosta
  • Resolution set to fixed
  • Status changed from new to closed

In 52549:

Fix: Update some default presets in use by default themes to the new format.

Themes that use the same preset slugs as WordPress uses need to be updated.
From the devnote https://make.wordpress.org/core/2022/01/08/updates-for-settings-styles-and-theme-json/
The CSS for some of the presets defined by WordPress (font sizes, colors, and gradients) was loaded twice for most themes in WordPress 5.8: in the block-library stylesheet plus in the global stylesheet. Additionally, there were slight differences in the CSS in both places.
In WordPress 5.9 those were consolidated into a single place, the global stylesheet whose name is global-styles-inline-css that is now loaded for all themes in the front-end.
For themes to override the default values they can use the theme.json and provide the same slug. Themes that do not use a theme.json can still override the default values by enqueuing some CSS that sets the corresponding CSS Custom Property. This commit does the second for the twenty twenty theme.

Props oandregal.
Fixes #54782.

#4 @jorgefilipecosta
2 weeks ago

In 52550:

Fix: Update some default presets in use by default themes to the new format.

Themes that use the same preset slugs as WordPress uses need to be updated.
From the devnote https://make.wordpress.org/core/2022/01/08/updates-for-settings-styles-and-theme-json/
The CSS for some of the presets defined by WordPress (font sizes, colors, and gradients) was loaded twice for most themes in WordPress 5.8: in the block-library stylesheet plus in the global stylesheet. Additionally, there were slight differences in the CSS in both places.
In WordPress 5.9 those were consolidated into a single place, the global stylesheet whose name is global-styles-inline-css that is now loaded for all themes in the front-end.
For themes to override the default values they can use the theme.json and provide the same slug. Themes that do not use a theme.json can still override the default values by enqueuing some CSS that sets the corresponding CSS Custom Property. This commit does the second for the twenty twenty theme.

Props oandregal.
Merges [52549] to the 5.9 branch.
Fixes #54782.

#5 @jorgefilipecosta
2 weeks ago

  • Keywords dev-reviewed commit added
  • Milestone changed from Awaiting Review to 5.9

Backported to the 5.9 branch.

This ticket was mentioned in PR #2140 on WordPress/wordpress-develop by oandregal.


2 weeks ago

Trac ticket https://core.trac.wordpress.org/ticket/54782
Related https://github.com/WordPress/wordpress-develop/pull/2130

## How to test

  • Use the TwentyNineteen theme.
  • Create a post that contains 5 paragraphs with the following content:
    • Small (19.5)
    • Normal (22)
    • Default
    • Large (36.5)
    • Huge (49.5)
  • To each paragraph apply the font size that corresponds to its content: apply small to the 1st paragraph, normal to the 2nd, nothing to the third, large to the 4th, and huge to the 5th.

The expected result is that the font size for each paragraph has the assigned value, both in the editor and front-end.

#8 @oandregal
2 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm reopening as there's still two themes to fix: TwentyNineteen and TwentyTwentyOne.

#9 @hellofromTonya
2 weeks ago

  • Keywords has-patch dev-reviewed commit removed

Resetting keywords for ongoing work.

#10 @hellofromTonya
2 weeks ago

  • Component changed from General to Bundled Theme

#11 @desrosj
2 weeks ago

Wanted to copy over the concerns about [[52549-[52550] that I raised over in Slack.

These changes need to be tested in older versions of WordPress. This change will likely result in unstyled elements with adjusted class names when a site running an old version of WordPress updates the themes to the new version.

For default themes, changes need to be made with the assumption that this will happen (because it does very often). This change can be left safely in Twenty Twenty-Two, but the older themes will need this change reverted, or a fallback will need to be developed.

For Twenty Twenty-One, the minimum version of WP required is 5.3. All other default themes can potentially be active on WP 5.0 or earlier. I am not sure when the has-color related classes were introduced, but I'm pretty sure the has-font size ones have been around since 5.0 or 5.1, we'll have to test back to those versions.

This ticket was mentioned in PR #2154 on WordPress/wordpress-develop by oandregal.


2 weeks ago

  • Keywords has-patch added

Trac ticket https://core.trac.wordpress.org/ticket/54782
Follow-up to https://github.com/WordPress/wordpress-develop/pull/2130

WordPress 5.9 uses CSS Custom Properties to define the presets, and themes without a theme.json need to use them to set their value. However, the default themes need to work with older versions of WordPress that don't have the CSS Custom Properties, therefore we can't remove the existing regular CSS properties. This PR brings them back.

## How to test

Test this both in WordPress 5.9 and WordPress 5.7. In 5.9 the CSS Custom Properties are present for all themes and in 5.7 are not. This covers all the cases.

### Font sizes

  • Use the TwentyTwenty theme.
  • Create a post that contains 5 paragraphs with the following content:
    • Small (18)
    • Normal (21)
    • Default
    • Large (26.25)
    • Larger (32)
  • To each paragraph apply the font size that corresponds to its content: apply small to the 1st paragraph, normal to the 2nd, nothing to the third, large to the 4th, and larger to the 5th.

The expected result is that the font size for each paragraph has the assigned value, both in the editor and front-end.

## Colors

  • Use the TwentyTwenty theme.
  • Create a post.
  • Create a paragraph and apply custom color classes by pasting this text has-white-background-color has-black-color into the "Additional classes" text field of the "Advanced" section of the block sidebar.
  • Create a new paragraph and apply custom color classes by pasting this text has-black-background-color has-white-color into the "Additional classes" text field of the "Advanced" section of the block sidebar.

Via the browser devtools make sure that the first paragraph's color is rgb(0,0,0) and its background is rgb(255,255,255). The second paragraph should be the opposite.

#13 @oandregal
2 weeks ago

PRs for TwentyNineteen https://github.com/WordPress/wordpress-develop/pull/2140 and TwentyTwenty https://github.com/WordPress/wordpress-develop/pull/2154 are ready for review.

I've tested them in WordPress 5.9 and WordPress 5.7. The first has the default presets as CSS Custom Properties and the second does not. This suffices to test all the existing scenarios in which these themes may be executed.

Last edited 2 weeks ago by oandregal (previous) (diff)

This ticket was mentioned in PR #2158 on WordPress/wordpress-develop by oandregal.


2 weeks ago

Trac ticket https://core.trac.wordpress.org/ticket/54782
Related https://github.com/WordPress/wordpress-develop/pull/2154 https://github.com/WordPress/wordpress-develop/pull/2140

WordPress 5.9 uses CSS Custom Properties to define the presets, and themes without a theme.json need to use them to set their value. However, the default themes need to work with older versions of WordPress that don't have the CSS Custom Properties, therefore we can't remove the existing regular CSS properties.

## How to test

Test this both in WordPress 5.9 and WordPress 5.7. In 5.9 the CSS Custom Properties are present for all themes and in 5.7 are not. This covers all the cases.

### Font sizes

  • Use the TwentyTwentyOne theme.
  • Create a post that contains paragraphs with the following content:
    • Extra Small (16)
    • Small (18)
    • Normal (20)
    • Large (24)
    • Extra Large (40)
    • Huge (96)
    • Gigantic (144)
  • Use the font size control of each paragraph to apply the font size that corresponds to its content: apply "extra small" to the 1st paragraph, etc.

The expected result is that the font size for each paragraph has the assigned value, both in the editor and front-end.

#15 @oandregal
13 days ago

TwentyTwentyOne PR is ready as well https://github.com/WordPress/wordpress-develop/pull/2158

As far as I've checked, no other default theme needs changes. These are ready for review/commit.

cc @desrosj @hellofromTonya @jorgefilipecosta

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


9 days ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


9 days ago

#18 @costdev
9 days ago

Test Report

Env

  • WordPress: PR 2154
  • Chrome 97.0.4692.71
  • Windows 10
  • Theme: Twenty Twenty
  • Block Editor
  • Plugin: None activated

Steps to test

See PR 2154

Results

Font sizes

  • Paragraph 1: Has size 18px font in both the editor and frontend. ✅
  • Paragraph 2: Has size 21px font in both the editor and frontend. ✅
  • Paragraph 3: Has the default font size in both the editor and frontend. ✅
  • Paragraph 4: Has size 26.25px font in both the editor and frontend. ✅
  • Paragraph 5: Has size 32px font in both the editor and frontend. ✅

Colors

  • Paragraph 1:
    • Has rgb(0,0,0) as the font color in both the editor and frontend. ✅
    • Has rgb(255,255,255) as the background color in both the editor and frontend. ✅
  • Paragraph 2:
    • Has rgb(255,255,255) as the font color in both the editor and frontend. ✅
    • Has rgb(0,0,0) as the background color in both the editor and frontend. ✅

#19 @pbearne
9 days ago

https://github.com/WordPress/wordpress-develop/pull/2140
TwentyNineteen is not working
I get these font sizes

Small (19.5) = 16px
Normal (22) = 18px
Default = 18px
Large (36.5) = 28px
Huge (49.5) = 36px

#20 @prbot
9 days ago

hellofromtonya commented on PR #2154:

In testing on WP 5.8.3, the fonts do not render the same in the block editor and front-end. The front-end is correctly using the preset font size. The block editor though is using the px setting which is inline and overriding the preset. Notice deactivating the inline px font size in the element changes the rendered size as it switches to the present 0.842em.

https://i0.wp.com/user-images.githubusercontent.com/7284611/149987367-87b1fafb-2887-45f4-a27e-a783cb5f359c.gif

Test Env:

  • OS: MacOS
  • Localhost: Local
  • WP Version: 5.8.3
  • Plugins: None

#21 @prbot
9 days ago

hellofromtonya commented on PR #2154:

Tested with 5.9 current branch and RC2. The fonts are rendering the same in both the block editor and front-end. In the block editor, the preset does take precedence over the font-size inline on the element. Why?

{{{css
.editor-styles-wrapper .has-small-font-size {

font-size: var(--wp--preset--font-size--small) !important;

}
}}}
https://i0.wp.com/user-images.githubusercontent.com/7284611/149989565-88aead55-38c4-47f0-9662-04afff951037.png

#22 @prbot
8 days ago

hellofromtonya commented on PR #2154:

# Testing Color Presets

## 5.9

Front-end and block editor render the same.

Works as expected ✅

https://i0.wp.com/user-images.githubusercontent.com/7284611/149994484-2e6d3136-9f3b-4cdc-85a5-7805a0b15b1e.png

## 5.6.7, 5.7.5, 5.8.3

Front-end and block editor render the same ✅

For has-white-background-color has-black-color classes:

{{{css
:root .editor-styles-wrapper .has-black-color {

color: #000;

}
:root .editor-styles-wrapper .has-white-background-color {

background-color: #fff;

}

:root .has-black-color {

color: #000;

}

:root .has-white-background-color {

background-color: #fff;

}

.has-black-color {

--wp--preset--color--black: #000;

}
.has-white-background-color {

--wp--preset--color--white: #fff;
color: #000;

}
}}}

For has-black-background-color has-white-color classes:

{{{css
:root .editor-styles-wrapper .has-white-color {

color: #fff;

}
:root .editor-styles-wrapper .has-black-background-color {

background-color: #000;

}

:root .has-black-background-color {

background-color: #000;

}

.has-white-color {

--wp--preset--color--white: #fff;

}
.has-black-background-color {

--wp--preset--color--black: #000;
color: #fff;

}
}}}
https://i0.wp.com/user-images.githubusercontent.com/7284611/149996602-a2ec20ee-cd7c-418c-ab72-d17e005aae7f.png

#23 @prbot
8 days ago

hellofromtonya commented on PR #2154:

The presets and fallbacks are working with the exception of:

  • Presets for the "larger" font size
  • Font sizes not taking precedence in the block editor on < 5.9 (though not the problem existed before this PR and adding presets)

@oandregal and @desrosj What do you think?

#24 @hellofromTonya
8 days ago

  • Keywords needs-testing added

Adding notes from @desrosj during today's Test Team session:

I haven’t gotten to fully dive in and understand the moving parts yet, and I need to step away for a few hours. But, here’s my thinking and questions so far:

  • Themes, while bundled in Core, are separate. And can be released independently of WordPress versions.
  • If this change is not included, is there an unsatisfactory experience? Are sizes off? Does the site owner lose control of their content and how it looks?
  • I think we could probably make an additional change to the older default themes (pre TT2) after RC3 and not require an additional RC, as long as we replace that with sufficient testing. There’s a very low number of test sites for 5.9 RC/Beta, and I’d bet that the overwhelming majority of these are running TT2 and not the older themes we’re targeting for fixes here.
  • I could also see releasing these themes without this change (revert the original change), and then plan on a fast follow update for the affected themes.

Testing results:

Let's wait until after RC3 to discuss and further test as well as make decisions on which themes to update or possibly revert.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


8 days ago

#26 @prbot
8 days ago

oandregal commented on PR #2154:

In the block editor, the preset does take precedence over the font-size inline on the element. Why?

This is by design. The inline styles in the editor only exist to cover themes that don't add the CSS of their preset in the editor. In this case, what happens is that the CSS class of the preset has higher priority, so it's fine

#27 @prbot
8 days ago

hellofromtonya commented on PR #2154:

In the block editor, the preset does take precedence over the font-size inline on the element. Why?

This is by design. The inline styles in the editor only exist to cover themes that don't add the CSS of their preset in the editor. In this case, what happens is that the CSS class of the preset has higher priority, so it's fine

Is okay then that this theme's font-size (whether by preset or fallback) does not take affect in the block editor, meaning that the rendering is different in the block editor than in the front-end? See it in action here https://github.com/WordPress/wordpress-develop/pull/2154#issuecomment-1015645920

#28 @prbot
8 days ago

oandregal commented on PR #2154:

Is okay then that this theme's font-size (whether by preset or fallback) does not take affect in the block editor, meaning that the rendering is different in the block editor than in the front-end? See it in action here #2154 (comment)

Yeah, it works as it worked before that PR, nothing in this PR is changed for that matter.

#29 @prbot
7 days ago

hellofromtonya commented on PR #2154:

Hey @desrosj, I think this PR resolves the issues you raised. I tested back to WP 5.6.7. Any other concerns or testing needed?

#30 @oandregal
7 days ago

@pbearne could you share your environment to reproduce what you're experiencing in testing https://github.com/WordPress/wordpress-develop/pull/2140?

I've tested it in the latest 5.9, in 5.8.3, and in 5.7.5. Both with Gutenberg and without Gutenberg active. In editor and front. It works fine for me in every scenario.

For the record, I do see some small differences between editor & frontend for normal and large font sizes, but they are previous to this PR. In the front, normal is ~24px and not 22px because the theme is using ems; large is ~37px and not 36.5px for the same reason.

#31 @pbearne
6 days ago

@oandregal I pulled your branch into the WP core docker build

#32 @prbot
6 days ago

ockham commented on PR #2140:

Testing with 5.9 (by building this branch, and running it according to the instructions).

|Font size|Editor|Frontend|

|

| Small (19.5) | 19.5 | 19.5 |
| Normal (22) | 22 | 24.75 |
| Default | 22 | 22 |
| Large (36.5) | 37.125 | 37.125 |
| Huge (49.5) | 49.5 | 49.5 |

I.e. I see slight discrepancies for Large in both the editor and the frontend, and for Normal on the frontend.

#33 @prbot
6 days ago

oandregal commented on PR #2140:

Those discrepancies for normal and large are a result of how the theme is built: using px in some places and ems in other places and were present before this PR. That's an ok result.

#34 @prbot
6 days ago

ockham commented on PR #2140:

Testing with 5.7, by doing the following:

On this branch:

git diff $(git merge-base trunk $(git rev-parse --abbrev-ref HEAD)) > ../default-presets-twentynineteen.patch

Then switch to the 5.7 branch, apply the patch, and rebuild as follows:

git checkout 5.7
patch -p1 < ../default-presets-twentynineteen.patch.patch
npm i
npm run build:dev

Results:

|Font size|Editor|Frontend|

|

| Small (19.5) | 19.5 | 19.5 |
| Normal (22) | 22 | 24.75 |
| Default | 22 | 22 |
| Large (36.5) | 36.5 | 37.125 |
| Huge (49.5) | 49.5 | 49.5 |

I.e. same small discrepancies on the frontend as for WP 5.9, but accurate values in the editor.

#35 @desrosj
5 days ago

  • Milestone changed from 5.9 to 5.9.1

Since this still seems to need a little more work, @hellofromTonya and I have discussed and decided to revert these changes.

Themes can be released independent of WordPress versions, so whenever this is ready, we can discuss a new release for these themes.

#36 @desrosj
5 days ago

In 52615:

Bundled Themes: Reverts [52549].

[52549] updated some default presets in use by default themes to the new format. However, this change would cause elements to lose their styling when active on an older version of WordPress.

See #54782.

#37 @desrosj
5 days ago

In 52616:

Bundled Themes: Reverts [52549].

[52549] updated some default presets in use by default themes to the new format. However, this change would cause elements to lose their styling when active on an older version of WordPress.

Merges [52615] to the 5.9 branch.
See #54782.

Note: See TracTickets for help on using tickets.