-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add stylesheet minification for bundled themes (Block themes) #10081
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
4d543ef
d3beb63
96cc018
d2a352b
0e2085e
d479361
0f0df2f
2a1d893
f908cca
b83b6d0
3a9539d
b43e826
ff55cb6
8ceb867
a9e839c
39bac29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -567,6 +567,16 @@ module.exports = function(grunt) { | |
src: [ | ||
'wp-admin/css/colors/*/*.css' | ||
] | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of the other default themes are included. Is there a reason to only include these two? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in this PR only TT2 and TT5 have their stylesheets minified, other themes with a build step do not minify their stylesheets. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea is to start out with just minding the stylesheets for block themes since they are small and eligible to be inlined. Once we have a good workflow in place for block themes, we can follow up with minification for classic themes. However the performance gains won't be as large because the larger stylesheets wouldn't be eligible for inlining. A reduced size for an external stylesheet doesn't translate into as much of a performance boost as being able to eliminate that render blocking stylesheet in the first place, as I understand. |
||
themes: { | ||
expand: true, | ||
cwd: WORKING_DIR, | ||
dest: WORKING_DIR, | ||
ext: '.min.css', | ||
src: [ | ||
'wp-content/themes/twentytwentytwo/style.css', | ||
'wp-content/themes/twentytwentyfive/style.css', | ||
] | ||
} | ||
}, | ||
rtlcss: { | ||
|
@@ -1591,6 +1601,7 @@ module.exports = function(grunt) { | |
'rtl', | ||
'cssmin:rtl', | ||
'cssmin:colors', | ||
'cssmin:themes', | ||
'usebanner' | ||
] ); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
=== Contributing to Twenty Twenty-Five === | ||
|
||
= Minifying CSS = | ||
|
||
Twenty Twenty-Five has a single stylesheet `style.css` which is enqueued in addition to the global styles coming from core. On a normal production site, when `SCRIPT_DEBUG` is disabled, then the minified version `style.min.css` will be enqueued instead. If you make a change to `style.css`, you'll need to re-minify the `style.min.css` using the built-in npm build tool. As always, it is preferable to use the Site Editor to supply Additional CSS instead of directly editing the theme stylesheet. | ||
|
||
Installation instructions | ||
|
||
1. Using a command line interface, go to the “twentytwentyfive” directory `cd /my-computer/local-wordpress-install/src/wp-content/themes/twentytwentyfive`. | ||
|
||
2. Type `npm install` into the command line, and press the [return] key, to install all Node.js dependencies. | ||
|
||
3. The dependencies may take a few minutes to download but once it completes, you’re done. | ||
|
||
Usage instructions | ||
|
||
1. After making a change to the `style.css` file, run `npm run build` from within the theme directory to regenerate `style.min.css` with your new changes. | ||
|
||
2. You can also “watch” the theme directory for CSS changes and re-minify the CSS anytime a change occurs by running: `npm run watch`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will have the effect of never shipping the minified files since the files won't be in the checked out code.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, since the minified files are ignored, the ZIP files are being prepared from the repository without the minified files. See below:
wordpress-develop/.github/workflows/test-and-zip-default-themes.yml
Lines 216 to 230 in 73e822f
There are two options I can think of to resolve this:
We can include the minified files in the repo, similar to how classic themes require contributors to include the built CSS and CSS map files generated from SCSS files.
We could add a build step for TT2 and TT5 in the workflow for the
bundle-theme
job before theUpload theme ZIP as an artifact
step is run.This would work if the release of the bundled theme is handled through a GitHub workflow.
ref: https://make.wordpress.org/core/handbook/about/release-cycle/update-bundled-themes-2/#creating-theme-zip-files
cc: @westonruter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like the second option would be preferred, as I really don't like minified files being committed to the repo. For the classic themes using SASS it's a bit different because the changes to the
.scss
files at least result in changes to.css
files which can get a clean diff. But for minified files, and changes just results in a lot of noise.I will admit I don't have any experience with the theme release workflows, so I don't know specifically what is required to implement that second option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I drafted the new
bundle-theme
job steps with assistance from Gemini Code Assist: 39bac29There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the new job step is working correctly!
https://github.com/WordPress/wordpress-develop/actions/runs/18329688092?pr=10081
In the above action run, the generated artifacts now contain the minified files.
TT5: https://github.com/WordPress/wordpress-develop/actions/runs/18329688092/artifacts/4209767198
TT2: https://github.com/WordPress/wordpress-develop/actions/runs/18329688092/artifacts/4209767505