-
Notifications
You must be signed in to change notification settings - Fork 219
Product SKU: Adds support for color, typography, and spacing #8913
Conversation
This commit introduces a new file `supports.ts` for the Product SKU block in the WooCommerce Blocks plugin, which adds color, typography, and spacing support. The following configuration has been added: 1. In `assets/js/atomic/blocks/product-elements/sku/supports.ts`, a new `supports` object is created, which extends the shared configuration and adds support for the following properties: - `text` and `background` color - `fontSize`, `lineHeight`, `__experimentalFontWeight`, `__experimentalFontFamily`, `__experimentalFontStyle`, `__experimentalTextTransform`, `__experimentalTextDecoration`, and `__experimentalLetterSpacing` for typography - `margin` and `padding` for spacing
…ll Products block This commit extends the support for color, typography, and spacing to the Product SKU block when used within the All Products block. The following changes have been made: 1. In `assets/js/atomic/blocks/product-elements/sku/block.tsx`, the `useColorProps`, `useSpacingProps`, and `useTypographyProps` hooks are imported and used to add the appropriate className and style properties to the rendered SKU element. 2. In `assets/js/atomic/blocks/product-elements/sku/edit.tsx`, the `style` property is separated from `useBlockProps()` and used directly on the `div` element for the SKU block.
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: +787 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
This commit adds the ".wp-block-woocommerce-product-sku" class to the Product SKU Gutenberg block for server-side rendering purposes. The inclusion of this class ensures consistent styling between the editor and the front-end view of the block, providing a seamless experience for users. The class name is generated based on the Gutenberg block prefix "wp-block", the namespace "woocommerce", and the block name "product-sku", creating a unique and identifiable class for the block. This class enables developers to target the block specifically in both the editor and front-end styles, maintaining the integrity of the block's appearance and functionality across different environments. By adding this class to the server-side rendered version of the block, we ensure a unified and coherent styling experience throughout the block's usage.
@Aljullu, I am also adding you as reviewer because this PR adds I also considered creating a recursive function that checks block parents and their ancestors. Something like this:
But I dropped this idea because I thought this would have performance impact. Please let me know your thoughts about it 🙂 |
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.
It's testing perfectly and code looks good to me. Nice improvement to the Product SKU block. 👏
Not sure if it's the expected behavior, but font-weight is only applied to the SKU:
prefix, but not to the value:
@Aljullu, I am also adding you as reviewer because this PR adds
isDescendantOfAllProducts
flag for all products elements.
I see we have similar flag for other parent blocks, so it makes sense to me. 🙂
* External dependencies | ||
*/ | ||
import { isFeaturePluginBuild } from '@woocommerce/block-settings'; | ||
import { __experimentalGetSpacingClassesAndStyles } from '@wordpress/block-editor'; |
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.
Heads-up that this line introduced an ESLint warning.
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.
ESLint warning is present wherever we use __experimentalGetSpacingClassesAndStyles, therefore I thought it was acceptable. However, I will add the following lines to ignore the warning going forward 🙂
// @ts-expect-error We check if this exists before using it.
// eslint-disable-next-line @wordpress/no-unsafe-wp-apis
__experimentalTextDecoration: true, | ||
__experimentalLetterSpacing: true, | ||
}, | ||
__experimentalSelector: '.wc-block-components-product-sku', |
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.
Is __experimentalSelector
required? I tried removing it and everything seemed to keep working as expected. 🤔
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.
Good catch! I think we can remove __experimentalSelector
flag.
color: { | ||
text: true, | ||
background: true, | ||
}, | ||
typography: { | ||
fontSize: true, | ||
lineHeight: true, |
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.
If we can remove __experimentalSelector
, then we could make these styles available in WC core as well, so only the experimental ones would be gated to the feature plugin. 🙂
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.
That makes sense. I will make the necessary changes 🤝
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 taking care of this! Things I spotted while testing:
- Little caveat (edge case I think) is the text is wrapped in the Editor while it may escape the container in the frontend. Below is the example in All Products, but it behaves the same in Products block.
Editor | Frontend |
---|---|
- Letter case is not reflected in the Editor but it works ok in Frontend while used in GLobal Styles. Below you can see it's set to lowercase in Editor, but it's kept uppercase in there.
Editor | Frontend |
---|---|
Everything else looks good! I left some minor comments otherwise.
Also, the above may require the changes in Gutenberg, and in such case, I don't think they should block this PR from being merged 🙌
This commit refactors the Product SKU block supports to enable color, typography, and spacing features for all builds, while keeping experimental features specific to the feature plugin build. It also adds necessary lint and TypeScript error handling for the experimental spacing API.
Actually, we have value text in a strong tag:
I thought we wanna keep
Excellent, this will simplify the process of keeping product elements compatible with the All products block! 🙌🏻 |
This commit adds the overflow-wrap property to the Product SKU block styles to handle long SKU values gracefully. It also updates the comment for the experimental spacing API to better reflect the usage check.
This commit refactors the Product SKU block classname and style handling. It combines classnames for better readability and removes the hardcoded textTransform style in favor of default styles. This also fix the issue where textTransform style was not working as expected.
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.
Looks good on my end, so approving. Leaving @kmanijak to do the final review on the items he brought up to the discussion.
I thought we wanna keep
strong
tag. I don't have a strong opinion about it, if you want, I can remove the strong tag. 🙂
I don't have a specific preference either. I would say let's keep it as-is now. At some point it might be good to get design feedback on it, though.
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'll wait for @kmanijak final review and address any concerns he may have before merging this PR 🙌🏻
Excellent work! I double checked the previously raised glitches and smoke tested - all works as expected. Great work @imanish003! 💪
Hey @imanish003 when editing the case of the SKU block, it also affects the SKU itself. Is this intended? Would we prefer to display the SKU exactly as the merchant entered in the product page? |
This is intended as per the current implementation. I understand your concern about displaying the SKU as the merchant entered it. Perhaps we could get input from @shaunandrews on this matter? 🙂 |
This Pull Request enhances the styling capabilities of the Product SKU block by introducing additional support for color, typography & spacing. This improvement provides users with more flexibility when customizing the appearance of the Product SKU block.
Here is what Product SKU block supports now:
Color:
Typography:
Spacing:
Part of #8691 & #7954
User Facing Testing
Test using blocks sidebar
Test using Global Styles
Screen.Recording.2023-03-30.at.2.17.57.PM.mov
WooCommerce Visibility
Changelog