-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Post Content: Fix display of block support styles #66003
Post Content: Fix display of block support styles #66003
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +103 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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 really like the approach here, I think it's a good pragmatic path forward for 6.7, especially since the Post Content block is used so prominently there. I tested this very quickly in TT4, too, with element styles and it displays block content consistently for users who can edit the post that's being previewed:
Overall, especially since most of the time users who are interacting with the post content block will have access to edit the posts that they're previewing, I think this fix should cover most cases.
Let's see what others think!
This looks great, thank you for working on this, and as shown below, it solves the issue for me: Note though that I'm seeing here an old issue pop up again, namely that full-wide content becomes content-wide in the TT5 Blog Home template. This was supposedly fixed here. So I dove in and found that, unfortunately that appears to be working in trunk Gutenberg, meaning wide and fullwide may have broken in this PR: So, section styles are working, but wide/fullwide broke. Is the latter possible to fix in this one? |
Thanks for the testing and feedback @jasmussen and @carolinan 👍 I suspect this might be related to how the layout classes are applied to the wrapper around the read-only post content while the inner content has a specific layout. I'm out of time today but will see what can be done here first thing tomorrow. |
Just reiterating @jasmussen and @carolinan's feedback above.
|
👋 - I pushed something and would be great if you could retest. Thank you! |
This seems to be working! site-editor-section-styles-testing.mp4 |
Based on some quick testing, I think this could work as a temporary fix. It would be better to get the styles available to the assets when using I'm about to step out for an appointment so will come back to this in a few hours to follow up further. |
LGTM - agree that this is a huge improvement and a good fix to get in now. Before2024-10-11.10.20.43.mp4After2024-10-11.10.21.13.mp4 |
Thanks for pushing the parent layout fix @ntsekouras 🙇 It makes sense and tests well for me also. I appreciate everyone jumping in to test and review this one. I'll hold off for a little longer before merging to give @peterwilsoncc a chance to take a closer look.
Agreed. The thinking was however that approach might be a much larger change with broader impact. At this stage of the release cycle it felt like a bridge too far for 6.7. |
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.
These changes look good to me from a functionality perspective.
I used a very simple filter to confirm the raw data is only used for users with edit permissions and it behaved as expected.
add_filter( 'map_meta_cap', function( $caps, $cap, $user_id, $args ) {
if ( 'edit_post' === $cap || 'delete_post' === $cap ) {
$caps = array( 'do_not_allow' );
}
return $caps;
}, 10, 4 );
I also confirmed that the additional use of userCanEdit
doesn't result in additional HTTP requests and it does not.
As a few of you have approved the code I think this is good to merge.
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.
LGTM, too, thanks for honing in on a pragmatic fix for 6.7 @aaronrobertshaw! 🙇
Nice teamwork on this one, thanks everyone 👍 I'll get this merged then! |
Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: peterwilsoncc <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: ndiego <[email protected]> Co-authored-by: colorful-tones <[email protected]>
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 0ca8d56 |
Kudos, thank you all for this work and the reviews! This is impactful. |
Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: peterwilsoncc <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: ndiego <[email protected]> Co-authored-by: colorful-tones <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: peterwilsoncc <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: carolinan <[email protected]> Co-authored-by: ndiego <[email protected]> Co-authored-by: colorful-tones <[email protected]>
Fixes:
Revisits approach from:
What?
Renders actual blocks for Post Content previews when the user can edit the post being previewed. This allows the editors block support hooks to do their job and render additional styles as needed.
Props to @andrewserong for the approach.
Why?
Currently, the Post Content block renders read-only content via a REST API call. The server side rendered content returned via that request lacks additional stylesheets normally enqueued by block supports. This results in element, layout, and block style variation styles all missing when viewing Query loops containing Post Content previews.
How?
#35863 was originally blocked due to concern around potentially exposing private block data if the raw block data was rendered. This PR takes a ever-so-slightly different approach in that it will only render the raw blocks instead of server side rendered HTML when the user has access to edit the post in question.
My understanding is that plugins providing visibility control over blocks within a page, still allow any user with rights to edit a particular page to edit those visibility or access-controller blocks e.g. change them, alter the user capabilities required for their display etc. In other words, the access control there is primarily for the frontend. (happy to be corrected here 🙂)
If a post is password protected and a user doesn't have edit permissions for that post, the original warning is still displayed in place of the post's content.
Testing Instructions
Using TT5:
Example block content to test with if it helps
Screenshots or screencast