Mini Cart as template part #5025
Conversation
Aljullu
added
focus: FSE
rubikuserbot
requested review from
a team and
tjcafferkey
and removed request for
a team
|
Size Change: +3.17 kB (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
Aljullu
force-pushed
the
fix/mini-cart-as-template-part
branch
3 times, most recently
from
1c21969
to
de2ab4d
Compare
| @@ -36,7 +36,7 @@ const setUp = (): void => { | |||
| const addListeners = (): void => { | |||
| setUp(); | |||
|
|
|||
| if ( ! window.wcBlocksStoreCartListeners.count ) { | |||
| if ( window.wcBlocksStoreCartListeners.count === 0 ) { | |||
There was a problem hiding this comment.
This change is not directly related to this PR, but still wanted to add it. 🙂
count is going to be an integer, so it's more precise to compare it to 0 than to check if it's falsy.
Aljullu
added
skip-changelog
There was a problem hiding this comment.
This is working great on my end. I just have some minor comments:
- Because we support the template parts now, we need to pass the template type to line this and this line. Without this change, the edit link of the Mini cart content template is broken because of wrong template type.
- I got a warning about the missing
areaproperty of the template part:PHP Warning: Undefined property: WP_Block_Template::$area in /wp-content/plugins/gutenberg/lib/compat/wordpress-5.9/class-gutenberg-rest-templates-controller.php on line 418.
Aljullu
force-pushed
the
fix/mini-cart-as-template-part
branch
2 times, most recently
from
550e0bf
to
63222ba
Compare
|
Thanks for the review @dinhtungdu! Good catch! Should be fixed in a730af0.
For now, I set it to I think this issue (WordPress/gutenberg#35989) is blocking us, but might be good to take a decision anyway so we can create an issue in our repo and follow-up based on it. |
@Aljullu At the first glance, it seems that the Mini Cart only needs one template part because we only allow one mini cart per page. But given that we have multiple header template parts, and Mini Cart is "usually" added to the header, the cart content style should match with the header (and page style), to do that, we need an area for Mini Cart. So for your question, my answer is yes, we should create a Mini Cart area. |
|
Note that with the latest changes merged to
|
Aljullu
force-pushed
the
fix/mini-cart-as-template-part
branch
from
63222ba
to
c9c2f57
Compare
|
TIL, thanks for letting me know @dinhtungdu! I created three follow-up issues based on that: #5194, #5195 and #5196. Could you review them and let me know if they make sense to you and look like the correct next steps? Regarding this PR, what do you think about merging it in the way it currently works and handling anything else in follow-ups? |
There was a problem hiding this comment.
@Aljullu This is working great on my end, I don't get any PHP error or warning 👍🏽 . I have some minor comments btw, please check the review comment for more detail.
src/Utils/BlockTemplateUtils.php
Outdated
| if ( 'wp_template_part' === $post->post_type ) { | ||
| $template->area = 'uncategorized'; | ||
| } |
There was a problem hiding this comment.
I think we should get the template area from the database instead of hard-coding it like this. We can get the area of a template part using get_the_terms( $post, 'wp_template_part_area' );
src/Utils/BlockTemplateUtils.php
Outdated
| if ( 'wp_template_part' === $template_type ) { | ||
| $template->area = 'uncategorized'; | ||
| } |
There was a problem hiding this comment.
With this code, all template parts will have area set to uncategorized. This is fine for now. But I think we need to add a todo here to improve this in the future. Each template part may have a different area, and we can filter that area base on the slug IMO.
@Aljullu The follow-up issues make a lot of sense to me! The #5196 is actually a very nice UX enhancement! 🎉
I think we should fix #5025 (comment) and leave #5025 (comment) for the next release. |
|
Thanks for taking another look @dinhtungdu!
Actually, in 5ee58d2 I tried to fix it in both places, but happy to undo the second change if you are not comfortable with it. In any case, I think that's something that we can keep iterating on in the follow-up issues. |
There was a problem hiding this comment.
Actually, in 5ee58d2 I tried to fix it in both places, but happy to undo the second change if you are not comfortable with it. In any case, I think that's something that we can keep iterating on in the follow-up issues.
@Aljullu 5ee58d2 looks good to me. Initially, I think it's better to extract the logic for the template part area into its own function (under the BlockTemplateUtils class). But given that this PR is already large, and we haven't known if we need other template part areas yet. So I think we so keep it as it is for now, and improve it later if we actually have so many template part areas.
src/BlockTemplatesController.php
Outdated
| if ( 'wp_template_part' === $template_type ) { | ||
| $dir_name = self::TEMPLATE_PARTS_DIR_NAME; | ||
| } else { | ||
| $dir_name = self::TEMPLATES_DIR_NAME; | ||
| } |
There was a problem hiding this comment.
We should move this check outside of the foreach loop.
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
* Fix wrong event prefix in doc comment * Make className prop in CartLineItemsTableProps optional * Mini Cart as template part * Remove BlockTemplatePartsController and instead use BlockTemplatesController * Remove old code * Clean up frontend rendering * Update tests * Improve if clause * Fix wrong tests title * Fix wrong variable name * Make sure Mini Cart contents block is unmounted whem mini cart closes or unmounts * Remove unnecessary waitFor * Fix PaymentMethodDataProvider wrong children type * TypeScript fixes * Make comment shorter * Remove test code * Fix contant unmounts of Mini Cart contents block * Fix wrong template_type passed * Set Template part area to 'uncategorized' * Set Template part area to the correct value * Move template dir check outside loop
* Fix wrong event prefix in doc comment * Make className prop in CartLineItemsTableProps optional * Mini Cart as template part * Remove BlockTemplatePartsController and instead use BlockTemplatesController * Remove old code * Clean up frontend rendering * Update tests * Improve if clause * Fix wrong tests title * Fix wrong variable name * Make sure Mini Cart contents block is unmounted whem mini cart closes or unmounts * Remove unnecessary waitFor * Fix PaymentMethodDataProvider wrong children type * TypeScript fixes * Make comment shorter * Remove test code * Fix contant unmounts of Mini Cart contents block * Fix wrong template_type passed * Set Template part area to 'uncategorized' * Set Template part area to the correct value * Move template dir check outside loop
This PR updates how the Mini Cart block works so contents shown inside the drawer belong to a template part. That allows themes to override it and users to edit it from the Template Parts editor.
In summary, the changes are:
BlockTemplatesControllerso it can load block template parts alongside block templates.Testing
To make it easier to test, checkout this branch from Gutenberg so you can edit and see the template parts added by WooCommerce: WordPress/gutenberg#36379.
Also, make sure to test with a block theme, ie: Quadrat or Tove.
WooCommerce template part
Theme template part
/block-template-parts/mini-cart.htmlfile in your theme directory with these contents:This is the theme template parttext appears in the drawer.User template part
mini-carttemplate and edit its contents (ie, replacingThis is the theme template part.withThis is the user edited template part.).Classic theme