use product details Key to generate classname. closes #6512

[helgatheviking]

For product details, split CSS classname and display name into separate variables.

Fixes #6512

Accessibility

• [ ] I've tested using only a keyboard (no mouse)
• [ ] I've tested using a screen reader
• [ ] All animations respect prefers-reduced-motion
• [ ] All text has at least a 4.5 color contrast with its background

Other Checks

• [ ] This PR adds/removes a feature flag & I've updated this doc .
• [ ] This PR adds/removes an experimental interfaces and I've updated this doc
• [ ] I tagged two reviewers because this PR makes queries to the database or I think it might have some security impact.

Screenshots

Before	After

Testing

Automated Tests

• [ ] Changes in this PR are covered by Automated Tests.
  • [ ] Unit tests
  • [ ] E2E tests

User Facing Testing

• [ ] Do not include in the Testing Notes

WooCommerce Visibility

• [ ] WooCommerce Core
• [ ] Feature plugin
• [ ] Experimental

Performance Impact

Changelog

Add suggested changelog entry here.

[alexflorisca]

Hey @helgatheviking, thanks for opening this PR, great to see people contribute.

Unfortunately it's not so simple, as we need to support both name and key, depending on where the data comes from. This was a change we made a while back to support this. If we are displaying product variations, we use key, however any item data that comes from other sources, (for example the Product Add-ons plugin) will have a name property instead of key. They are both translatable strings anyway so this doesn't really solve the problem.

What we could do instead is maybe use the array index within the array map instead of the name. Something like this:

{ details.map( ( detail, index ) => {
  const className = `wc-block-components-product-details__${ index }`
  ...
})

It would be good to understand what your are hooking onto the wc-block-components-product-details__${ name } class for? Do you need to target the specific attribute in that list? Would the above solution work for you?

[helgatheviking]

I am work-shopping how to best display the selections/contents of a box for my Mix and Match Products plugin. If I want to apply any special style rules to the "Selections" meta key/values

then I have to also add inline styles in the header to account for the translated classes.. because I can't dynamically change class names in my CSS file.

This is a bit nonsensical, but it's just as an example.. this is the kind of workaround needed to style a specific meta pair's name :

$meta_suffix = _wp_to_kebab_case( __( 'Selections', 'woocommerce-mix-and-match-products' ) );

if ( 'selections' !== $meta_suffix ) {
	$inline_css   = array();
	$inline_css[] = 'table.wc-block-cart-items .wc-block-cart-items__row.is-mnm-container .wc-block-components-product-details__' . $meta_suffix . ' .wc-block-components-product-details__name { color:red; }';
}

Though I am seeing that the PHP templates do this too... and am starting to explore a different path that may be more helpful... so this probably isn't in high demand :)

Still, I think it makes sense to be able to have a class that is static, (Isn't the index a number? As that could change depending on the product's position in the cart, it would also not be reliable). I think this will make it easier to style long-term and nobody has to think about using wp_add_inline_style as a workaround.

[nielslange]

@helgatheviking In https://github.com/woocommerce/woocommerce-blocks/issues/6512, you mentioned the following HTML code:

<ul class="wc-block-components-product-details">
	<li class="wc-block-components-product-details__includes">
		<span class="wc-block-components-product-details__name">Includes:</span>
		<span class="wc-block-components-product-details__value">Chocolate Chip Cookie × 1</span>
	</li>
	<li class="wc-block-components-product-details__includes">
		<span class="wc-block-components-product-details__name">Includes:</span>
		<span class="wc-block-components-product-details__value">Macadamia Nut Cookie × 1</span>
	</li>
	<li class="wc-block-components-product-details__includes">
		<span class="wc-block-components-product-details__name">Includes:</span>
		<span class="wc-block-components-product-details__value">Oatmeal Raisin Cookie × 1</span>
	</li>
</ul>

On a fresh WordPress installation, I see the following code:

<ul class="wc-block-components-product-details">
	<li class="wc-block-components-product-details__color">
		<span class="wc-block-components-product-details__name">Color:</span>
		<span class="wc-block-components-product-details__value">Blue</span>
	</li>
	<li class="wc-block-components-product-details__logo">
		<span class="wc-block-components-product-details__name">Logo:</span>
		<span class="wc-block-components-product-details__value">Yes</span>
	</li>
</ul>

When applying this PR, the attribute labels get lost:

<ul class="wc-block-components-product-details">
	<li class="wc-block-components-product-details__color">
		<span class="wc-block-components-product-details__value">Blue</span>
	</li>
	<li class="wc-block-components-product-details__logo">
		<span class="wc-block-components-product-details__value">Yes</span>
	</li>
</ul>

Before:

After:

Am I missing something here? It appears that the behaviour of the PR is not in sync with its title.

[alexflorisca]

@nielslange see my comment above. The product variations use the key prop which this PR removes. Which is why you're not seeing the attribute labels. I can't add a static class as these are all generated dynamically, and I can't see anything sensible to use other than name or key props so not really sure how to solve this one. We could maybe pair on it and brainstorm sometime, that's just from memory when looking at this before.

[helgatheviking]

Well I am still very new to React so it's possible I messed something up. :) And now I'm a little underwater with other projects. The intent was not to remove the attribute label from the markup. I'm open to anything that will provide a static/predictable class name.

[nielslange]

Well I am still very new to React so it's possible I messed something up. :) And now I'm a little underwater with other projects. The intent was not to remove the attribute label from the markup. I'm open to anything that will provide a static/predictable class name.

Thanks for clarifying this, @helgatheviking! @alexflorisca and I will look into options, and I'll reach out to you if we need further input. 😀

[alexflorisca]

@helgatheviking do you need to target individual items or would it be enough to have a static class for all items, e.g: <li className="wc-block-components-product-details__item">? If the latter is ok, then this PR will fix that

[helgatheviking]

I can't really test at the moment, but I think that might do the trick! I am trying to target the .wc-block-components-product-details__name class of a specific item, I have to get the class as a translatable string and then print that out as inline CSS so that it changes according to the language.

$meta_suffix = _wp_to_kebab_case( esc_html__( 'Selections', 'woocommerce-mix-and-match-products' ) );

$inline_css   = array();

if ( 'selections' !== $meta_suffix ) {
	$inline_css[] = 'table.wc-block-cart-items .wc-block-cart-items__row.is-mnm-container .wc-block-components-product-details__' . $meta_suffix . ' .wc-block-components-product-details__name { display:none; }';
}

wp_add_inline_style( 'wc-mnm-checkout-blocks', implode( ' ' , $inline_css ) );
``

[alexflorisca]

I am trying to target the .wc-block-components-product-details__name class of a specific item

Ok so to select a specific item in the list, you'd have to do that with the :nth-of-type css selector as each row of items will have the static wc-block-components-product-details__item instead of a class with the name of the attributes displayed in the row. I hope that works for you, let me know when you try it out. This is a more standards friendly way of doing things anyway, as it'snot great practice to tie css names to the specific content inside a div.

[helgatheviking]

Hi @alexflorisca Thinking about this a bit more... I do still need to target a specific .wc-block-components-product-details__$key but that the $key needs to always be the same and not translatable.

:nth-of-type is not sufficient as what if you have detail items that are

<li class="wc-block-components-product-details__item>SOMETHING ELSE</li>

but on different products/occasions FIRSTSOMETHING and SOMETHING you have

<li class="wc-block-components-product-details__item>SOMETHING</li>

You could still not reliably target the <li> for SOMETHING by its index position.

[alexflorisca]

Hi @helgatheviking, I've just checked the code again, and it doesn't look like the name or key props are actually translated before they are used within the class. So they should always be the same. Can you show me an example of where they are being translated?

[helgatheviking]

@alexflorisca I was fairly certain they were... but it's been a minute since I looked. Since your comment, this commit https://github.com/woocommerce/woocommerce-blocks/commit/c410644e9441897ef512f5fa464d972bf90cbd64 has removed any dynamic class name for the meta details... which is also problematic for me. I need to be able to CSS hide a specific meta data. And now there's no way to target it at all by class because they all have the same classes.

I need to be able to target specific detail list items but now there's no way to distinguish them

[alexflorisca]

Hey @helgatheviking I've commented on this PR. We can revert that change if it's useful to you but I would really like to know where and how you saw the classname strings translated as we couldn't replicate that.

[alexflorisca]

Closing this as it was fixed in https://github.com/woocommerce/woocommerce-blocks/pull/7328