Skip to content
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 class to <ul> in List block. #12420

Closed
pagelab opened this issue Nov 29, 2018 · 40 comments
Closed

Add class to <ul> in List block. #12420

pagelab opened this issue Nov 29, 2018 · 40 comments
Assignees
Labels
[Block] List Affects the List Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Decision Needs a decision to be actionable or relevant [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@pagelab
Copy link
Contributor

pagelab commented Nov 29, 2018

Is your feature request related to a problem? Please describe.

The subject of adding classes to default blocks was discussed at #6639, and I know you can filter the List block to add a new class, but the wp-block-list class should be present by default.

Without a specific class, any style applied to the generic <ul> will inadvertently affect elements from third-party plugins or custom HTML added elsewhere, and we can't know beforehand if they will be affected.

One practical example happens when you try, for instance, to style the List block in a theme to add new bullets and you are also using the Elementor plugin.

Without a class, the generic .ul style will affect <li> elements used by Elementor in their control handles – unless you explicitly filter the class or resort to a convoluted solution as ul:not(.elementor-editor-element-settings):not(.wp-block-gallery):not(.wp-block-categories):not(.wp-block-latest-posts)... and so on.

This will surely happen with lots of other plugins that use the <ul> tag for tasks like this.

Image 1: Custom style added to <ul> tag.

image

Image 2: Custom style affecting a third-party plugin.

image

Describe the solution you'd like
Just add the wp-block-list class to the <ul> tag by default, in the List block.

Describe alternatives you've considered

Tested with 5.0-RC1-43946

@swissspidy
Copy link
Member

Note that using non-specific selectors like ul is a bad idea in general. I'm sure you could use something like .content-area ul in your theme.

@pagelab
Copy link
Contributor Author

pagelab commented Nov 29, 2018

@swissspidy That was just an example.

The problem would persist if you utilize .content-area ul – or other more specific selector.

The only way to solve the issue is to add a class to the <ul> tag itself.

@designsimply designsimply added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Block] List Affects the List Block labels Nov 29, 2018
@designsimply
Copy link
Member

I think that the reason for avoiding adding classnames for everything by default is that not every theme would need them by default and so filtering in the cases where it makes sense is the preferred method when possible. Adding the Needs Design Feedback label to get a check on that assumption and to pull in feedback from the core design team.

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Nov 29, 2018
@mtias mtias added this to the WordPress 5.0.x Follow Ups milestone Nov 30, 2018
@MichaelCampanella
Copy link

I'm in favor of unordered lists having a class attached, especially with other blocks like the gallery block using the ul tag.

@saltnpixels
Copy link

I am in favor of the ul block have some sort of class. Without it I cannot target those blocks easily.
I already made an issue once before and showed how the blocks could use a class or even a wrapper to stop from major issues in themes, especially ones where there is a sidebar or where pre and code are used without a wrapper...
:/ sadly it seems classes and div wrappers are not being added to the basic blocks.

@swissspidy
Copy link
Member

sadly it seems classes and div wrappers are not being added to the basic blocks.

Adding div wrappers around every block by default would be quite a backward compatibility break.

@saltnpixels
Copy link

saltnpixels commented Dec 3, 2018

Adding div wrappers around every block by default would be quite a backward compatibility break.

How so? Backwards to what? Gutenberg hasn't been officially released yet has it?
And I doubt it would break anything except floats, which have their own issues.

I am currently dealing with the issue that p tags made by Gutenberg should have a margin bottom, while in other areas it should not. There is no way to target the Gutenberg ones.

@MichaelCampanella
Copy link

I kept having styles from my unordered lists fall onto the gallery block so I resorted to styling my unordered lists by appending :not([class]) to them. Which feels very hacky.

@cecoates
Copy link

cecoates commented Apr 17, 2019

A user emailed in:

1961093-zen

reporting that the gallery block on their site seems to be inheriting the margins for the theme itself.

2HrDDw9

That sounds very similar to what you're describing here. Would giving the block its own class allow things like "resetting" ul/li margins?

See also:

Automattic/themes#735
Automattic/themes#447
Automattic/themes#596

@Fcog
Copy link

Fcog commented Jun 17, 2019

Please add a class to the ul and li tags. I need them to style the default gutenberg list style. I can't use the parent class because the custom blocks ul and li will be styled.

@onetrev
Copy link

onetrev commented Jul 8, 2019

I very much agree wp-block-list should be included by default for <ul> and <ol> items. But the good news is for any developer you can at least easily add the class using the code found here.

UPDATE!! The original answer in the link above technically works, but there are issues doing it this way, in particular you can then no longer add new classes later. But below in this issue is very much a good workaround.

@aduth
Copy link
Member

aduth commented Jul 10, 2019

Without a specific class, any style applied to the generic <ul> will inadvertently affect elements from third-party plugins or custom HTML added elsewhere, and we can't know beforehand if they will be affected.

[...]

Without a class, the generic .ul style will affect <li> elements used by Elementor in their control handles – unless you explicitly filter the class or resort to a convoluted solution as ul:not(.elementor-editor-element-settings):not(.wp-block-gallery):not(.wp-block-categories):not(.wp-block-latest-posts)... and so on.

If I understand correctly: This is not inadvertent, the default list block intentionally absorbs generic ul styling (either explicit or browser-default). A list block should expect to be styled the same as any other standard list on the page, and vice-versa. The block implementation shouldn't be optimized on the assumption of a CSS reset (lists stripped of their default styling), but instead that the onus is on a custom variation of a list to unset its own default styling, or choose a more appropriate HTML tag which does not carry the same styling implications.

@aduth aduth added Needs Decision Needs a decision to be actionable or relevant [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Jul 10, 2019
@pagelab
Copy link
Contributor Author

pagelab commented Jul 11, 2019

But the good news is for any developer you can at least easily add the class using the code found here.

Unfortunately, this is not a solution because it brings a bigger problem. If you add a class to the List block like this on the frontend (via enqueue_block_assets), you will also add many JS files from the editor.

editor-js-files-frontend

@pagelab
Copy link
Contributor Author

pagelab commented Jul 11, 2019

(...) that the onus is on a custom variation of a list to unset its own default styling,

This would only work if we could control the whole codebase, which is obviously not the case.

A custom style created by any theme (or plugin), can't know beforehand if a certain property is applied to the default tag. So the onus can't be on it.

(...) or choose a more appropriate HTML tag which does not carry the same styling implications.

If you have a list of things you should use the semantic tag on it. We should not resort to this kind of change just to avoid problems with styles.

Both <ul> and <ol> are particular cases, because they are commonly used for lots of things besides displaying a simple list of text items, so this block should be treated differently.

@strarsis
Copy link
Contributor

strarsis commented Dec 23, 2019

@MichaelCampanella: Also, using :not([class]) can cause new issues, as when a typographically used list gets additional classes assigned, e.g. for color or font-size, or a block style class that should just extend a small aspect of it on top of the base styles.

Maybe add typographic class to all blocks that are used in a typographical context, semantically, navigation and normal lists are the same, but not in their purpose.

@mtias
Copy link
Member

mtias commented Aug 30, 2020

Seems we should add standard block classes to a few of the elements we have left without.

@mtias mtias added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Aug 30, 2020
@alexandrebuffet
Copy link
Contributor

alexandrebuffet commented Dec 5, 2022

Block style do not seem to be the best solution. Adding a style to the list block (even with is_default/isDefault) does not generate the "is-style-default" parameter until the user has selected the style in the interface... See #38890.

Now that the decision has been made to add support for class name on "Heading" blocks. Can we consider the same support for "List" blocks until we develop a better solution to not saving className in the markup? @adamziel has correctly stated the problem and @andrewserong seems to validate the idea here: #42122.

@pagelab
Copy link
Contributor Author

pagelab commented Dec 6, 2022

Can we consider the same support for "List" blocks

@alexandrebuffet This is being discussed at #42269.

@carolinan
Copy link
Contributor

Can we resurrect this?
When the new per block custom CSS is used with the list block, it affects all lists.

@RavanH
Copy link

RavanH commented May 4, 2023

Please resurrect.

It also affects Navigation block submenus with a custom background color. See #50342

@mrwweb
Copy link

mrwweb commented May 22, 2023

Here's a PHP polyfill for doing this if it's useful to anyone:

add_filter( 'render_block', __NAMESPACE__ . '\add_class_to_list_block', 10, 2 );
/**
 * Polyfill wp-block-list class on list blocks
 *
 * Should not be necessary in future version of WP
 *
 * @see https://github.com/WordPress/gutenberg/issues/12420
 * @see https://github.com/WordPress/gutenberg/pull/42269
 */
function add_class_to_list_block( $block_content, $block ) {

	if ( 'core/list' === $block['blockName'] ) {
		$block_content = new WP_HTML_Tag_Processor( $block_content );
		$block_content->next_tag(); /* first tag should always be ul or ol */
		$block_content->add_class( 'wp-block-list' );
		$block_content->get_updated_html();
	}

	return $block_content;
}

This requires WordPress 6.2 which included the new WP_HTML_Tag_Processor class.

@strarsis
Copy link
Contributor

@mrwweb: The inclusion of WP_HTML_Tag_Processor is great, finally an external HTML5 parser library can be skipped.

@talldan
Copy link
Contributor

talldan commented Nov 27, 2023

I noticed there's also #50486, which might be a duplicate of this. It has quite a lot of detail on it.

@shaunjc
Copy link

shaunjc commented Dec 21, 2023

This still seems to be a major issue when developing block themes, as setting any of the default styling or setting custom CSS via Styles > Blocks > List will create CSS rules for ul, ol, affecting all manner of other blocks, such as latest posts, query loop and navigation blocks.

The polyfill above currently only works at putting a class name into the markup. If we want to use the same class name in our CSS, we either need to use the Styles > Additional CSS or find another way to populate the styles.

With that in mind, I registered a second filter that replaces the selector with my own custom classes, and it seems to work well for me so far.

add_filter( 'register_block_type_args', function( $args, $block_name ) {
	switch ( $block_name ) {
		case 'core/list':
			$args['supports']['__experimentalSelector'] = 'ol.wp-block-list,ul.wp-block-list';
			break;
	}
	return $args;
}, 10, 2 );

Of course, given that it has "experimental" in the name, I assume that this may change in future. Setting a default class name for this block in core would be the best way to move forward.

@tellthemachines
Copy link
Contributor

Given the way that global styles can now be defined on blocks, attaching classes only on the server side doesn't seem like a viable approach any longer. We need to be able to attach List block-specific styles to a classname that works in both the editor and the front end.

@onetrev
Copy link

onetrev commented Mar 5, 2024

What about adding className support as demonstrated in the Block Filters docs? It seems to work well on a new site, but it will result in "This block contains unexpected or invalid content" if added to a site with existing lists. So still not perfect.

@carolinan carolinan added the Needs Decision Needs a decision to be actionable or relevant label Mar 5, 2024
@carolinan
Copy link
Contributor

@priethor @mtias Hi
There are two suggestions for changing the approach on how to add the wp-block class name to the list block.

One is to use the class in both the editors and front:

Given the way that global styles can now be defined on blocks, attaching classes only on the server side doesn't seem like a viable approach any longer. We need to be able to attach List block-specific styles to a classname that works in both the editor and the front end.

The other is to also add a class on the inner list item block:

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 wp-block-list-item to the list item block?

Given the previous discussion and change requests from @mtias, how do you propose that we move forward with the issue?

@carolinan carolinan moved this to 🗣️ In Discussion / Needs Decision in WordPress 6.6 Editor Tasks Mar 18, 2024
@carolinan
Copy link
Contributor

How can we move this forward?

@youknowriad
Copy link
Contributor

Given the way that global styles can now be defined on blocks, attaching classes only on the server side doesn't seem like a viable approach any longer. We need to be able to attach List block-specific styles to a classname that works in both the editor and the front end.

I think @mtias's idea was to add it to both the frontend and the editor but to not serialize it in the saved markup. I think it's a given that the markup of the editor should match the frontend.

@carolinan
Copy link
Contributor

carolinan commented Apr 12, 2024

I am going to need more context because when I apply the PR the class is in all editors and the front...

@github-project-automation github-project-automation bot moved this from 🗣️ In Discussion / Needs Decision to ✅ Done in WordPress 6.6 Editor Tasks May 9, 2024
@carolinan carolinan removed this from the WordPress 5.x milestone May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] List Affects the List Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Decision Needs a decision to be actionable or relevant [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
No open projects
Status: Done
Development

No branches or pull requests