-
Notifications
You must be signed in to change notification settings - Fork 219
[Blockifying Product Archive Templates]: Implement the blockified template conversion for the Classic Template Block. #8248
Conversation
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
assets/js/blocks/classic-template/archive-product.ts
assets/js/blocks/classic-template/index.tsx assets/js/blocks/classic-template/product-search-results.ts assets/js/blocks/classic-template/utils.ts |
Size Change: +2.78 kB (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
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.
To answer your questions:
I left an inline comment regarding that. But considering this script is only called from the Classic Template block, I would move it there.
I don't have any alternative idea. Let's see if somebody has one. Otherwise, this approach LGTM.
Which additional styles? If they need specific CSS, that should be probably be fixed in the block leve. 🤔 |
@Aljullu, thank you so much for a great review! 🙏
Can I confirm, by "move it there", do you mean next to Classic Template or
I meant the styles related to lay outing the blocks, so putting Product Results Count and Catalog Ordering in a row. Exactly what you showed in a comment suggesting to use Row block. So with your recommendation, the additional styles won't be necessary anymore and so the custom class. 👍 |
When used in a Row block in a blockified Archive Product template, Product Results Count had additional unnecessary margin which caused misalignment with the Catalog Sorting block
I answered inline above. 🙂
Great! |
It was added couple of commits earlier, since the template was kept there, but it was decided to move it to assets directory, so entry is no longer necessary
…on the availability to convert to Products block
Uncomment when Breadcrumb block becomes available await e...Uncomment when Breadcrumb block becomes available await expect( page ).toMatchElement( breadcrumbs );
woocommerce-blocks/tests/e2e/specs/backend/site-editing-templates.test.js Lines 351 to 359 in a52107d
🚀 This comment was generated by the automations bot based on a
|
Uncomment and add selector when Breadcrumb block becomes ...Uncomment and add selector when Breadcrumb block becomes available breadcrumbs: '',
woocommerce-blocks/tests/e2e/specs/backend/site-editing-templates.test.js Lines 129 to 137 in a52107d
🚀 This comment was generated by the automations bot based on a
|
… include Term Description block
…/woocommerce-blocks into add/convert-classic-template
As per the comment in similar PR, this PR is now extended with blockified config variation for Products By * templates. The difference between Product Catalog config is the Term Description block is included (commit). |
@@ -11,7 +11,8 @@ import { TemplateDetails } from './types'; | |||
export const BLOCK_SLUG = 'woocommerce/legacy-template'; | |||
export const TYPES = { | |||
singleProduct: 'single-product', | |||
archiveProduct: 'archive-product', | |||
productCatalog: 'product-catalog', | |||
productsBy: 'products-by', |
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.
To me, it's a bit unclear what productsBy
means. Can we consider other alternative here? Somethings like productTaxonomy
, productLoop
, productGrid
? or even keep archiveProduct
for shared template, and use productCatalog
just for the catalog page.
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.
To me, it's a bit unclear what productsBy means.
Thanks for feedback, makes sense! I Changed that to productTaxonomy
(commit)
…mplate conversion
This PR has been waiting for a final decision if the blocks: Breadcrumbs, Store Notices, Product Result Count, Catalog Sorting should surround Products block or be inner blocks: As per the PR: #8308, these blocks are the inner blocks of Products. This commit applies the change here. Also, the testing steps are amended now. Given that, this PR is ready for final testing and review. |
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.
This is looking awesome, good job! The code looks good and it's testing as expected. I have only seen one issue: when converting the Classic Template block to the new one, I can see several visual differences.
Classic template | Blockified template |
---|---|
Some things I see:
- We should include the Product Rating block, which is not included in the default pattern of the Products block.
- We should make the Product Title a link (I don't know why the Products block don't have it enabled by default, it might be better to fix it there?).
- The Product Price should be centered (IMO, this might be worth doing in the Products block directly).
I noticed your screenshots don't have some these issues. So maybe I have been doing something wrong? 😕
I started implementing that... But I think it's worth implementing it in Products block directly and include Product Rating by default in the Products inner blocks template. The reason being:
Good catch. I see
That's weird, it's centered by default for me and it's defined in here. So I'm not sure how to approach this one 🤔 |
@Aljullu, ok, I can reproduce it (here and on EDIT: issue created: #8521 |
Summary:
It's raised for internal discussion to confirm if the change could be included in the Products block directly. If not I'll add it to the templates within this PR.
It's fixed and merged to
Reproducible on |
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 would raise the question about extending Products with Product Rating. What do you think? 🤔
It makes sense to me. 👌
Good catch. I see
post-title
has an attributeisLink
. But I'll prepare a separate PR directly in Products block, so it can be confirmed by others.
Thanks!
Although, I tend to think we could merge this in a current shape (without Product Rating) and apply the change once there's a decision.
Sounds good. I agree that we can merge this PR now and work on the remaining tasks in follow-up PRs.
Thanks, @kmanijak!
Description
The scope of this PR:
align
attribute is passed to the PHP blocksRemaining work:
Fixes #8066
Screenshots/video
Product Archive - side by side comparison (editor):
Product Archive - side by side comparison (frontend):
Product Archive - copy update (editor)
Single product - copy update (editor)
Video presents the replacement of a Classic Template on two templates: Product Catalog and Products by Category
classic-template.mov
Testing
Automated Tests
E2E tests will be added as a separate PR
User Facing Testing
Conversion is not yet possible (hidden behind feature flag)
Prerequisites:
WordPress: >=6.1
Make sure Single Product template is cleared out to the default state. To achieve that:
/wp-admin/site-editor.php?postType=wp_template
)Steps
Expected
Repeat for Product Catalog, Products by Category and Product Search Results templates. The placeholder description may vary slightly.
User Facing Testing - behind the experimental flag.
Conversion is not possible on WP lower than 6.1
Prerequisites:
WordPress: <6.1
Make sure Product Catalog and Products by Category templates are cleared out to the default state. To achieve that:
/wp-admin/site-editor.php?postType=wp_template
)Steps
Expected
Convert classic templates on Product Archive templates
Prerequisites:
/wp-admin/site-editor.php?postType=wp_template
)Steps
Expected
WooCommerce Product Grid Block is replaced with blocks:
Steps - continue
/shop
) and check it the blocks are displayed correctly.Expected
- Check the Product Results Count displays proper numbers, e.g. go to the page 2 (it should say "Showing 17–17 of 17 results" if you're using "standard" products set). - Check the Catalog Sorting sorts the products correctly, e.g. by price.
Repeat the above for template:
/product-category/clothing/
)Convert classic templates on Product Search Results
Prerequisites:
/wp-admin/site-editor.php?postType=wp_template
)Steps
Expected
WooCommerce Product Grid Block is replaced with blocks:
Steps - continue
/shop
)t shirt
orbeanie
Expected
Steps - continue
blah blah
orcars
Expected
Align attribute is preserved
Prerequisites:
WordPress: >=6.1
Make sure Product Catalog template is cleared out to the default state. To achieve that:
/wp-admin/site-editor.php?postType=wp_template
)Steps
align
setting of the Classic Template block to either "None" or "Full width"3. Click "Upgrade to Products block" button
Expected
align
setting in Editor:Steps - continue
/shop
)Expected
WooCommerce Visibility
Changelog