Product Elements: Margin need to be reviewed

[danieldudzic]

Similar issue for the All Products block: #2565

We should review Product Elements margins. Ideally, we should transition from using margins in CSS to utilizing global styles (to allow for better customizability). We also need to make sure not to break any of the existing (other) blocks relying on some of these margins.

Example of current CSS margin for the Product Price:

[Aljullu]

@danieldudzic do you think we can close this issue now? Or is there something left?

[danieldudzic]

I'm still waiting on feedback from @vivialice regarding p1674213756256509-slack-C02FL3X7KR6

[imanish003]

I conducted an evaluation of the Global style support for the following blocks, with a particular focus on margin and padding support.

• Product Image
• Product Title
• Product Price
• Product SKU
• Product Summary
• Product Rating
• Product Stock Indicator
• On sale badge
• Add to Cart Button

My findings

During my evaluation, I discovered that some blocks lack support for global margin or padding, or both. Specifically, the following blocks do not support either:

• Product Stock Indicator
• Product Summary
• Product SKU

I will create a new PR that adds support for padding and margin for all the product elements used in the Products block.
Additionally, I have discovered that the global style support for the product title is not functioning as intended. Therefore, I will investigate the issue and resolve it.

[imanish003]

As we have removed margins from CSS, we have to pass margin in attributes, as you can see here:

woocommerce-blocks/assets/js/blocks/product-query/constants.ts

Lines 87 to 92 in 0c69a83

	spacing: {
	margin: {
	bottom: '0.75rem',
	top: '0',
	},
	},

This causes one problem. As you can see in demo video, our margin bottom will be set to 0.75rem by default in sidebar settings. i.e.

• Passing an attribute = user changing the setting from sidebar

Screen.Recording.2023-04-06.at.3.20.57.PM.mov

Because of this, even if user changes block margins from Global settings, it won't have any effect. In other words, these blocks bottom margin can't be set using Global styles.

Solution 1: Defining margins in style.scss of product elements

This approach would involve defining the styles directly in the style.scss file, which would apply to the blocks by default.

Cons:

• Limited flexibility: As product elements will be reused in many other blocks, in which we may not want default margins.

Pros:

• Straightforward implementation: Adding styles directly to the style.scss file is a simple and direct way to define default styles for Gutenberg blocks.

Solution 2: Using nested selectors

Here is what we can do:

1. First, we can add any className, for example, products-block-post-template to core/post-template block, which contains all the product elements
2. Then we can add following CSS to style.scss

.products-block-post-template .wp-block-post > * {
	margin-bottom: $gap-small;
	margin-top: 0;
	background-color: #f0f0f1;
}

Here is how the result will look like:

Will the global styling hierarchy continue to function effectively following this?

AFAIK global styling hierarchy looks like this:

Editor Styles > Global Styles > Child Theme > Parent Theme > Core

I tested it and found out that global styling hierarchy ****works fine in Editor, but doesn’t work on Frontend.

Working in Editor but not frontend. Why?

Let’s take Product Price as an example. Global Styles, Child Theme, and Parent Theme use the selector .editor-styles-wrapper .wp-block-woocommerce-product-price in the editor, but on the frontend, the selector is only .wp-block-woocommerce-product-price. This means that the selector's specificity is not high enough on the frontend, causing the styles to not work properly.

How can we fix it?

To fix it, we will need to increase the specificity of selector on Frontend. One way to do it is by adding __experimentalSelector flag for each product element. For example:

__experimentalSelector: '.wp-block-woocommerce-product-price.wp-block-woocommerce-product-price-2'

Here, we have enhanced the selector's specificity by adding two classes to it.

Let’s discuss pros and cons of using this approach:

Pros:

• Targeted styling: This way, we can only target product elements only in Products block, providing more control over the appearance of the blocks.
• Flexibility: Product elements remain flexible & can be used in various use cases.
• Compatibility with global styling hierarchy: The nested selector approach can work well with the global styling hierarchy, ensuring that styles from various sources (editor styles, global styles, child themes, parent themes) are applied correctly.

Cons:

• We must add __experimentalSelector for each product element to make this approach work.
• AFAIK There is no way for us to add __experimentalSelector for product elements that are variations.
• If we use __experimentalSelector then I think we will need to add everything in supports under isFeaturePluginBuild flag because now supports depends upon an experimental flag.

Feedback needed

I would appreciate your feedback on the following:

• Are there any other solutions that I should consider that I may not be aware of?
• Which of the approaches above do you think would be best to implement?

CC: @tjcafferkey @nerrad @Aljullu

[Aljullu]

Thanks for this research, @imanish003. My feeling is that none of the approaches is perfect because we want those elements to have margin by default without the user having set any margin in advance, so there is always going to be some confusion: "why does this block have margin if I hadn't defined it?".

Having said that, I consider that passing the margins as an attribute is the best approach we have right now. I understand that means that margin global styles are not applied by default, but I think that's expected in cases where the user adds a block that contains inner blocks: some styling of those inner blocks might come with a pre-defined state to make it look good in the context it was added.

Then, if I as a user add more instances of that inner block (let's imagine, Product Price), yes, I would expect it not to have margin by default and inherit the global styles. But the Product Price which is added with the default pattern, I would expect it to have opinionated margins to make it look good in the default pattern.

IMO that also applies to text-align, font-size and all other styles we are passing via attributes. I think it makes sense that the default Products block pattern comes with opinionated styles for its inner blocks. That means global styles aren't applied if the user doesn't reset the default styles, but I think that's fine.

So the tl;dr is that I don't think we should fix anything here and I would keep it as-is.

However, if that's not an option, from the ones you proposed I lean more towards solution 2, as that won't impact those blocks when added in other contexts (as the Single Product template).

.products-block-post-template .wp-block-post > * {

Related to the specificity problem you mention below in Working in Editor but not frontend. Why?. I wonder if using :where() in the selector above would help decreasing the specificity to avoid the CSS selector overriding the styling set via the UI. We are using :where() for this purpose in some other parts of the codebase, ie:

woocommerce-blocks/assets/js/blocks/mini-cart/frontend.ts

Line 180 in c5da4a0

`:where(.wp-block-woocommerce-mini-cart-contents) {

[imanish003]

Thanks, @Aljullu, for your feedback. It's very helpful. I'd like to share some thoughts in response to your comments 🙌🏻

My feeling is that none of the approaches is perfect because we want those elements to have margin by default without the user having set any margin in advance, so there is always going to be some confusion: "why does this block have margin if I hadn't defined it?".

I simply want themes to be able to customize the Products block using a theme.json file. Since the Products block will be used in various scenarios, I don't think we should restrict any styles. The approaches I shared enable theme developers to style product elements, and the Gutenberg style hierarchy will also function as expected. I believe this would be ideal for the Products block 🙂

To be honest, I'll consider passing styles as an attribute only as a last resort, as it disrupts the styling hierarchy. However, it might just be me who thinks this way. 🤷🏻‍♂️

We are using :where() for this purpose in some other parts of the codebase, ie:

Wow, that sounds like a great idea! I'm going to try :where() selector. Thank you so much! 🙌🏻