Make WordPress Core

Opened 3 weeks ago

Last modified 6 days ago

#63007 accepted enhancement

Bundled themes: Stylesheets for block themes are missing path data for inlining

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 6.9 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch 2nd-opinion
Focuses: css, performance Cc:

Description (last modified by westonruter)

I'm investigating slow LCP caused by render delay due to external stylesheets. The Twenty Twenty-Five (T25) and Twenty Twenty-Two (T22) themes enqueue their own small theme stylesheets (e.g. 7 style rules in TT5 and 13 in T22). However, it does so without adding the path style data which means this small stylesheet cannot be inlined. This negatively impacts LCP.

Consider the following plugin code to test the performance impact of inlining the theme stylesheet as well as the stylesheet for the Navigation block which by default is not inlined since it is larger than the default inline style size limit of 20,000 bytes:

<?php
if ( isset( $_GET['styles_inline_size_limit'] ) ) {
        add_filter( 'styles_inline_size_limit', function () {
                return intval( $_GET['styles_inline_size_limit'] );
        } );
}

if ( isset( $_GET['inline_theme_stylesheet'] ) && rest_sanitize_boolean( $_GET['inline_theme_stylesheet'] ) ) {
        add_action(
                'wp_enqueue_scripts',
                function () {
                        wp_style_add_data(
                                'twentytwentyfive-style',
                                'path',
                                get_parent_theme_file_path( 'style.css' )
                        );
                },
                20
        );
}

With this plugin in place and a vanilla site with TT5 active with the LCP element being text (P), I tested the following scenarios:

  1. Neither inlined: The control behavior where the theme's stylesheet and the Navigation block's style sheet are not inlined.
  2. Theme inlined: The theme's stylesheet is inlined but the Navigation block's stylesheet remains external.
  3. Navigation inlined: The Navigation block's stylesheet is inlined but the theme's stylesheet remains external.
  4. Both inlined: Both the theme's stylesheet and the Navigation block's stylesheet are inlined.

These scenarios are accessed via the following URLs (tt5-style-inlining-urls.txt):

http://localhost:10063/?inline_theme_stylesheet=false
http://localhost:10063/?inline_theme_stylesheet=true
http://localhost:10063/?inline_theme_stylesheet=false&styles_inline_size_limit=40000
http://localhost:10063/?inline_theme_stylesheet=true&styles_inline_size_limit=40000

I benchmarked via benchmark-web-vitals with network emulating Slow 3G:

npm run research -- benchmark-web-vitals --file=tt5-style-inlining-urls.txt --number=25 --network-conditions="Slow 3G" --output=csv

Results:

Scenario LCP-TTFB (median)
Neither inlined 4196.1 ms
Theme inlined 4111.8 ms
Navigation inlined 4223.6 ms
Both inlined 2230.8 ms

Inlining the theme styles reduces the LCP by 84.3 ms, which isn't a whole lot relatively at just ~2%. Inlining just the theme styles or the Navigation block styles alone doesn't significantly impact LCP, but if both are inlined then the LCP is cut in half, from a poor value to a good one.

So I think that T25 and T22 should add the path data for its stylesheet so it is eligible for inlining so that there could be the possibility for all render-blocking stylesheets to be avoided.

Aside: When inlining a theme's style.css I think it would make sense to strip out the theme metadata comment automatically.

Attachments (1)

twentytwentyfivechild.zip (518 bytes) - added by sabernhardt 3 weeks ago.
minimal child theme

Download all attachments as: .zip

Change History (29)

This ticket was mentioned in PR #8392 on WordPress/wordpress-develop by @ankitkumarshah.


3 weeks ago
#1

  • Keywords has-patch added

Trac Ticket: #63007

## Description
Adds path data to the theme's style.css to enable inlining capabilities, which can improve the Largest Contentful Paint (LCP).

## Screenshot:

#### Before patch:

##### LCP:
https://github.com/user-attachments/assets/30f93a50-ad63-4c83-a024-b04a8e8d055b

#### Network:
https://github.com/user-attachments/assets/ed34ed81-a653-4a25-86c2-f13eb7577a46

### After patch:

##### LCP:
https://github.com/user-attachments/assets/cf5c6237-9bc5-46ed-9d88-0f65918b8e26

#### Network:
https://github.com/user-attachments/assets/f191b83f-1f1b-46d1-aff3-456295932a53

This ticket was mentioned in PR #8396 on WordPress/wordpress-develop by @westonruter.


3 weeks ago
#2

Trac ticket: https://core.trac.wordpress.org/ticket/63007

This allows for the Twenty Twenty-Five theme's style.css to be inlined so as to eliminate a render-blocking resource from the page. See Core-63007 for impact metrics on LCP.

This also introduces minification for style.css to reduce the size of the stylesheet from 2,505 bytes down to 585 bytes: a ~75% reduction. This is important when the overall limit for inlining CSS (styles_inline_size_limit) is by default 20,000 bytes. The same scheme is used for core, where the file extension.min.css is used for the minified files. (It would be nice if a sourcemap were generated as well.)

I'm not sure if this is the right place to introduce the minification, as perhaps it should be part of the theme's own build process. Also, ideally other themes would also have their stylesheets minified (see comment on Core-49665).

HTML Diff with SCRIPT_DEBUG on:

  • .html

    old new  
    1291145                        z-index: 100000;
    1301146                }
    1311147</style>
    132 <link rel='stylesheet' id='twentytwentyfive-style-css' href='http://localhost:8889/wp-content/themes/twentytwentyfive/style.css?ver=1.1' media='all' />
     1148<style id='twentytwentyfive-style-inline-css'>
     1149/*
     1150Theme Name: Twenty Twenty-Five
     1151Theme URI: https://wordpress.org/themes/twentytwentyfive/
     1152Author: the WordPress team
     1153Author URI: https://wordpress.org
     1154Description: Twenty Twenty-Five emphasizes simplicity and adaptability. It offers flexible design options, supported by a variety of patterns for different page types, such as services and landing pages, making it ideal for building personal blogs, professional portfolios, online magazines, or business websites. Its templates cater to various blog styles, from text-focused to image-heavy layouts. Additionally, it supports international typography and diverse color palettes, ensuring accessibility and customization for users worldwide.
     1155Requires at least: 6.7
     1156Tested up to: 6.7
     1157Requires PHP: 7.2
     1158Version: 1.1
     1159License: GNU General Public License v2 or later
     1160License URI: http://www.gnu.org/licenses/gpl-2.0.html
     1161Text Domain: twentytwentyfive
     1162Tags: one-column, custom-colors, custom-menu, custom-logo, editor-style, featured-images, full-site-editing, block-patterns, rtl-language-support, sticky-post, threaded-comments, translation-ready, wide-blocks, block-styles, style-variations, accessibility-ready, blog, portfolio, news
     1163*/
     1164
     1165/*
     1166 * Link styles
     1167 * https://github.com/WordPress/gutenberg/issues/42319
     1168 */
     1169a {
     1170        text-decoration-thickness: 1px !important;
     1171        text-underline-offset: .1em;
     1172}
     1173
     1174/* Focus styles */
     1175:where(.wp-site-blocks *:focus) {
     1176        outline-width: 2px;
     1177        outline-style: solid;
     1178}
     1179
     1180/* Increase the bottom margin on submenus, so that the outline is visible. */
     1181.wp-block-navigation .wp-block-navigation-submenu .wp-block-navigation-item:not(:last-child) {
     1182        margin-bottom: 3px;
     1183}
     1184
     1185/* Increase the outline offset on the parent menu items, so that the outline does not touch the text. */
     1186.wp-block-navigation .wp-block-navigation-item .wp-block-navigation-item__content {
     1187        outline-offset: 4px;
     1188}
     1189
     1190/* Remove outline offset from the submenus, otherwise the outline is visible outside the submenu container. */
     1191.wp-block-navigation .wp-block-navigation-item ul.wp-block-navigation__submenu-container .wp-block-navigation-item__content {
     1192        outline-offset: 0;
     1193}
     1194
     1195/*
     1196 * Progressive enhancement to reduce widows and orphans
     1197 * https://github.com/WordPress/gutenberg/issues/55190
     1198 */
     1199h1, h2, h3, h4, h5, h6, blockquote, caption, figcaption, p {
     1200        text-wrap: pretty;
     1201}
     1202
     1203/*
     1204 * Change the position of the more block on the front, by making it a block level element.
     1205 * https://github.com/WordPress/gutenberg/issues/65934
     1206*/
     1207.more-link {
     1208        display: block;
     1209}
     1210
     1211</style>
    1331212<link rel="https://api.w.org/" href="http://localhost:8889/wp-json/" /><link rel="EditURI" type="application/rsd+xml" title="RSD" href="http://localhost:8889/xmlrpc.php?rsd" />
    1341213<meta name="generator" content="WordPress 6.8-alpha-59274-src" />
    1351214<script type="importmap" id="wp-importmap">

HTML Diff with SCRIPT_DEBUG off:

  • .html

    old new  
    129129                       z-index: 100000;
    130130               }
    131131</style>
    132 <link rel='stylesheet' id='twentytwentyfive-style-css' href='http://localhost:8889/wp-content/themes/twentytwentyfive/style.css?ver=1.1' media='all' />
     132<style id='twentytwentyfive-style-inline-css'>
     133a{text-decoration-thickness:1px!important;text-underline-offset:.1em}:where(.wp-site-blocks :focus){outline-width:2px;outline-style:solid}.wp-block-navigation .wp-block-navigation-submenu .wp-block-navigation-item:not(:last-child){margin-bottom:3px}.wp-block-navigation .wp-block-navigation-item .wp-block-navigation-item__content{outline-offset:4px}.wp-block-navigation .wp-block-navigation-item ul.wp-block-navigation__submenu-container .wp-block-navigation-item__content{outline-offset:0}blockquote,caption,figcaption,h1,h2,h3,h4,h5,h6,p{text-wrap:pretty}.more-link{display:block}
     134</style>
    133135<link rel="https://api.w.org/" href="http://localhost:8889/wp-json/" /><link rel="EditURI" type="application/rsd+xml" title="RSD" href="http://localhost:8889/xmlrpc.php?rsd" />
    134136<meta name="generator" content="WordPress 6.8-alpha-59274-src" />
    135137<script type="importmap" id="wp-importmap">

@westonruter commented on PR #8392:


3 weeks ago
#3

Oh! Sorry, I didn't see your PR. I opened one as well, but also included minification off the style.css which I think is also important: https://github.com/WordPress/wordpress-develop/pull/8396

#4 @westonruter
3 weeks ago

Related: Comment on #49665 about whether we should add CSS minification for core themes generally.

#5 follow-up: @spacedmonkey
3 weeks ago

There is a similar ticket in #58519. This was closed by @karmatosed. These tickets are very similar would like to understand reasoning behind that ticket's closure and this ticket going ahead.

#6 in reply to: ↑ 5 @sabernhardt
3 weeks ago

There is a similar ticket in #58519.

No, a proposal to inline and minify Twenty Twenty-Two's stylesheet would be very similar to this ticket.

The blocks.css and dark color stylesheets are significantly larger, and I would not want those inline without minification. I also requested a performance metrics comparison for loading a second page (in addition the first) because browsers would already have the external file cached.

#58519 was closed simply because fixing bugs was/is a higher priority, especially for the older themes.

@sabernhardt
3 weeks ago

minimal child theme

#7 follow-up: @sabernhardt
3 weeks ago

I made a simple child theme to verify that the theme enqueues the (minified) parent stylesheet, and the current PR 8396 correctly loaded the TT5 styles inline for me with and without SCRIPT_DEBUG (but you could test again in case I missed something).

Last edited 3 weeks ago by sabernhardt (previous) (diff)

#8 in reply to: ↑ 7 @westonruter
3 weeks ago

  • Description modified (diff)
  • Summary changed from Twenty Twenty-Five: Theme stylesheet is missing path data for inlining styles to Bundled themes: Stylesheets for block themes are missing path data for inlining

Replying to sabernhardt:

There is a similar ticket in #58519.

No, a proposal to inline and minify Twenty Twenty-Two's stylesheet would be very similar to this ticket.

Oh! I missed that one. I'll update this ticket to actually include T22 in the scope. So this ticket would be about allowing inlining block themes' styles.

The blocks.css and dark color stylesheets are significantly larger, and I would not want those inline without minification.

Nevertheless, the blocks.css stylesheets you listed are all less than 20K bytes, the styles_inline_size_limit. So they would be eligible for inlining. The largest blocks.css is from Twenty Thirteen at 16,254 bytes. And we minify stylesheets in core themes (cf. comment from #49665), then this stylesheet is reduced to 11,299 bytes.

I also requested a performance metrics comparison for loading a second page (in addition the first) because browsers would already have the external file cached.

We should do this, but note that the impact to an increase to the size of the HTML in the second page load will be lessened by Speculative Loading (#62503).

#58519 was closed simply because fixing bugs was/is a higher priority, especially for the older themes.

Nevertheless, I think a better approach for inlining classic themes' blocks.css would be to split up the stylesheet into multiple stylesheets, one for each block. Then if we can enable should_load_separate_core_block_assets for classic themes, then we can only enqueue the specific theme block styles which are actually used via wp_enqueue_block_style(). This would require the use of output buffering from #43258 (which I've just milestoned for 6.8), however, since we'd need to inject the CSS from the head after the rest of the template has been rendered. Maybe the themes' blocks stylesheets can't be split up so easily, however.

Replying to sabernhardt:

I made a simple child theme to verify that the theme enqueues the (minified) parent stylesheet, and the current PR 8396 correctly loaded the TT5 styles inline for me with and without SCRIPT_DEBUG (but you could test again in case I missed something).

Excellent. Yes, this is as expected since the get_parent_theme_file_uri() and get_parent_theme_file_path() functions are being used, not get_stylesheet_uri(). And the T22 theme is using get_template_directory_uri() . '/style.css' which is the same as doing get_parent_theme_file_uri( 'style.css' ) with the added benefit of also applying the parent_theme_file_uri filter in addition to the template_directory_uri filter.

So you approve the PR for commit?

#9 @westonruter
3 weeks ago

  • Owner set to westonruter
  • Status changed from new to accepted

#10 @poena
3 weeks ago

Please allow the bundled theme maintainers time to read through and review this before it is committed.

#11 @poena
3 weeks ago

Of course, I am not opposed to speed improvements.
But before this can be considered, the minification must be solved.

  • Which tools to use and where
  • Which documentation efforts are needed (where will it live?), and who will manage this task?

(high) No matter what tool that is used to minify the files, it will drastically increase the difficulty for users to use the theme as an example for their own projects.
Please don't dismiss this as a non-issue. It does increase what level of coding knowledge that is needed.

Adding build tools defeats the purpose of block themes, which are intended to be able to be created directly from the Site Editor and exported as new themes, without any coding knowledge.

(medium) No matter if the tooling is is added to the theme, or if the tools in core are used, it will drastically increase the difficulty for new contributors contributing back to the themes by submitting patches to Trac.

(low) It increases the maintenance burden of these files, which we have seen in the other bundled themes where the contributor needs to for example use SASS or build RTL files, and repeatedly submits incomplete pull requests.

#12 @westonruter
3 weeks ago

@poena I'm not sure I understand. The tools are already part of core using the existing build process (grunt build). The minified CSS files would not be committed to SVN, so patches would not include any changes to the minified files.

#13 @poena
3 weeks ago

No, you are right, I don't understand: The result of that would be theme versions where not all files are included, and the CSS would not be loading because its missing. Because it is not safe to assume what SCRIPT_DEBUG is set to on any given installation.
Not all users would download a built theme from the .org theme directory, or understand the difference.

#14 @westonruter
3 weeks ago

@poena The theme would always be distributed with the minified CSS files in the same way that core is distributed with the minified CSS files for wp-admin and wp-includes. If they downloaded the theme from GitHub via the build repo <https://github.com/WordPress/WordPress/tree/master/wp-content/themes> then they would get the minified files. They would also get the minified files from the SVN build repo at <https://core.svn.wordpress.org/trunk/wp-content/themes/>. How else would users normally download a theme? The only case where the they would not get the minified files is if they download a theme from a develop repo, namely <https://develop.svn.wordpress.org/trunk/src/wp-content/themes/> and <https://github.com/WordPress/wordpress-develop/tree/trunk/src/wp-content/themes>, but users are expected to do a build process for develop.

Last edited 3 weeks ago by westonruter (previous) (diff)

This ticket was mentioned in Slack in #core-performance by westonruter. View the logs.


3 weeks ago

#16 follow-up: @poena
3 weeks ago

But they are not expecting to need to build core files to use, test, or contribute to the themes.
It has never been done before, it has never been needed before, how would anyone expect it? Especially as a change to an existing theme.

What happens when a user takes Twenty Twenty-Five, makes some changes in the Site Editor, exports it, renames it and then adds new CSS to style.css? That is not a unique or rare use case. Block themes are built using a different perspective, they are meant to be extended by users not only developers.

#17 @sabernhardt
3 weeks ago

  • Keywords 2nd-opinion added
  • Milestone changed from 6.8 to Future Release
  • Type changed from defect (bug) to enhancement
  • Version 6.7 deleted

I do not fully understand the situation of users editing style.css for block themes, but this should not be committed in the 6.8 cycle.

#18 follow-ups: @swissspidy
3 weeks ago

@sabernhardt Why not, what's the rationale?

#19 @joemcgill
3 weeks ago

  • Milestone changed from Future Release to 6.9

I think this is worth pursuing, but also think that @poena's request that the Theme maintainers have time to review and give feedback to this feature before it's committed is worth respecting. Given that we're so close to the Beta1 deadline, punting this to 6.9 makes sense to me.

That said, if everyone agrees on an approach in time for 6.8 then I'd be happy for this to be moved back to the original milestone.

Last edited 3 weeks ago by joemcgill (previous) (diff)

#20 in reply to: ↑ 16 @westonruter
3 weeks ago

Replying to poena:

What happens when a user takes Twenty Twenty-Five, makes some changes in the Site Editor, exports it, renames it and then adds new CSS to style.css? That is not a unique or rare use case. Block themes are built using a different perspective, they are meant to be extended by users not only developers.

As I understand, changes to a block theme in the Site Editor do not result in changing any CSS file in the theme. Changes in the Site Editor result in changes to "Global Styles" which are not actual files but get dynamically rendered inline by PHP already. As far as I know, the only way a style.css file is modified is via the Theme File Editor, same as in a classic theme and same as any other theme file, whether CSS, PHP, or JS. (I could be wrong!)

#21 in reply to: ↑ 18 @sabernhardt
3 weeks ago

@poena, who knows block themes much better than I do, objected. We all need time to confirm whether these changes should be made and/or whether some less common situations would require adjusting the conditions.

#22 in reply to: ↑ 18 ; follow-up: @poena
3 weeks ago

I can't explain this better, and I would prefer if it was addressed and not dismissed.
If I am coming on as aggressive right now, it is because I am not feeling heard.

When the ticket was opened, it included this comment:
"I'm not sure if this is the right place to introduce the minification, as perhaps it should be part of the theme's own build process."

I added this as one point of my comment, in addition to documentation.

But then without further discussion it seems to have been decided that it must be the minification script that is in core. Which, would require knowledge of this script. I believe that is not an incorrect claim.

And the comment about documentation has not been addressed at all.

Secondly, I described a scenario where a user repurposes the theme and adds CSS to style.css.
It does not matter if the user uses a text editor or the built in theme file editor.

What matters is that style.css is not enqueued, instead the minified files is. And the user does not have knowledge of that they need to edit the minified files, or how, or knowledge about script debug.
So the users changes are not working, and it is not a positive experience for them.

Increasing the complexity of block themes is in direct opposition of the goal and purpose of block themes (Ideally, these themes should not have had any CSS at all). This is not an opinion that is only my own. This has impact on the project.

The situation and expectation is different for the classic bundled themes, some of them already have build tools, they already require advanced coding knowledge.

#23 in reply to: ↑ 22 @westonruter
3 weeks ago

Replying to poena:

I can't explain this better, and I would prefer if it was addressed and not dismissed.
If I am coming on as aggressive right now, it is because I am not feeling heard.

Sorry, I'm trying to understand. And sorry if I was pushing too hard to get this in this release. To me it seemed like an easy performance win, but I'm not a themes component maintainer so I defer to your expertise. I won't push for this to be part of 6.8 without the blessing of the component maintainers.

When the ticket was opened, it included this comment:
"I'm not sure if this is the right place to introduce the minification, as perhaps it should be part of the theme's own build process." [...]
But then without further discussion it seems to have been decided that it must be the minification script that is in core. Which, would require knowledge of this script. I believe that is not an incorrect claim.

Yeah, I included minification of the style.css files for the block themes here because when they are inlined they should be minified to ensure they don't exceed the styles_inline_size_limit limit. So perhaps #63012 should be addressed first as a dependency for this ticket.

I added this as one point of my comment, in addition to documentation. [...]
And the comment about documentation has not been addressed at all.

In regards to documentation, the documentation for testing a patch already instructs to run grunt build. So any additional documentation specifically for testing themes should include this if it doesn't already.

Secondly, I described a scenario where a user repurposes the theme and adds CSS to style.css.
It does not matter if the user uses a text editor or the built in theme file editor.

What matters is that style.css is not enqueued, instead the minified files is. And the user does not have knowledge of that they need to edit the minified files, or how, or knowledge about script debug.
So the users changes are not working, and it is not a positive experience for them.

If someone forks a theme and modifies the style.css then this indeed would be an issue. I described this in a previous comment on #49665 where I wondered if the forking scenario was a reason why minification hasn't been done in the past. Nevertheless, there are themes today that already have a build step for their CSS, namely Twenty Nineteen and Twenty Twenty-One. So technically if someone forks those themes they already should not be using the theme file editor to edit the style.css file directly but rather to edit the underlying SASS files and then run npm install && npm run build in the theme. For themes that use SASS and for those which have minified CSS files, there should probably be a big notice comment in the style.css that instructs the users to run npm run build after making a change, and that for themes using SASS that the change should be made to the SASS files directly. For minification, the comment can say something like:

/*
 * NOTICE: 
 * This file is served minified to improve performance. In order to see edits 
 * to this file on the frontend, either enable the `SCRIPT_DEBUG` constant or 
 * run `npm run build` to regenerate `style.min.css`. Alternatively, if you do
 * not want to use minified version, then remove the `style.min.css` version from 
 * being enqueued in `functions.php`.
 */

Every theme would need to include their own package.json which has the npm run build command that which would result in the minified files being generated so that they wouldn't have to rely on core's build process to generate those files.

Increasing the complexity of block themes is in direct opposition of the goal and purpose of block themes (Ideally, these themes should not have had any CSS at all). This is not an opinion that is only my own. This has impact on the project.

The situation and expectation is different for the classic bundled themes, some of them already have build tools, they already require advanced coding knowledge.

In regards to block themes, is the style.css related to the Site Editor in any way? As far as I know, the Site Editor only results in changes to "Global Styles" and doesn't touch the style.css at all.

#24 follow-up: @poena
3 weeks ago

There are pros and cons for both using the core tool and for placing it in the theme.

Using the tool that is already in core means that we, those participating in the discussion, needs to do very little, and long term, it is less to maintain.

Adding the tools to the themes means there are more files to manage and keep up to date. And I may be wrong but doing this usually falls on one or two people for every release.
The pros are that a beginner developer can keep using the themes as examples, and all they need is the built, distributed theme. They don't have to concern themselves with a development version of WordPress.

But what if its not the only option?
Removing the style.css meta data still has not been looked at.

Twenty Twenty-Five:
The last two styles in style.css could very likely be moved to theme.json.
Child themes would still inherit the style unless the child theme completely overrides the parent theme.json.

Twenty Twenty-Two:
This would require more manual testing to see if the styles can be moved or removed.

I am curios about this comment, that says that the CSS is temporary.
But the people who contributed to the theme are not active in the project anymore, so I don't expect an answer. Maybe @karmatosed remember if this was intended to be removed at a specific WordPress version.

/*
 * Alignment styles.
 * These rules are temporary, and should not be relied on or
 * modified too heavily by themes or plugins that build on
 * Twenty Twenty-Two. These are meant to be a precursor to
 * a global solution provided by the Block Editor.
 *
 * Relevant issues:
 * https://github.com/WordPress/gutenberg/issues/35607
 * https://github.com/WordPress/gutenberg/issues/35884
 */

And my least favorite, loading different CSS for different WP versions.

Button elements can now have hover styles in theme.json.
So this CSS is only needed on the versions of WP that don't support it.

/*
 * Button hover styles.
 * Necessary until the following issue is resolved in Gutenberg:
 * https://github.com/WordPress/gutenberg/issues/27075
 */

.wp-block-search__button:hover,
.wp-block-file .wp-block-file__button:hover,
.wp-block-button__link:hover {
	opacity: 0.90;
}
Last edited 3 weeks ago by poena (previous) (diff)

#25 in reply to: ↑ 24 @westonruter
3 weeks ago

Replying to poena:

There are pros and cons for both using the core tool and for placing it in the theme.
Using the tool that is already in core means that we, those participating in the discussion, needs to do very little, and long term, it is less to maintain.
Adding the tools to the themes means there are more files to manage and keep up to date. And I may be wrong but doing this usually falls on one or two people for every release.
The pros are that a beginner developer can keep using the themes as examples, and all they need is the built, distributed theme. They don't have to concern themselves with a development version of WordPress.

For the sake of both the core build process and for the sake of developers who fork the themes, how about the minification process be available in both? Granted, this is more relevant to #63012 which is specifically about minification for all themes. As seen in my PR, adding minification for bundled themes is trivial for using core's existing tooling since it's just a matter of adding paths to Gruntfile.js.

But for the sake of developers who fork a theme who do not have core's build tooling available, each theme could also have a package.json that also provides the same minification tooling. This would indeed be more to maintain, but it would address the concern of providing devs with the ability to keep the minified files updated after edits to the CSS source files. Nevertheless, as part of the release it could be core's tooling that is used for minification, so the tooling bundled with each theme wouldn't need to be maintained in order to actually perform a release.

But what if its not the only option?
Removing the style.css meta data still has not been looked at.

Being able to eliminate the CSS files entirely sounds great, although that would just eliminate the need for this ticket and not the overall need to minify CSS for all themes (#63012).

#26 follow-up: @poena
2 weeks ago

What other tools do you recommend for testing? Since this tool does not run on Windows.

As much as I want the themes to be faster, and as curious as I am about the bench mark testing tool, I do not have the bandwidth to also troubleshoot the tool.

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

#27 in reply to: ↑ 26 @westonruter
2 weeks ago

Replying to poena:

What other tools do you recommend for testing? Since this tool does not run on Windows.

As much as I want the themes to be faster, and as curious as I am about the bench mark testing tool, I do not have the bandwidth to also troubleshoot the tool.

Do you mean benchmark-web-vitals? It should work on Windows, as long as you have npm installed, same as using the build tooling in WordPress core or Gutenberg. I don't think you'll need to troubleshoot the tool. It won't be needed on an ongoing basis. Otherwise, you can use Chrome DevTools with network emulation set to "Slow 3G" and then look at the LCP metric which shows up in the Performance panel. This will be the same result. The benefit of the benchmark-web-vitals tool is that it can do many repeated tests and then compute the median result, which helps to eliminate the noise and variation that can happen with a single test.

Aside: As part of #63018 I'm planning to extend the benchmarking for check the impact for inlining more minified CSS on initial page-visits versus subsequent page visits with that CSS instead being in cached external stylesheeets.

#28 @westonruter
2 weeks ago

While waiting for this to move forward in core, here's a workaround plugin that minifies the Twenty Twenty-Five stylesheet and adds the path data so it can be inlined: https://gist.github.com/westonruter/09e553a7b66d1a2e68cd5a9ed351c59b

This may be helpful for performance testing.

Last edited 6 days ago by westonruter (previous) (diff)
Note: See TracTickets for help on using tickets.