Add product puppeteer class and simplify component update behavior

[lhoffbeck]

⚠️ This PR builds on the work to support combined listings and 2k variants #3378

tl;dr - this PR primarily copies code out of the variant picker class in global.js into product-info.js, which has been promoted to act as a wrapper for Product.

Why are these changes introduced?

This PR came out of a conversation with @tyleralsbury earlier this year while working on adding support for combined listings / 2k variants. In that PR, the VariantSelects class needs to swap out the page content for its product when the selected variant is associated with a different Combined Listing child product. VariantSelects can be embedded in a few different product contexts (main product, featured product, quick add modal), and I was running into an issue where the class needed some pretty janky logic to determine its wrapping context (here and here). For a more stable architecture, child components shouldn't care what context they are rendered in.

An additional problem is that if components need to be updated when an event in a sibling happens, there wasn't a great "namespaced" context to manage the update. We had two primary patterns for handling this:

Directly update sibling content (link)

class VariantSelects extends HTMLElement {
  
  onVariantChange(event) {
    ...
    this.updatePickupAvailability();
    ...
  }

  updatePickupAvailability() {
    const pickUpAvailability = document.querySelector('pickup-availability');
    if (!pickUpAvailability) return;

    if (this.currentVariant && this.currentVariant.available) {
      pickUpAvailability.fetchAvailability(this.currentVariant.id);
    } else {
      pickUpAvailability.removeAttribute('available');
      pickUpAvailability.innerHTML = '';
    }
  }
}

Publish an event with the section, sibling subscription triggers update (link)

class VariantSelects extends HTMLElement {
    ...
    publish(PUB_SUB_EVENTS.variantChange, {
      data: {
        sectionId,
        html,
        variant: this.currentVariant,
      },
    });
  ...
}

class ProductInfo extends HTMLElement {
  connectedCallback() {
      this.variantChangeUnsubscriber = subscribe(PUB_SUB_EVENTS.variantChange, (event) => {
      const sectionId = this.dataset.originalSection ? this.dataset.originalSection : this.dataset.section;

      // check whether the event came from this ProductInfo's section
      if (event.data.sectionId !== sectionId) return;
      this.updateQuantityRules(event.data.sectionId, event.data.html);
      this.setQuantityBoundries();
    });
  }
}

What approach did you take?

We have 2 primary issues: child components need to be context aware, and handling product component updates was complicated and inconsistent.

To solve both of these problems, I introduced a product class that wraps the entire rendered product and serves as an event handling puppeteer.

We already had a product wrapper class called ProductInfo that wrapped part of the rendered product; I moved it higher up the tree and extracted component update logic into it.

Other considerations

I left a comment, but we possibly could decompose further by moving the old ProductInfo content into a QuantityForm class.

Visual impact on existing themes

None

Testing steps/scenarios

Here is a zip file for these theme changes: https://github.com/Shopify/dawn/archive/refs/heads/add-product-wrapper-class.zip

• Option value switching still works as expected
  • Switching between option values
  • Switching to an option value that has no associated variant
  • Switching between combined listings children
• Product recommendations load when switching between combined listings children
• Quick order list is updated when switching between combined listings children
• Quick add modal
  • works as expected for a normal product
  • works as expected for a combined listing product
• Quantity and volume price rules. (A catalog is set up on the shop, feel free to add yourself as a user or reach out to me for a test account)

Demo links

• https://combined-listings-test.myshopify.com/products/chinos-orange

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

[thagxt]

This proposal really catches my attention. This PR could be the perfect opportunity to make a clear cut decision: use data attributes or IDs for JavaScript targeting and reserve classes solely for styling. I believe this would be a huge help for developers leveraging Dawn as their starter theme.

Making it standard practice to target elements in JavaScript using only IDs or data attributes could seriously streamline things. Right now, it's a bit of a jumble—sometimes you're working with classes, other times with elements, data attributes, or IDs, and it can get pretty confusing. Especially if you're trying to use Dawn as a base theme and want to tidy it up without losing any JavaScript functionality, it's hard to know which class is essential for the JS to work.

[lhoffbeck]

This is the primary change of this PR: extracting product-update logic out of VariantSelects to a wrapper class.

Instead of this class mutating siblings, it now only updates children and publishes an event that indicates a variant change started. The new ProductInfo parent then mutates its children. This separation of logic hopefully makes it easier to understand what controls updates to a component (i.e. itself or a parent vs itself or one of its siblings).

This also enables us to eliminate some gnarly selectors from here and here.

[lhoffbeck]

I migrated this functionality from the VariantSelects class, where it was called in the variant change event

[lhoffbeck]

This class was used for quantity input event handlers, and was rendered further down the product template tree. I repurposed it to be a general Product wrapper class and migrated the onVariantChange handler and (related functions) out of the VariantSelects class.

⚠️ I think there's an argument to be made that the quantity input events shouldn't be in this class at all, and should be their own custom element instead. I did a spike of what that could look like and welcome your feedback on whether that's worth doing now :)

[lhoffbeck]

As a stylistic note, I made functions in this class private if they don't seem like they'd be useful to access from outside of this class.

[lhoffbeck]

Update -- to maintain consistency with the rest of Dawn, I rolled back the private class function change and everything is now public.

[lhoffbeck]

currentVariant can be grabbed from the product form class, submitButton wasn't used anywhere.

[lhoffbeck]

We don't update_url here anymore because this logic lives on the product

[lhoffbeck]

All of this is now handled by the product class

[MiniLuke]

I like this approach. I think the tophatting I did on the combined listing/2k variant PR covers this one.

[lhoffbeck]

From here through #setQuantityBoundries was migrated from VariantSelects with minimal changes.

[lhoffbeck]

I originally added this code so that the component would self-refresh when swapping between combined listing siblings.

This is no longer necessary, since we now replace the entire content of <main> when swapping between siblings.

[lhoffbeck]

I originally added this code so that the component would self-refresh when swapping between combined listing siblings.

This is no longer necessary, since we now replace the entire content of <main> when swapping between siblings.

[lhoffbeck]

toggleSubmitButton logic was pulled out of the VariantSelects class

[lhoffbeck]

Changes in this file are because of changing how main-product section is wrapped in a product-info component

[lhoffbeck]

Worth flagging, this node was changed from a section to a product-info

[ludoboludo]

while I like the idea of clearly mentioning which methods are private and won't be needed outside of the class logic, we're only introducing it here and it creates inconsistency with the rest of the JS.

Not a big deal but just having multiple take/approach in the same codebase adds to its confusion when reading through it I think.

[ludoboludo]

This seem good 👌
Just one suggested nitpick, and I guess there is a conflict to fix.