Create Cross-Sells product list #6645
Conversation
nielslange
added
type: new block
Script Dependencies ReportThe
This comment was automatically generated by the |
1 similar comment
Script Dependencies ReportThe
This comment was automatically generated by the |
|
Size Change: +18.8 kB (+2%) Total Size: 893 kB
ℹ️ View Unchanged
|
Script Dependencies ReportThe
This comment was automatically generated by the |
nielslange
force-pushed
the
add/6558-create-cross-sells-product-list
branch
from
c109c1b to
ea612d6
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
There was a problem hiding this comment.
Hi @nielslange! You can find below the fix of the bug you reported on Slack!
Script Dependencies ReportThe
This comment was automatically generated by the |
8 similar comments
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
|
One thing missing is support for products with options. See attached screenshots for differences between blocks and shortcode on cross-sells: Block version Shortcode version We can make this an actionable item to work in our pairing session @nielslange Also, empty cross-sells still display the heading And we're missing the grey placeholder styles |
Script Dependencies ReportThe
This comment was automatically generated by the |
7 similar comments
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
There was a problem hiding this comment.
Thank you, @nielslange, for working on this great PR! 🙌
This is the first time I'm testing the PR, so I'm not sure if I did something wrong, but, I'm getting an error on the editor side of the Cart Block:
And on the front-end I'm getting the following error:
@tarhi-saad I double-checked the PR and noticed that the error occurred due to renaming variables in 8f1c545. I just resolved that problem. The Cross-Sells block should now appear, as expected, both in the editor and the frontend. |
There was a problem hiding this comment.
Thanks @nielslange I tested and it works great - I'll mention something I wondered about. When it comes to removing the block, if you remove the actual block and the heading above it the block still remains, is there anything we can do about this?
Also, the block reappeared once when I removed the product list and heading. Could you look into why this is happening? (You can see this on the video below)
Thanks for your review and pointing this out, @opr.
I looked into options, but cannot find anything for now. That said, I noticed that Gutenberg's Group block and our Product Query and Products on Sale blocks behave similar. When deleting inner blocks, both siblings and the parent block remain unaffected. In addition, the parent block does not get deleted if it has no more inner blocks. I agree that this is not ideal, but I'd rather look into this issue in a follow-up issue. Given that this problem also appears when using Gutenberg's Group block, this issue might be addressed in upstream.
|
There was a problem hiding this comment.
Thank you, @nielslange, for working on this PR! I've gone through all the tests mentioned above, and everything is working as expected! 🙌
I looked into options, but cannot find anything for now. That said, I noticed that Gutenberg's Group block and our Product Query and Products on Sale blocks behave similar. When deleting inner blocks, both siblings and the parent block remain unaffected. In addition, the parent block does not get deleted if it has no more inner blocks. I agree that this is not ideal, but I'd rather look into this issue in a follow-up issue. Given that this problem also appears when using Gutenberg's Group block, this issue might be addressed in upstream.
I believe what makes the Cross-Sells different than some of Gutenberg's blocks, is that we prevent the user from adding the Cross-Sells product list block if the parent isn't deleted. This can be frustrating to the user wondering why he isn't allowed to add the block again after removing it using the Editor UI (see image below):
The image above shows that there is no Cross-Sells product lits block, nonetheless, we can't add one! The only way to fix the issue will be for the user to open the List View and notice the parent block is still there.
If there isn't an easy solution to remove the parent when removing its inner block, can we look into preventing removing the inner block itself, and only allowing removing the block by deleting the parent? I believe there is a Gutenberg block that works this way.
Also, the block reappeared once when I removed the product list and heading. Could you look into why this is happening? (You can see this on the video below)
I was able to replicate this bug on my end as well. It only happens once after a page refresh, so to reproduce the same bug you'll need to refresh the page and try again:
- Go to Cart with the Cross-sell block
- Remove the
Cart Cross-Sells productsinner block first (using the UI or through the List View). - Then remove the
Headinginner block. Both inner blocks will show up again! - If the inner blocks were removed as expected, refresh the page and try again!
Lastly, I noticed that the Cart Cross-Sells block doesn't show for the existing Cart Blocks. We should remove our existing Cart and add it again to work. Is this an expected behavior? To reproduce:
- Checkout to the
trunkbranch - Execute
npm run build - Create a new page and add a Cart Block
- Checkout to this branch (i.e.,
add/6558-create-cross-sells-product-list) - Execute
npm run build - refresh the newly create Cart page. Check there isn't a
Cart Cross-Sells block
…:woocommerce/woocommerce-blocks into add/6558-create-cross-sells-product-list
|
Thanks for your review, @tarhi-saad!
I also had that solution in mind, but initially decided against it, as I found the usability too confusing. That said, I just locked the Cross-Sells products inner block. That way, a merchant can either delete the headline or the whole Cross-Sells block, but not the Cross-Sells products inner block.
Apparently, by locking the Cross-Sells products inner block, this problem seems to have been solved as well. At least, I can no longer reproduce it, while I was able to reproduce it, using your steps, before the inner block was locked.
That's intentional and was one of the project deliverables. Would you mind reviewing this PR again, @tarhi-saad ? |
There was a problem hiding this comment.
Thank you for the clarification, @nielslange! I've left some comments below, but nothing crucial! It's just things good to have or debatable! So, feel free to merge the PR after this! 🙌
assets/js/blocks/cart/inner-blocks/cart-cross-sells-products/block.json
Outdated
Show resolved
Hide resolved
sunyatasattva
added
the
type: enhancement
* Create Cross-Sells product list * Show “Read more” button for out-of-stock cross-sells products * Update assets/js/blocks/cart/inner-blocks/cart-cross-sells-products/block.tsx Co-authored-by: Thomas Roberts <[email protected]> * Update assets/js/blocks/cart/cart-cross-sells-product-list/index.tsx Co-authored-by: Thomas Roberts <[email protected]> * Remove obsolete isLoading and placeholderRows * Fix TS errors * Rename crossSellsProduct to product * Fix critical error * Lock “Cart Cross-Sells products” inner block * Update assets/js/blocks/cart/inner-blocks/cart-cross-sells-products/block.json Co-authored-by: Saad Tarhi <[email protected]> * Prevent moving of the Cross-Sells block Co-authored-by: Thomas Roberts <[email protected]> Co-authored-by: Saad Tarhi <[email protected]>
Fixes #6558
Accessibility
prefers-reduced-motionOther Checks
Screenshots
Testing
Test editor behaviour when adding Cart block
Test editor behaviour after removing Cross-Sells block
Test frontend behaviour if cart items does not have Cross-Sells products
Test frontend behaviour if cart items have Cross-Sells products
Test frontend behaviour if cart items have a Cross-Sells product that is out of stock
Add to Cartbutton, but aRead Morebutton, that links to the corresponding product page.Test number of visible Cross-Sells products in the frontend.
WooCommerce Visibility
Performance Impact
n/a
Changelog