-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change product recommendations name to related products #2161
Conversation
sections/main-product.liquid
Outdated
@@ -293,7 +293,7 @@ | |||
</p> | |||
{%- endif -%} | |||
{%- when 'complementary' -%} | |||
<product-recommendations class="complementary-products quick-add-hidden no-js-hidden{% if block.settings.make_collapsible_row %} is-accordion{% endif %}{% if block.settings.enable_quick_add %} complementary-products-contains-quick-add{% endif %}" data-url="{{ routes.product_recommendations_url }}?section_id={{ section.id }}&product_id={{ product.id }}&limit={{ block.settings.product_list_limit }}&intent=complementary"> | |||
<related-products class="complementary-products quick-add-hidden no-js-hidden{% if block.settings.make_collapsible_row %} is-accordion{% endif %}{% if block.settings.enable_quick_add %} complementary-products-contains-quick-add{% endif %}" data-url="{{ routes.product_recommendations_url }}?section_id={{ section.id }}&product_id={{ product.id }}&limit={{ block.settings.product_list_limit }}&intent=complementary"> |
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.
So we have two types of recommended products... related recommendation and complementary recommendations. I'm not sure changing the html element to related-products
makes sense, as this is a block that's referencing complementary.
So a related product is a specific type of recommendation and a complementary product is another specific type of recommendation.
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.
Gotcha 🤔 So the custom element with the general behaviour could be kept as product-recommendations basically ?
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 believe so. I'm jumping into a meeting now so I didn't really get a chance to look at all the code, but "product recommendation" should be our general case with specialties around "complementary" and "related"
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.
Everything looks good to me and works as expected but I wanted to share some feedback.
Heading name label
Unrelated to this PR, I'm not sure if this was intentional but it could pose more problems in the future when it comes to other heading names / labels: screenshot
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 adding in these changes: 37e3044
- Go to the product page and make the "related products" as working as usual/expected
- Add the complementary block to the product information and make sure it works as expected as well. There are complementary products setup for the
puppy
product
I also tested in Safari and Firefox on desktop and mobile and everything looks great and works as expected.
dc3534c
dc3534c
to
59228e6
Compare
Had to do a rebase, so re requesting approvals. Translations are back too 👍 |
@ludoboludo is this good to merge now? |
* shopify/main: (57 commits) Change sticky header settings (Shopify#2165) Image setting updates (Shopify#2148) Update 1 translation file (Shopify#2183) Update 2 translation files (Shopify#2182) Fix mega-menu vertical shadow issue (Shopify#2147) Update translations: buyer (Shopify#2180) Change product recommendations name to related products (Shopify#2161) Update 2 translation files (Shopify#2178) Add default text for cart-notification link (Shopify#2167) Update 1 translation file (Shopify#2177) Update 20 translation files (Shopify#2176) Update 1 translation file (Shopify#2174) Updates to collage settings and defaults (Shopify#2150) Update 1 translation file (Shopify#2173) Update issue templates Update layout label (Shopify#2164) Search ux improvements (Shopify#2127) Prettier all liquid files [part 2] (Shopify#2126) [Footer] Add 'Show policy links' setting (Shopify#2084) [Header] Move 'Logo' to Global Theme settings (Shopify#2083) ...
* change product recommendations to related products * change any reference to product-recommendations into related-products * rebase * edit CSS file and class name * Update 20 translation files * fix rebase Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>
PR Summary:
A copy change requested by team working on recommendations.
Why are these changes introduced?
Fixes https://github.com/Shopify/discovery-app/issues/1871
What approach did you take?
Renamed the header of the section.
Other considerations
All our naming in the code refers to
product-recommendation(s)
. I'm made a commit in which I change all that too so it doesn't end up being confusing, IF we think it is confusing. Happy to revert based on feedback.Decision log
Visual impact on existing themes
The only visual impact should be the section name on the product page.
Testing steps/scenarios
puppy
product.Demo links
Checklist
cc: @NathanPJF @jas7457