Skip to content
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

Combined listings with multiple product templates #3526

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions assets/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,4 @@ const PUB_SUB_EVENTS = {
optionValueSelectionChange: 'option-value-selection-change',
variantChange: 'variant-change',
cartError: 'cart-error',
sectionRefreshed: 'section-refreshed',
};

const SECTION_REFRESH_RESOURCE_TYPE = {
product: 'product',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was introduced as part of combined listings but is not being used anywhere. Removing to prevent locking ourselves into supporting this event (i.e. if apps start consuming it).

};
41 changes: 14 additions & 27 deletions assets/product-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,6 @@ if (!customElements.get('product-info')) {
this.postProcessHtmlCallbacks.push((newNode) => {
window?.Shopify?.PaymentButton?.init();
window?.ProductModel?.loadShopifyXR();
publish(PUB_SUB_EVENTS.sectionRefreshed, {
data: {
sectionId: this.sectionId,
resource: {
type: SECTION_REFRESH_RESOURCE_TYPE.product,
id: newNode.dataset.productId,
},
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above

});
}

Expand All @@ -77,12 +68,14 @@ if (!customElements.get('product-info')) {
const productUrl = target.dataset.productUrl || this.pendingRequestUrl || this.dataset.url;
this.pendingRequestUrl = productUrl;
const shouldSwapProduct = this.dataset.url !== productUrl;
const shouldFetchFullPage = !this.isFeaturedProduct && shouldSwapProduct;
const shouldFetchFullPage = this.dataset.updateUrl === 'true' && shouldSwapProduct;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Prior to this change, we were requesting a full PDP from both main-product and quick add modal, and just requesting the section for featured product. However, for the quick add modal we only need the main product section.

This change makes it so that we request the full page when swapping between combined listings in a main product section, and request just the product section for both featured product and quick add modal.


this.renderProductInfo({
requestUrl: this.buildRequestUrlWithParams(productUrl, selectedOptionValues, shouldFetchFullPage),
targetId: target.id,
callback: shouldSwapProduct ? this.handleSwapProduct(productUrl) : this.handleUpdateProductInfo(productUrl),
callback: shouldSwapProduct
? this.handleSwapProduct(productUrl, shouldFetchFullPage)
: this.handleUpdateProductInfo(productUrl),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explicitly pass whether we're fetching the full page to the handleSwapProduct callback

});
}

Expand All @@ -92,33 +85,27 @@ if (!customElements.get('product-info')) {
productForm?.handleErrorMessage();
}

get isFeaturedProduct() {
return this.dataset.section.includes('featured_product');
}

handleSwapProduct(productUrl) {
handleSwapProduct(productUrl, updateFullPage) {
return (html) => {
this.productModal?.remove();

// Grab the selected variant from the new product info
const variant = this.getSelectedVariant(html.querySelector(`product-info[data-section=${this.sectionId}]`));
const selector = updateFullPage ? "product-info[id^='MainProduct']" : 'product-info';
const variant = this.getSelectedVariant(html.querySelector(selector));
dan-menard marked this conversation as resolved.
Show resolved Hide resolved
this.updateURL(productUrl, variant?.id);

// If we are in an embedded context (quick add, featured product, etc), only swap product info.
// Otherwise, refresh the entire page content and sibling sections.
if (this.dataset.updateUrl === 'false') {
if (updateFullPage) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reversed this conditional so that we can rely on the updateFullPage variable instead of re-querying the dom

document.querySelector('head title').innerHTML = html.querySelector('head title').innerHTML;

HTMLUpdateUtility.viewTransition(
this,
html.querySelector('product-info'),
document.querySelector('main'),
html.querySelector('main'),
this.preProcessHtmlCallbacks,
this.postProcessHtmlCallbacks
);
} else {
document.querySelector('head title').innerHTML = html.querySelector('head title').innerHTML;

HTMLUpdateUtility.viewTransition(
document.querySelector('main'),
html.querySelector('main'),
this,
html.querySelector('product-info'),
this.preProcessHtmlCallbacks,
this.postProcessHtmlCallbacks
);
Expand Down
Loading