Opened 13 months ago
Closed 12 months ago
#59466 closed defect (bug) (fixed)
Twenty Twenty-Four custom button block style path incorrectly registered
Reported by: | flixos90 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | 6.4 |
Component: | Bundled Theme | Keywords: | has-patch has-testing-info |
Focuses: | performance | Cc: |
Description
As noted in #59465, a render-blocking button-outline.css
file from TT4 is enqueued, even though that CSS file is so small it should be inlined.
Looking at the TT4 code, it does intend to pass the stylesheet's path to WordPress core, so that core can inline the file contents, however the way that happens contains a bug.
Given the block style relies on wp_enqueue_block_style()
(not wp_enqueue_style()
), wp_style_add_data()
cannot be used with it, since the actual stylesheet won't be registered yet. In other words, the wp_style_add_data()
call there doesn't do anything.
This is a bit confusing, particularly since wp_enqueue_block_style()
lacks documentation that the $path
can actually be provided there as an argument, which only becomes clear when looking at the code of that function. This should be updated here as well.
Change History (14)
This ticket was mentioned in PR #5321 on WordPress/wordpress-develop by @flixos90.
13 months ago
#1
- Keywords has-patch added; needs-patch removed
#2
@
13 months ago
- Keywords has-testing-info added
To test the PR https://github.com/WordPress/wordpress-develop/pull/5321:
- Load the regular TT4 home page (which includes buttons).
- Without the PR, the
wp-content/themes/twentytwentyfour/assets/css/button-outline.css
file should be loaded (you should also see it in the source code). - With the PR, that file should no longer be loaded. Instead, there should be a
style
tag with IDtwentytwentyfour-button-style-outline-inline-css
in the source code.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
12 months ago
#6
follow-up:
↓ 9
@
12 months ago
Per this comment. The development GitHub repo for the TT4 theme is still open through RC1, so let's make sure these changes happen there first so they get properly synced in a future TT4 theme merge.
@hellofromTonya commented on PR #5321:
12 months ago
#7
@felixarntz I think this needs to be fixed upstream in the TT4 repo as development is happening there https://github.com/WordPress/twentytwentyfour.
@hellofromTonya commented on PR #5321:
12 months ago
#8
@felixarntz I think this needs to be fixed upstream in the TT4 repo as development is happening there https://github.com/WordPress/twentytwentyfour.
#9
in reply to:
↑ 6
@
12 months ago
Replying to joemcgill:
Per this comment, the development GitHub repo for the TT4 theme is still open through RC1, so let's make sure these changes happen there first so they get properly synced in a future TT4 theme merge.
I agree with @joemcgill. The report and fix needs to happen in the TT4 repo, rather than in Core itself.
@flixos90 can you open the issue in that repo and then port the PR over there please?
Then this ticket can be closed as reported upstream.
#10
@
12 months ago
I have implemented a Twenty Twenty-Four PR for this in https://github.com/WordPress/twentytwentyfour/pull/545.
I would prefer that we keep this ticket open and close it as part of the merge / update of TT4 to WP core trunk
with the fix included.
@flixos90 commented on PR #5321:
12 months ago
#12
Closing in favor of https://core.trac.wordpress.org/changeset/56754 and https://github.com/WordPress/twentytwentyfour/pull/545.
#13
@
12 months ago
The theme change was part of changeset 56764. Is this completed now?
'path'
argument ofwp_enqueue_block_style()
needs to be used instead ofwp_style_add_data()
. The latter only works when usingwp_register_style()
orwp_enqueue_style()
.get_theme_file_uri()
andget_theme_file_path()
, so that they can be optionally overwritten in a child theme.wp_enqueue_block_style()
function has its documentation improved to add the missing parameter.Trac ticket: https://core.trac.wordpress.org/ticket/59466