-
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
Add block class name to the list block #56469
Conversation
This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit. Thank you! ❤️ |
Flaky tests detected in a9325d3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8004083055
|
Size Change: -391 B (-0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
Do you think the |
I think it is needed to be able to style the block using theme.json instead of unintentionally styling all |
It does get noisy, especially with nested lists. |
Yeah, it's quite noisy. We can probably figure out a way to make sure |
I like the idea: I don't think I can code it. |
I think we can run into styling issues when the css class is added both to the first ul/ol, as well as the ol/ul used to create nested lists, especially with spacing and border. |
For this we could update the selector generated for the (edited to make it a descendant selector) |
Might need a descendant selector |
Question? Seeing the amount of work it takes to add a class name to a block, wouldn't it make sense to use this oportunity when we are already chaging the selector that is used for the list item block so also just add the class |
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 this PR @carolinan ! This is testing well for me as regards global styles defined on the List and List Item blocks not bleeding over to other blocks that use list markup.
Tested by adding a gradient background to List, and custom font size, letter spacing and letter case to List Item.
On trunk, the styles also apply to the Navigation block:
On this branch, they are confined to List/List Item:
The only thing that still needs addressing is support for existing lists, as currently the classname only gets added to new blocks in the front end. That can be fixed by explicitly adding the classname in the front end, similarly to what was done for the Heading block.
It's a valid concern, but it's not specific to this branch; my testing showed the same happening on trunk. Arguably it's not a problem specific to this block either - any block nested in itself will have both instances styled - though nested lists are kind of a special case because it's unlikely anyone would want the same styles on parent and child list. I think it's worth discussing and exploring solutions to that in a dedicated issue or PR; it shouldn't block this PR. |
I am concerned that a solution would involve removing the class name from the nested block, which would then be a breaking change for anyone who has relied on it. |
I don't believe that would be a viable solution, especially if at any point we want to extend the List block to allow nesting all types of children, as discussed in #57272. It would then become closer to a Group in behaviour. |
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 updating! Code LGTM now and testing shows global styles applying on the front end to all previously created lists 🚀
This change does not need a standalone dev note. It can be in the misc. dev note or wherever is appropriate. Styling a list block using the Site Editor or theme.json also applied the style to other lists, partially or in full. |
@@ -21,8 +21,8 @@ | |||
}, | |||
"supports": { | |||
"className": false, | |||
"__experimentalSelector": ".wp-block-list > li", |
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.
Just noting this should have used the selectors API which landed at the end of 2022, rather than the deprecated __experimentalSelector
.
For example:
{
...
"selectors": {
"root": ".wp-block-list > li"
}
}
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'm in the process of dusting off an old PR to add color support to the List Item block. I can switch this over to use selectors
as part of that.
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.
thank you!
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 following up on this!
What?
Adds the
wp-block-list
class to the list blockFor #12420
Why?
The missing class makes it difficult to style the block separately from other lists.
How?
List:
"className": false,
from block.json.__experimentalSelector": "ol,ul",
from block.json so that the styles are applied to.wp-block-list
instead of plain ul and ol.List item:
.wp-block-list > li
Testing Instructions
Activate a block theme.
Open theme.json in your code editor and add a visible style to the list and list-item blocks, for example a background color.
If you are using Twenty Twenty-Four, the theme already has a styles for the list block, find them on line 515 in theme.json to avoid validation errors.
Example:
Create a new post or page.
Add an HTML block with a list inside.
Example:
Select preview from the HTML block toolbar and confirm that the list does not have a background color.
Add a list block with some inner list items.
wp-list-block
is present on the block.wp-list-block
is present on the block.Save, and view the front of the website.
Return to the editor and style the blocks via the settings in the sidebar; adjust color, typography and spacing. Confirm that the custom styles are applied correctly in the editor and on the front.
Bonus:
If you are using Twenty Twenty-Four, you can also confirm that the navigation block (which uses a list element) in the theme's header and footer does not have a pink background.
Screenshots or screencast