-
Notifications
You must be signed in to change notification settings - Fork 218
Conversation
Add Search Result Product Template
…products-block into add/6218-search-result-product-template
Size Change: 0 B Total Size: 862 kB ℹ️ 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.
Really excited with this PR! In combination with #6134 it will allow adding filters to the Search results template. 🥳 Good job!
It works great and code LGTM. I left some minor suggestions below related to the copy. 👇
* | ||
* @internal | ||
*/ | ||
class SearchResultsProductTemplate { |
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.
Just curious, any reason why this is named SearchResultsProductTemplate
? ProductSearchResultsTemplate
would seem more natural to me.
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.
namespace Automattic\WooCommerce\Blocks\Templates; | ||
|
||
/** | ||
* ProductSearchTemplate class. |
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 guess this comment should use the name of the class?
*/ | ||
class SearchResultsProductTemplate { | ||
|
||
const SLUG = 'search-results-product'; |
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.
If possible, I would change the slug as well:
const SLUG = 'search-results-product'; | |
const SLUG = 'product-search-results'; |
|
||
const SLUG = 'search-results-product'; | ||
const TITLE = 'Product Search Results'; | ||
const DESCRIPTION = 'Template used to display search results for products'; |
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.
Nit: missing dot in the description.
const DESCRIPTION = 'Template used to display search results for products'; | |
const DESCRIPTION = 'Template used to display search results for products.'; |
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.
Sidenote: it would be nice to add descriptions to all other WC templates, not in this PR, but maybe a follow-up good first issue?
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.
Good point, created a new issue #6233
} | ||
|
||
/** | ||
* When the search is for products and it used a block theme the Product Search Template is rendered. |
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.
Small rewording suggestion:
* When the search is for products and it used a block theme the Product Search Template is rendered. | |
* When the search is for products and a block theme is active, render the Product Search Template. |
…products-block into add/6218-search-result-product-template
…woocommerce/woocommerce-gutenberg-products-block into add/6218-search-result-product-template
a3b2f14
to
a063c5a
Compare
Thanks for the review. I added your suggestions! |
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.
Now I'm getting a Saving failed
error when trying to save the Product Search Results template. 😕 e2e tests related to this PR seem to be failing too.
|
||
it( 'should contain the "WooCommerce Product Grid Block" classic template', async () => { | ||
await goToTemplateEditor( { | ||
postId: 'woocommerce/woocommerce//search-results-product', |
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.
Should we update the slug here too just for consistency?
…products-block into add/6218-search-result-product-template
0a8f9fd
to
f1d51de
Compare
f1d51de
to
392861b
Compare
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.
LGTM, thanks for working on this @gigitux!
This PR adds a new template called
Product Search Results
. This template will be rendered only on the block themes when the user lands on?s={keyword}&post_type=product
.Fixes #6218
Screenshots
Product Search Results in the template list
Editor view when the user edits the Product Search Result template
Frontend side when the user lands on
?s=shirt&post_type=product
url. The template Product Search Results is renderedTesting
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
Check out this branch
Twenty Twenty-Two
).?s={product_to_search}&post_type=product
to your website URL. For examplehttp://dev.local/?s=shirt&post_type=product
. You should see a grid with the products that match the keyword (or a not found message if any product matches the keyword). Keep this page open.?s={product_to_search}&post_type=product
everything works well. IMPORTANT: Remember that for the classic themes the templates are not loaded. So the implementation of this page depends on the theme.Extendibility for the themes
With these steps, we will check that the themes can extend the new template:
Twenty Twenty-Two
).Twenty Twenty-Two
the path is/themes/twentytwentytwo/
.templates
and create a filesearch-results-product.html
with this content.?s={product_to_search}&post_type=product
to your website URL. For examplehttp://dev.local/?s=shirt&post_type=product
. You should see a grid with the products that match the keyword (or a not found message if any product matches the keyword) and a paragraph with the textColumn One, Paragraph One
.Storefront
). Check that if you visit?s={product_to_search}&post_type=product
everything works well. IMPORTANT: Remember that for the classic themes the templates are not loaded. So the implementation of this page depends on the theme.User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
Twenty Twenty-Two
).?s={product_to_search}&post_type=product
to your website URL. For examplehttp://dev.local/?s=shirt&post_type=product
. You should see a grid with the products that match the keyword (or a not found message if any product matches the keyword). Keep this page open.Storefront
). Check that if you visit?s={product_to_search}&post_type=product
everything works well. IMPORTANT: Remember that for the classic themes the templates are not loaded. So the implementation of this page depends on the theme.Changelog