Add Single Product Details block #8225
Conversation
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +712 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
| { | ||
| "name": "woocommerce/single-product-details", | ||
| "version": "1.0.0", | ||
| "title": "Single Product Details", |
There was a problem hiding this comment.
Should we name it Product Details or Single Product Details? 🤔 cc @vivialice
There was a problem hiding this comment.
Just name it Product Details for now.
| "attributes": {}, | ||
| "textdomain": "woo-gutenberg-products-block", | ||
| "apiVersion": 2, | ||
| "$schema": "https://schemas.wp.org/trunk/block.json" |
There was a problem hiding this comment.
Miss the declaration for the icon. Any idea about what icon should we pick? @vivialice
| @@ -0,0 +1,46 @@ | |||
| .wp-block-woocommerce-single-product-details { | |||
There was a problem hiding this comment.
If we resolve #8314, is this CSS still necessary?
There was a problem hiding this comment.
I hope we don't need it anymore after solving that issue; however, if we need to change some things in the UX I think we still are going to need this
| import metadata from './block.json'; | ||
| import edit from './edit'; | ||
|
|
||
| registerBlockType( metadata, { |
There was a problem hiding this comment.
We should make the block available only on the Single Product Template. I created a util function to do this.
There was a problem hiding this comment.
I think that function is not merged into trunk yet, right?
There was a problem hiding this comment.
Hi there @thealexandrelara! Good to see this block shaping up here, well done! It works like a charm, but there's one caveat, the star ratings are still duplicated:
Did you have the chance to look into this? I'm glad to jump on a call & try to figure this out if that helps!
Apart from that I'd also recommend addressing the PHP CS warnings.
Thank you for reviewing it! I'll be addressing all of those and let's wait for @vivialice to decide about the ones related to UX.
Hey @nefeline, thank you for your review! Yeah, I took a look into this, this issue occurs if you put the block in the Single Product template together with the classic template for the WooCommerce Single Product Block. To avoid that, I suggest to remove the default Single Product Block from the Single Product template before testing (step 5 of the User Facing Testing section in this PR) About the PHP CS warnings, I'm going to address them now, thank you for pointing this out! |
|
Should we put this block inside the folder |
I think if this requires a significant time investment then it's fine to ship. We can later work out whether it's worth fixing before deprecating the Classic Product block depending on at what point in the future that will be happening. But yes, it seems that this issue only occurs in a specific scenario which I would consider an edge case. |
By product elements, do you mean within
Naming it Product Details and updating the relative folder makes sense to me 👍 . Regarding the duplicate star ratings issue:
I have created woocommerce/woocommerce#42475, so we can keep track: I'm not entirely sure if this is something that takes a significant amount of time to fix as I didn't investigate it myself; there could be alternative paths/workarounds to explore. I'll spend some time on it between today/tomorrow just in case: if no quick fix is available, then I agree it makes sense to work on it later as it occurs only under specific circumstances. |
…ocks into feat/add-single-product-details-block
Technically, there isn't any difference between atomic blocks and other blocks. For now, I would put all of them into the Via 96fab80, I addressed those two points. Furthermore, I created a follow-up about icon #8427. @nefeline, could you do another look? |
src/BlockTypes/ProductDetails.php
Outdated
| $classname = $attributes['className'] ?? ''; | ||
|
|
||
| return sprintf( | ||
| '<div class="wp-block-woocommerce-single-product-details %1$s"> |
There was a problem hiding this comment.
The styles are now broken due to the latest changes to rename the block from single-product-details to product-details. The following should do the trick:
| '<div class="wp-block-woocommerce-single-product-details %1$s"> | |
| '<div class="wc-block-woocommerce-product-details %1$s"> |
I also suggest renaming the class prefix from wp-block to wc-block so it is aligned with the standards we are using in other blocks.
There was a problem hiding this comment.
We decided to avoid adding a custom class. We should use WP generated class (more context: p1675160559571929-slack-C02UBB1EPEF)
There was a problem hiding this comment.
We decided to avoid adding a custom class. We should use WP generated class (more context: p1675160559571929-slack-C02UBB1EPEF)
Oh cool, I missed that convo, thanks for sharing! I'll also go ahead and update #8284 with this prefix.
In case someone else comes across this convo: the reason for this decision was that for new blocks, it's a good idea not to introduce new classes, as block themes interact with blocks via theme.json.
|
Nice work! Apart from the issue with the styles flagged in the review, everything else looks a-ok 👍
Isn't it the case that the atomic blocks for Product Elements are supposed to be used as children of contextual blocks? In other words, the context can be used to pass product data down to all children blocks in a given tree? |
|
I just noticed some oddities: there are two console errors related to the
In the context of this block (the Product Details), I suspect it might be related to the recent changes that migrated this block to |
Mmm, good observation. All the more reason we have to add the blocks in the product-elements block: the Single Product block will use those blocks :D
Fixed for the Single Product Details, I will create a dedicated issue for the Product Image Gallery #8436! Good catch! EDIT: My first attempt was 43dd558, but after worked on #8445 I realized that there is a better approach. Check 85e6247. |
There was a problem hiding this comment.
Thanks for addressing all the feedback @gigitux ! Confirmed that:
- Styles are fixed.
- The console 404 error is fixed.
Ref. this discussion:
Mmm, good observation. All the more reason we have to add the blocks in the product-elements block: the Single Product block will use those blocks :D
Considering that you already moved this block and also the gallery block to this directory, sounds like it makes sense to have the Add to cart form block moved as well, right? I'll open a separate PR to work on this so we can unblock & merge #8284
|
Thanks for your great review! (and thanks @thealexandrelara for working on this block).
It makes sense! 👍 |
Create a block version of the product details (visible on the single product template), so merchants can add the product description, information, and reviews to their stores. In this first iteration, we are focusing in porting the existing functionality without allowing users to customize the block
Fixes #8186
Accessibility
prefers-reduced-motionScreenshots
Testing
Automated Tests
User Facing Testing
Changelog