-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add Core CSS to styles compat hook for editor iframe #40846
Conversation
let isFilenameMatch = false; | ||
if ( styleSheet.href ) { | ||
const url = new URL( styleSheet.href ); | ||
if ( url.pathname === '/wp-admin/css/common.css' ) { |
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.
Is it okay to check for the full pathname? Is the check sufficient?
Do we need an allowlist of other Core styles that we want to carry over to the iframe?
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.
why do we need this stylesheet inside the iframe? AFAIK it's not something that targets frontend?
AFAIK alignleft
and alignright
are either supposed to be provided by the theme for classic themes or injected in layout styles for block themes.
Is the issue here about classic or block themes?
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.
why do we need this stylesheet inside the iframe? AFAIK it's not something that targets frontend?
AFAICT, it is being used on the frontend by Core's theme-compat/comments.php
fallback, which in turn is used by the Post Comments block (which is a thin wrapper around comments_template()
):
Is the issue here about classic or block themes?
Block themes -- I think that's the main target for the Post Comments block.
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.
AFAICT, it is being used on the frontend by Core's theme-compat/comments.php fallback, which in turn is used by the Post Comments block (which is a thin wrapper around comments_template()):
I guess that also means that the current fix is not enough since it only impacts the editor. I think the best solution here is to just add this to the "style.css" of the block like suggested by @Mamaduka that way it applies in both editor and frontend regardless of the theme used. Also, you might you want to target these links specifically and avoid a global rule that apply cross blocks and markup.
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 guess that also means that the current fix is not enough since it only impacts the editor. I think the best solution here is to just add this to the "style.css" of the block like suggested by @Mamaduka that way it applies in both editor and frontend regardless of the theme used.
Thanks! I had some reservations about this, but I guess it might be the most pragmatic approach then 😅
Also, you might you want to target these links specifically and avoid a global rule that apply cross blocks and markup.
Can do. Still feels like there might be some need for a more far-reaching solution that makes some of these classes available that are relied on by Core functions that are used by some dynamic blocks on the frontend, but maybe we don't have to solve that here and now... 😬
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.
Still feels like there might be some need for a more far-reaching solution that makes some of these classes available that are relied on by Core functions that are used by some dynamic blocks on the frontend, but maybe we don't have to solve that here and now...
No, I actually think the opposite. The alignment styles are now generated by layout in block themes for all use-cases. The issue here is an outlier and that doesn't deserve a global fix. In other words blocks shouldn't be assuming that they can use "alignleft", "alignright" and expect these to be defined in all situations. These define on their container for block themes.
So actually, the block could have use custom classes and just target these instead but since it's an old template from Core. We can just add specific styles to the block using the old template.
Size Change: +45 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
@ockham, I think you can list the gutenberg/lib/compat/wordpress-6.0/client-assets.php Lines 45 to 50 in acf2957
Any reason we're not adding those classes to the Post Comments |
Thanks @Mamaduka!
I tried applying the following patch to both GB, and my local Core repo (that GB is using): diff --git a/lib/compat/wordpress-6.0/client-assets.php b/lib/compat/wordpress-6.0/client-assets.php
index 77f0593dc8..ed10b71b5d 100644
--- a/lib/compat/wordpress-6.0/client-assets.php
+++ b/lib/compat/wordpress-6.0/client-assets.php
@@ -47,6 +47,7 @@ function gutenberg_resolve_assets() {
'wp-block-library',
'wp-block-library-theme',
'wp-edit-blocks',
+ 'common-css',
);
if ( 'widgets.php' === $pagenow || 'customize.php' === $pagenow ) { Neither worked. Now there's a chance that GB's compat layer isn't being picked up, and that I didn't properly rebuild my local Core; but is it possible that these (PHP) changes don't impact which of the enqueued styles are carried over to the iframe? 😅 🤔
It seemed like these styles might be something that should be available on a more "global" level (since they already are present in the Post Editor; and they're relied on by Core's (Also see my reply to Riad: #40846 (comment)) |
@ockham, I think you need to remove the
In that case, do you think it should be a part of |
Ah yes, that did the trick! Thank you 😄
Maybe? 😅 TBH, I feel like I'm a bit out of my wheelhouse when it comes to how these styles are (or should be) organized 😅 Anyway, I'll close this PR and will file an alternative on Monday -- I'll probably just try Riad's suggestion after all. |
What?
Follow-up to #40842. Fixes #40827.
Make sure the
common.css
stylesheet is carried over to the Site Editor's iframe.Why?
See #40827. tl;dr:
Users that still have the now-deprecated Post Comments block in one of their templates will find that the "Older Comments" and "Newer Comments" links aren't left- and right-aligned like they should be, but stacked on top of each other.
Comparing to the Post Editor, it seems like the "Older Comments" and "Newer Comments" links have the
alignleft
andalignright
classes assigned, respectively:These classes are coming from Core's
common.css
: https://github.com/WordPress/wordpress-develop/blob/b27069117eb22c39d5230ceb39439f2dab82dc08/src/wp-admin/css/common.css#L92-L99I checked the Site Editor's page source, and the
common.css
is enqueued there. However, it is not carried over to the Site Editor's iframe.How?
By checking if the stylesheet is coming from Core's
common.css
, and if so, adding its styles to the iframe.Testing Instructions
Verify that #40827 is fixed.
Screenshots or screencast
Looks like we might have to update some Post Comments block styling accordingly 😬