Shopify / dawn

Shopify's first source available reference theme, with Online Store 2.0 features and performance built-in.
Other
2.36k stars 3.13k forks source link

Fix variant media not changing in the quick add modal #3451

Closed ludoboludo closed 3 weeks ago

ludoboludo commented 1 month ago

PR Summary:

In the quick add modal, on version 14, when you change variant it doesn't update media shown when it used to.

Why are these changes introduced?

Fixes https://github.com/Shopify/dawn/issues/3448

What approach did you take?

From what I see it's looking a mediaGalleyDestination but it can't grab any cause this.dataset.section can't be found in the new html. (recording)

With this new approach, it seems like the issue comes from the fact that we have preventDuplicatedIDs() to prevent duplicate IDs in the quick add modal by adding quickadd-. So when we do

const mediaGalleryDestination = html.querySelector(`[id^="MediaGallery-${this.dataset.section}"] ul`);

It's not finding that destination as it doesn't have a similar ID. ~We could tweak that data-section and remove the quickadd- within updateMedia(html){...} 🤷~

In the function that prevents the duplicate IDs we're setting a data attribute that keeps the original section ID. So we're using this in priority if it's present and we don't need to do the string manipulation 👍

If I do this instead it seem to fix the issue:

const sectionId = this.dataset.originalSection ? this.dataset.originalSection : this.dataset.section;
...
const mediaGalleryDestination =
      html.querySelector(`[id^="MediaGallery-${sectionId}"] ul`);

Visual impact on existing themes

It now works when changing variant that has a feat. image

Testing steps/scenarios

Demo links

Checklist

jonpennington commented 1 month ago

Could we perhaps use this functionality instead? It would follow similar logic used elsewhere in global.js. The problem seems to originate with how Section Rendering API now works.

const sectionId = this.dataset.originalSection ? this.dataset.originalSection : this.dataset.section;
const mediaGallerySource = document.querySelector(`[id^="MediaGallery-${this.dataset.section}"] ul`);
const mediaGalleryDestination = html.querySelector(`[id^="MediaGallery-${sectionId}"] ul`);