Product Elements: Sales badge should expose styling to theme.json

[sunyatasattva]

In some way, the Sales badge should be exposed for styling to inherits from theme.json. A suggestion would be to use a custom element directive. In this case, we should make sure to fall back to a default.

[danieldudzic]

After looking into this, I have determined, that currently, the Sale badge is stylable via theme.json with the following result:

theme.json code:

"styles": {
	"blocks": {
		"woocommerce/product-sale-badge": {
			"border": {
				"radius": "0"
			},
			"color": {
				"background": "purple",
				"text": "pink"
			},
			"typography": {
				"fontSize": "0.8rem"
			}
		},
	},
}

We should determine if the above is the desired behavior.

• Both Product Image Sale Badge and regular Sale Badge blocks are sharing the same class (.wc-block-components-product-sale-badge) and getting styled together.

• The Product Image Sale Badge is missing block customization controls (except for the alignment).

[sunyatasattva]

Thank you for your exploration @danieldudzic !

currently, the Sale badge is stylable via theme.json with the following result

That's great, but I think the spirit of @nerrad's comment on the kick-off thread (pdnLyh-30z-p2#comment-1957) was to add an Element directive so that such related elements could be styled globally and generically without styling the specific block. @nerrad, is that a correct interpretation of your words?

If that's so, what kind of custom element directive should be explore? Should we have a woocommerce/badge or something similar? I wonder that if we make it too specific, it could be almost the same as styling the block specifically like @danieldudzic showed, and if we make it too generic, we make it semantically incongruous and possibly having some side effects (as mentioned in my previous comment: pdnLyh-30z-p2#comment-1954).

The Product Image Sale Badge is missing block customization controls (except for the alignment).

I think we do not need to worry about this for now: likely, we are going to remove the “Product Image” block altogether, in favor of an even more atomic approach of combining the “Cover” block with the “Sale badge”.

[nerrad]

It looks like there are a number of different directions that we could go with this (despite my earlier suggestions - your assessment was correct Lucio):

• Inherit from existing element styles: For example button - so any styling for buttons in a theme will automatically be applied to the sales badge. This is bad because semantically the sales badge is not a button. The benefit is that most block themes will style the button element so the badge would, at least color wise, match all themes out of the box.
• Custom Element Directive: Typically the element properties of theme.json are used for controlling the style of HTML elements such as button, h1, caption, cite etc. I was thinking we could implement something custom for the sales badge but in thinking about it more, it doesn't really seem to be a good fit for this property as it's not a low-level html element.
• block specific styles in theme.json, this is what Danny has demonstrated here and a benefit is that with the Sales Badge being a block, it's a good method of explicitly styling the block (and a way for themes to override). We could utilize the theme.json filters so that we can set default styles for the block out of the box with any block theme (and block themes can still override).

I'm now actually leaning toward the third option here. The only difficulty of course is determining what the default styles for the block would be that would work reasonably well for a sales badge on most themes. I'm also wondering if we should create and register a default "palette" for Woo Elements that we use for things like the sales badge and then we can automatically register that palette for every Woo installation. Block themes that want to customize the starting point for Stores could potentially set the palette colors (cc @vivialice).

[sunyatasattva]

This is bad because semantically the sales badge is not a button. The benefit is that most block themes will style the button element so the badge would, at least color wise, match all themes out of the box.

Yes, and besides just the semantics of it, wrong styles (such as hover effects indicating interactability) could be applied this way.

it doesn't really seem to be a good fit for this property as it's not a low-level html element.

Agreed, that's what I was trying to articulate. I think elements do not need to be exactly low-level HTML element (for example, Core exposes a heading element which is a catch-all), but I think it would only make sense if we had a group of things which are low-level enough. For example, one could have, I think, perhaps a collapsible custom element which would apply to all sorts of accordion-type elements. Or something like this.

I'm now actually leaning toward the third option here. The only difficulty of course is determining what the default styles for the block would be that would work reasonably well for a sales badge on most themes. I'm also wondering if we should create and register a default "palette" for Woo Elements that we use for things like the sales badge and then we can automatically register that palette for every Woo installation.

I agree 100% with this and find the idea of providing a starting palette would benefit people immensely.

---

I know this is probably a stupid question, but I couldn't find specific information on the theme.json override hierarchy: but at not point of this hierarchy can plugins just provide a normal theme.json file without using those PHP filters, right? The hierarchy is something like:

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

right?

[nerrad]

at [no] point of this hierarchy can plugins just provide a normal theme.json file without using those PHP filters, right?

Correct. I participated in an early exploration where plugins just provided a theme.json file that would sit before themes in the hierarchy, however, the decision ended up going with filters instead. What needs to be remembered by the filter is that it executes using the relevant theme.json data. So if we want any values we provide via the filters to be overridden by themes, then we'll want to check for the properties first before supplying them.

[danieldudzic]

Hey @vivialice!

Do you have any suggestions regarding default styles we could add for the Sale badge (that would work reasonably well on most themes)?

@nerrad has also suggested exploring creating a default "palette" for Woo Elements that we use for things like the sales badge and then we can automatically register that palette for every Woo installation. Block themes that want to customize the starting point for Stores could potentially set the palette colors.

I'd be keen to hear your thoughts regarding this.

Thanks in advance!

[nerrad]

has also suggested exploring creating a default "palette" for Woo Elements that we use for things like the sales badge and then we can automatically register that palette for every Woo installation.

Let's hold off on that for now (based on further direction elsewhere).

[kmanijak]

Further direction

Let's use theme.json filters to add colors and set default styles for Sale Badges (both in Product Image and a standalone block).

We should use ref to align the styles between two badges:

Example (for button, but mechanics is the same):

Define ref:

"elements": {
	"button": {
		"color": {
			"background": {
				"ref": "styles.color.text"
			},
			"text": {
				"ref": "styles.color.background"
			}
		}
	}
}

Use ref:

"woocommerce/product-sale-badge": {
	"color": {
		"background": {
			"ref": "styles.elements.button.color.text"
		},
		"text": {
			"ref": "styles.elements.button.color.background"
		}
	}
}

Ref: pdnLyh-37o-p2#comment-2069

The question about default colors remains unless we decide to:

• preserve the current ones, but implement the mechanism to keep the badges consistent
• inherit the styles from buttons like in the example above (but that may be confusing to users as badge is not interactive element).

[imanish003]

Hi @kmanijak,
Thanks for continuing this discussion to unblock this issue 🙇

Please correct me if I am wrong. This is what I understood so far.
For now, we define sale badge styling in style.scss file (file link). But these styles doesn't show up in Global syles UI as shown in screenshot below:

Now as you mentioned, we can use theme.json filters instead of style.scss to define syles. Because styles set using filters will show up in Global Styles.
Also, themes can always override these using theme.json file, as explained by @danieldudzic in this comment.

• preserve the current ones, but implement the mechanism to keep the badges consistent
• inherit the styles from buttons like in the example above (but that may be confusing to users as badge is not interactive element).

In my opinion, we can go ahead & preserve the current styles. That way, we can complete the implementation & create a draft PR. Meanwhile, we can ask for feedback from Jarek & based on feedback; we can make the changes if required. Because AFAIU, we will be able to make the changes pretty quickly.

@kmanijak @tjcafferkey What do you think?

[kmanijak]

Because styles set using filters will show up in Global Styles.

I don't think that's possible at the moment, but that would be a desirable state in a long term.

Now as you mentioned, we can use theme.json filters instead of style.scss to define syles.

My understanding was that we want to style On Sale Badge block with some default colors (can be currents) and expose such styles through the ref so we can reuse it by Sale badge within Product Image block (and potentially others badges if any).

Does it make sense? But I must admit, your comment gave me second thoughts and I'm not 100% sure 🤔

In my opinion, we can go ahead & preserve the current styles. That way, we can complete the implementation & create a draft PR.

Totally agree!

[nerrad]

This:

"woocommerce/product-sale-badge": {
	"color": {
		"background": {
			"ref": "styles.elements.button.color.text"
		},
		"text": {
			"ref": "styles.elements.button.color.background"
		}
	}
}

(which would be added via usage of theme.json filters) would essentially have the sale badge styled similarly to what the theme is using for styling the button element's text and background styles (which I think would end up being the fallbacks that WP provides if the theme doesn't explicitly include those). There are a number of things you will need to consider here:

• Is there an active theme.json stylesheet? I'm not sure if the filters will get executed if there isn't. So you'd need fallback styles.
• If the theme.json included with theme doesn't define styles for the button elements, are there fallback values provided elsewhere? If not, you'll have to make sure we provide fallbacks.
• If the theme.json already includes these properties for the woocommerce/product-sale-badge block, we don't want to override them.
• You'll want to test with a few block themes and classic themes after any of the above changes to make sure there isn't any unexpected behaviour.

[imanish003]

Hi @nerrad, Thanks for listing the number of things that we will need to consider. That's very helpful 🙌🏻

I just wanted to confirm one thing.

would essentially have the sale badge styled similarly to what the theme is using for styling the button element's text and background styles

As already mentioned by you & @kmanijak, that may be confusing to users as badge is not interactive element.
I just wanted to confirm with you if that's okay. 🙂

[Aljullu]

Howdy! I wanted to re-open this conversation as we are facing a similar issue in #8939.

The thing is:

• we have the Product title block (currently used in All Products, Products and Single Product), which might have some custom styles
• we also display product titles in some other blocks (ie: Cart block), but they are not the Product Title block, instead, they are a React component

Then, the question is: do we want product titles in the Cart block to inherit the global styles applied to the Product title block?

I'm bringing this here because that issue reminded me of this one: we have the Sale Badge block and the sale badge component rendered inside the Product Image block. Currently, styles applied to the Sale Badge block are applied to the sale badge component. But IMO that's confusing, prone to break and has other issues (ie: the user can't override global styles in the component).

tl;dr IMO product title and sale badge components shouldn't inherit the styles of the Product Title and Sale Badge blocks. If we want their styles to be customizable, we should convert the components into inner blocks, but the current situation in the sale badge component is confusing and we should avoid doing the same with product titles. What do you think?

[nerrad]

I think you've raised a good point here Albert about the nuance that context has here. While from the perspective of the shopper to a store the fact something is rendered as a part of a block or a component is irrelevant, the context of that element does matter with regards to certain styles. There are some design attributes that might be still consistent such as those around typography (or color) whereas the variations are more likely to surface with size, or gaps (padding/margin).

So to add to the questions you've already asked, what is inherited from styles in the context (i.e. sizing and gaps defined by the block container components are in) vs what is specific to that element (individual blocks/components)?

On the surface, I agree though, generally, I don't think components should automatically inherit the styles of the individual blocks and my initial leaning is that components should be styled via the context they are in.

[imanish003]

Hi @Aljullu, Please bear with me as I clarify a few points.

Then, the question is: do we want product titles in the Cart block to inherit the global styles applied to the Product title block?

Are you referring to this import when mentioning product titles in the Cart block? Additionally, in the Products block, we utilize a variation of core/post-title, as evidenced here. To my knowledge, we use product titles in three ways:

1. A variation of core/post-title in the Products block
2. Importing the React component from atomic/blocks/product-elements/title/block for the Cart cross-sells product block
3. Registering woocommerce/product-title here

Is your question whether all three instances should inherit global styles? 🤔

we have the Sale Badge block and the sale badge component

Regarding the Sale Badge block and sale badge component, are you referring to the following?

1. Sale Badge block: Registered as woocommerce/product-sale-badge here
2. sale badge component: Imported from assets/js/atomic/blocks/product-elements/sale-badge/block.tsx here for the Product image

Please let me know if I've understood your points correctly. 🙂

[Aljullu]

Is your question whether all three instances should inherit global styles? 🤔

Yes, exactly. I think at least option 2 shouldn't inherit styles from the blocks.

Regarding the Sale Badge block and sale badge component, are you referring to the following?

Yup. Sorry for not being more precise! To clarify my point: I think what we are doing here is problematic:

woocommerce-blocks/assets/js/atomic/blocks/product-elements/image/block.tsx

Line 24 in 02524c5

import ProductSaleBadge from '../sale-badge/block';

We are importing the component of a block to be used in a completely different block. So it inherits the styles of Product Badge, but in the editor UI that's not rendered as the Product Badge block because it can't be selected and styles can be customized.

I'm not sure what's the best solution here, but I lean towards this being a proper inner block that can be selected and modified using the editor UI.

[imanish003]

Thanks, @Aljullu, for helping me understand your points! 🙌🏻

Yes, exactly. I think at least option 2 shouldn't inherit styles from the blocks.

I also agree with you. I don't believe any of those should share global styles because all three are distinct from one another.

• The 1st and 3rd are completely different blocks.
• When someone imports a React component instead of using the block directly, it doesn't make sense to share styles between the React component and the block. It essentially clarifies the intent that we just want to use the React component, not the block.

I'm not sure what's the best solution here, but I lean towards this being a proper inner block that can be selected and modified using the editor UI.

I agree with this as well. As far as I know, we plan to implement this in the future, where the Image block will be a combination of the Cover block and the Sale Badge block. I agree that we should be able to customize each of these individually. 🙂

[imanish003]

Based on the discussion on pull request #9068, I am going to close this issue. If it's needed, we can always reopen it.