Update main product sibling sections when switching between combined listing children #3368

lhoffbeck · 2024-03-25T19:29:53Z

PR Summary:

All child sections of a main product may contain references to the product through metafields. Given this, we want to ensure that all sections of the main product are refreshed when switching between child products.

Demo video of error: https://screenshot.click/23-29-tjpdt-m09nb.mp4

What approach did you take?

Chatted with @tyleralsbury , the recommendation was to request a full page update concurrently with the section api call to update the product info section, then use the response to update other sections. (The full page request seems to take anywhere from a few to a few hundred MS longer than the product section request so it likely makes sense to keep this as a separate request.)

Other considerations

We could add a refreshable-section component that listens for an event and self-refresh.
product-info.js could declaratively update all sections associated with the product

Visual impact on existing themes

This will only impact combined listings products

Demo links

https://combined-listings-test.myshopify.com/products/chinos-orange?variant=47651689038126
Product has a card that surfaces the name of product through a metafield. The card content should be updated to show the new product title after switching products: https://screenshot.click/26-35-1339m-j6nxn.mp4

Checklist

Added PR summary for release notes
Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
Followed theme code principles
Linted with Theme Check
Tested on mobile
Tested on multiple browsers
Tested for accessibility

lhoffbeck · 2024-03-26T13:04:07Z

assets/product-info.js

+            );
+
+            document.querySelectorAll('main > .shopify-section').forEach((section) => {
+              if (!section.id.includes('__main') && updatedSections[section.id]) {


@tyleralsbury / @tauthomas01 I'm not sure how section IDs are generated.... is it a safe assumption to make that the section IDs will match between the initial product and the sibling products?

Section id is generated via the JSON template so it should be fine.

In this case, __main is generated from the product.json templates. These are section identifier that you can set in the template.

Here's an example of collection page:

lhoffbeck · 2024-03-26T13:06:16Z

assets/product-info.js

+          .then((response) => response.text())
+          .then((responseText) => {
+            const html = new DOMParser().parseFromString(responseText, 'text/html');
+            const updatedSections = Array.from(html.querySelectorAll('main > .shopify-section')).reduce(


@tyleralsbury / @tauthomas01 It looked like selecting shopify-sections under main targets all sections that could contain a product metafield (and so would need to be updated). Is that correct?

lhoffbeck · 2024-03-26T13:11:57Z

assets/product-info.js

+              }
+            });
+          });
+      }


This function is pretty naive and assumes that sibling products of a combined listing will use the same product template. If the product sections differ the UX will be pretty poor.

On the other hand, the primary use case for combined listings is to combine many products to appear as a single product. Because of this, the functionality between combined listing children should be identical in most cases, and children having different content should be a slim edge case.

Given that this isn't a one-way door (e.g. merchants can update theme code to accommodate multi-template CLs) and that it is an edge case, our designer on combined listings thought this was an acceptable tradeoff.

What do you think?

I think this is a reasonable assumption. Is there anyway to detect if the template changes between child products and print a warning or anything?

I'm assuming this would manifest by updates not working or looking wonky, and I could see it being confusing/frustrating to try and figure out what is going on if you don't realize products have different templates or know that is a limitation.

Yeah good idea, maybe we add a console.error that mentions that's what's happening here 🤔

assets/product-info.js

MiniLuke

Looks good, tophat checks out for me. Thanks for the good explanations and annotations.

MiniLuke · 2024-03-26T18:56:56Z

assets/product-info.js

+              }
+            });
+          });
+      }


I think this is a reasonable assumption. Is there anyway to detect if the template changes between child products and print a warning or anything?

I'm assuming this would manifest by updates not working or looking wonky, and I could see it being confusing/frustrating to try and figure out what is going on if you don't realize products have different templates or know that is a limitation.

MiniLuke · 2024-03-26T18:59:24Z

assets/product-info.js

      #handleSwapProduct() {
        return (html) => {
          this.productModal?.remove();
          const newProduct = html.querySelector('product-info');
          this.swapProductUtility.viewTransition(this, newProduct);
-          this.relatedProducts?.initializeRecommendations(newProduct.dataset.productId);
-          this.quickOrderList?.refresh();


I assume these sections are covered by the overall updateProductSections now?

right exactly

tauthomas01 · 2024-03-27T13:44:03Z

assets/product-info.js

+            );
+
+            document.querySelectorAll('main > .shopify-section').forEach((section) => {
+              if (!section.id.includes('__main') && updatedSections[section.id]) {


Section id is generated via the JSON template so it should be fine.

In this case, __main is generated from the product.json templates. These are section identifier that you can set in the template.

Here's an example of collection page:

lhoffbeck · 2024-03-27T17:12:34Z

assets/product-info.js

+            });
+          });
+      }
+


TODO this function should publish the section_refreshed event once complete

…election is changed to a sibling

lhoffbeck · 2024-05-23T19:50:00Z

Closing, this will be fixed by the changes introduced here: #3289

Ultimately, the approach that combined listings takes for swapping product content is:

If the product is a main-product: Fetch and replace the entire content of <main>. In this case, any other sections on the page may need to be refreshed due to dependencies on the main product--recommended products based on main product ID, content powered by product metafields, etc. Additionally, combined listing siblings may have different product templates and contain different sections.
If the product is in an embedded context (modal, featured-product, etc): Replace just the <product-info> component.

lhoffbeck changed the base branch from main to add-product-wrapper-class March 25, 2024 19:30

lhoffbeck changed the title ~~Lh prototype combined listing pdp section updates~~ Update main product sibling sections when switching between combined listing children Mar 25, 2024

lhoffbeck force-pushed the lh-prototype-combined-listing-pdp-section-updates branch from d632f13 to c760ec5 Compare March 25, 2024 20:26

lhoffbeck force-pushed the add-product-wrapper-class branch from ae7a814 to 731a790 Compare March 25, 2024 20:54

lhoffbeck force-pushed the lh-prototype-combined-listing-pdp-section-updates branch from f531545 to 2a22f8a Compare March 26, 2024 13:01

lhoffbeck commented Mar 26, 2024

View reviewed changes

assets/product-info.js Show resolved Hide resolved

lhoffbeck requested review from tyleralsbury and tauthomas01 March 26, 2024 14:09

tauthomas01 requested a review from MiniLuke March 26, 2024 17:22

lhoffbeck force-pushed the lh-prototype-combined-listing-pdp-section-updates branch from 20510e2 to 2a22f8a Compare March 26, 2024 17:44

MiniLuke approved these changes Mar 26, 2024

View reviewed changes

tauthomas01 approved these changes Mar 27, 2024

View reviewed changes

lhoffbeck commented Mar 27, 2024

View reviewed changes

assets/product-info.js

});

});

}

Copy link

Contributor Author

lhoffbeck Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO this function should publish the section_refreshed event once complete

lhoffbeck mentioned this pull request Mar 28, 2024

Support Combined Listings and 2k Variants #3246

Merged

lhoffbeck mentioned this pull request May 17, 2024

Combined listings and 2k variants #3378

Merged

9 tasks

lhoffbeck force-pushed the add-product-wrapper-class branch 8 times, most recently from 831e093 to 45f65d3 Compare May 23, 2024 15:58

(#3368) Update all sibling sections when a combined listing product s…

7e35aa4

…election is changed to a sibling

lhoffbeck force-pushed the lh-prototype-combined-listing-pdp-section-updates branch from 2a22f8a to 7e35aa4 Compare May 23, 2024 16:06

lhoffbeck force-pushed the add-product-wrapper-class branch from 474be3d to 4f3b617 Compare May 23, 2024 16:29

lhoffbeck closed this May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update main product sibling sections when switching between combined listing children #3368

Update main product sibling sections when switching between combined listing children #3368

lhoffbeck commented Mar 25, 2024 •

edited

lhoffbeck Mar 26, 2024

tauthomas01 Mar 27, 2024

lhoffbeck Mar 26, 2024

lhoffbeck Mar 26, 2024

MiniLuke Mar 26, 2024

lhoffbeck Mar 26, 2024

MiniLuke left a comment

MiniLuke Mar 26, 2024

MiniLuke Mar 26, 2024

lhoffbeck Mar 26, 2024

tauthomas01 Mar 27, 2024

lhoffbeck Mar 27, 2024

lhoffbeck commented May 23, 2024

Update main product sibling sections when switching between combined listing children #3368

Update main product sibling sections when switching between combined listing children #3368

Conversation

lhoffbeck commented Mar 25, 2024 • edited

PR Summary:

What approach did you take?

Other considerations

Visual impact on existing themes

Demo links

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiniLuke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhoffbeck commented May 23, 2024

lhoffbeck commented Mar 25, 2024 •

edited