This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update the archive templates to use Products block #8308
Update the archive templates to use Products block #8308
Changes from 10 commits
4ad99c2
1353b3a
abd1482
4b48504
001685f
c2c1b77
763934e
9c2d701
2f3fd99
0cd5768
2e5d304
6b3d6d3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It would be great if we could leave a comment here just explaining why we're doing 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.
For the
Product Catalog
template there will be noterm
, orterm-description
however it is displayed in the Site Editor. Is it possible to conditionally render this if the user is viewing the templates: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.
Hmm, good catch.
Currently, all archive templates use the same
archive-product.html
template (see #7712). I think we can do one of:archive-product.html
.@albarin @tjcafferkey what do you think is the best direction here?
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 feels a little bit hacky and we could end up setting a precedent for other people to do the same thing for other template scenarios which might not be ideal. I'm open to people disagreeing though.
This feels like a better option but comes with the overhead of maintaining two templates, would that be a problem?
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.
You read my mind Tom! If I have to choose one now, that'd be the second direction. But I'm not happy with either of them.
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 missed this block in template conversion PR. I'll include it there as well for Products by Category, Products by Attribute and Products by Tag
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.
Not completely sure, but I think this would have more implications for the whole fallback system in place. Currently, if you make a modification to the
Product Catalog
template you should see it on the other three (cat, tag, attr). If we have a separate template with the description I think we'd loose this 🤔I'll throw another option there, maybe unpopular 😬 what about just keeping the description block in there?
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.
@albarin Aha! It's a legit one. Let's consider it also.
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.
Definitely an option for now, the only problem is that it does not reflect what users will experience on the frontend. I wonder if its possible to put a note next to these blocks perhaps which explains "On the main Product Catalog page this information will not be visible" etc.
Just cc'ing @vivialice into this incase she has any ideas or would like to weigh in on one of the options.