-
Notifications
You must be signed in to change notification settings - Fork 219
Fix: Mini Cart block: divide contents into three inner blocks #5386
Conversation
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/ce31fb0e1d3d99440fcd61dd118b099f59090d3f/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
Allow the user to edit the title of the mini cart.Allow the user to edit the title of the mini cart. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/ce31fb0e1d3d99440fcd61dd118b099f59090d3f/assets/js/blocks/cart-checkout/mini-cart-contents/inner-blocks/mini-cart-title-block/block.tsx#L14-L25🚀 This comment was generated by the automations bot based on a
|
Review the class naming convention for Mini Cart inner bl...Review the class naming convention for Mini Cart inner blocks. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/ce31fb0e1d3d99440fcd61dd118b099f59090d3f/assets/js/blocks/cart-checkout/mini-cart/style.scss#L79-L90🚀 This comment was generated by the automations bot based on a
|
Size Change: +85 B (0%) Total Size: 819 kB
ℹ️ View Unchanged
|
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/c98294580e08a1d0f18580150d7f56e0fb1ea68c/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
Co-authored-by: Albert Juhé Lluveras <[email protected]>
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/f2b50657a08ec3657435de35743a40a4aee190f9/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/ed2d86105be76ceced23ef22ea49af0eca31792f/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/8bbaf3853ab4b4c9522c8bfa10a9cbaf1e3230b6/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
@Aljullu Given that this PR is already big, I would prefer to work on those todos in follow-ups to make it easier to review. |
I think we should. But it should be addressed in a separated PR because of its complexity. I would like to change only the colors of the Mini Cart Contents block, including text and link color, then the inner blocks should reflect those customizations. I will create another to-do if the proposal above makes sense to you. |
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 updating this PR @dinhtungdu! It looks good to me.
@Aljullu Given that this PR is already big, I would prefer to work on those todos in follow-ups to make it easier to review.
Yes, it works for me. Just be aware that for each @todo
comment a new issue will be automatically created, so you will need to assign them to the Mini Cart epic.
I think we should. But it should be addressed in a separated PR because of its complexity. I would like to change only the colors of the Mini Cart Contents block, including text and link color, then the inner blocks should reflect those customizations.
I will create another to-do if the proposal above makes sense to you.
💯 Sure, that's what I had in mind as well.
const Block = (): JSX.Element => { | ||
const { cartItemsCount, cartIsLoading } = useStoreCart(); | ||
/** | ||
* @todo Allow the user to edit the title of the mini cart. |
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 will be tricky because the string contains a variable (x items). I think @nielslange faced the same challenge in the Free shipping progress bar block and solved it with a placeholder in curly braces ({amount}
). It would be good to make sure we align on the same solution.
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.
@dinhtungdu I wonder how many merchants would actually change the title of the mini-cart block. As @Aljullu mentioned, I used a placeholder while implementing woocommerce/woocommerce-free-shipping-progress-bar-block#15. However, that block has the following editable texts:
Spend only {amount} more to get free shipping!
You have qualified for free shipping. Great job!
That said, if the mini-cart block title should be editable, I achieved that using the switch-switcher, which I see you also used in #5304 already.
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 your inputs! I will remove the to-do for now. If we receive enough requests in the future, we can work on it then. For now, I don't think it's worth the effort.
assets/js/blocks/cart-checkout/mini-cart-contents/inner-blocks/mini-cart-footer-block/block.tsx
Outdated
Show resolved
Hide resolved
Something I noticed while doing some more testing is that in WP 5.9 beta 3 with Gutenberg disabled, I can check the color in the editor, but the new color is not applied in the frontend. @dinhtungdu can you reproduce that too? If so, it would be great to create an issue about that so we can investigate it further. |
…/mini-cart-footer-block/block.tsx Co-authored-by: Albert Juhé Lluveras <[email protected]>
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/45a6c00f6b411dba729275476c6c13b67eb794ca/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
The `div` wrapper of RawHTML isn't removed on the front e...The `div` wrapper of RawHTML isn't removed on the front end. */ } https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/8d08c6056afede0f97ac204e6add679bc8d6e0d1/assets/js/blocks/cart-checkout/mini-cart/block.tsx#L197-L200🚀 This comment was generated by the automations bot based on a
|
I can reproduce this issue as well. I created #5416 to track this. |
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.
Great work! 🎉 🚢
Fixes #4855
This PR:
<Drawer>
title.mini-cart
template part file.MiniCartContents.php
to match with the component.Screenshots
Click to expand
Editor
Front end
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
/wp-admin/edit.php?post_type=wp_template_part
.User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).