-
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 Template: Output number of columns as classname for backwards compatibility #51358
Post Template: Output number of columns as classname for backwards compatibility #51358
Conversation
Size Change: -182 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
Flaky tests detected in f57e86f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5216790413
|
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.
Thanks for picking this up!
I think we should avoid adding an extra classname to any block that might have this type of layout in the future.
Theme authors have an increasing amount of APIs at their disposal that allow for customising block styles without having to target dynamic classnames in their stylesheets, so I guess ideally this classname wouldn't be needed at all 😄 but given that it existed previously in the Post Template block, we should provide back compat for that block specifically.
We should be able to add some logic in Post Template itself, maybe after this condition? So other blocks aren't affected.
Would the classname also be needed in the editor? We could add it in the block's edit function too.
Ah, gotcha! Yes, great feedback — I'll refactor this so that it's applied only to the Post Template block, and not all layouts 👍 (and place the code in the Post Template block instead) |
Done! I've moved the logic over to the post template block, and that's looking better 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.
Thanks for fixing this! It's working well and classnames only apply where needed ✅
Thanks again for the reviews and feedback! 🙇 @ndiego do you mind cherry-picking this one for 16.0, too? |
Thanks for the quick fix @andrewserong, I will include this in RC3 today. |
…mpatibility (#51358) * Grid Layout: Output number of columns as classname for backwards compatibility * Refactor to only apply to the post template block
…mpatibility (WordPress#51358) * Grid Layout: Output number of columns as classname for backwards compatibility * Refactor to only apply to the post template block
What?
Fixes #51346
Output
columns-x
classname for the post template block when the grid layout is in use and when acolumnCount
value is set (wherex
is the number of columns).Why?
As raised in #51346, this is so that themes that are currently targeting post template blocks with a certain number of columns can continue to do so.
This change is only for backwards compatibility / to ensure themes can continue to target these blocks, as the old width-based columns class styling rules (e.g. here) no longer apply for grid layouts.
How?
columns-${ columnCount }
classname.grid
because the columnCount number isn't automatically removed when switching between grid and flow layouts in the editor. Personally, I think this is desired behaviour, as it means that someone switching between values doesn't lose their old value, but it does mean we need to check forgrid
before outputting the classname (which I think is fine).Testing Instructions
columns-3
classname or whichever number of columns you have setcolumns-3
classname should no longer appear.columns-3
classname should no longer appear there either.Screenshots or screencast