fix(discussions): emit CSLP_TAGS_UPDATED after variant switch so comment icons mount at correct positions

When the visual editor switches variants it sends GET_VARIANT_ID to the
iframe SDK.  The user's CSR app then re-renders with the new variant data,
changing every data-cslp attribute value (e.g. from
content_type.uid.locale.field to v2:content_type.uid_variantId.locale.field).
Previously the visual editor had no way of knowing when those attributes
had settled, so it would re-mount comment icons against stale CSLP values —
icons either missed their elements entirely or were positioned using
getBoundingClientRect() on the pre-variant DOM and then failed to track
scroll because updateHighlightedCommentIconPosition could no longer find
the element by the now-stale field-path attribute.

Changes
───────

src/visualBuilder/utils/types/postMessage.types.ts
  • Add CSLP_TAGS_UPDATED = "cslp-tags-updated" to the post-message enum.
    This is the signal the SDK sends back to the visual editor once the
    iframe's data-cslp attributes have settled after a variant switch.

src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts
  • updateVariantClasses now accepts an optional onSettled callback.
  • Inside the existing MutationObserver (which already watches each
    [data-cslp] element for attribute changes), scheduleSettled() is
    called on every relevant mutation.  After a 200 ms debounce with no
    further mutations, onSettled fires — meaning the burst of re-render
    updates has finished.
  • A 1500 ms fallback timer fires onSettled regardless, so entries that
    have no variant field overrides (zero mutations) still unblock the
    visual editor.
  • hasSettled flag ensures onSettled is called at most once per
    updateVariantClasses invocation, even if both the debounce and the
    fallback race.

src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts
  • GET_VARIANT_ID handler (CSR path): passes an onSettled callback to
    updateVariantClasses that sends CSLP_TAGS_UPDATED once the DOM settles.
  • GET_VARIANT_ID handler (SSR path): sends CSLP_TAGS_UPDATED immediately
    after addVariantFieldClass because SSR classes are applied synchronously
    on the existing DOM — no re-render delay exists.
  • Removed accidental console.log left from debugging.

src/visualBuilder/generators/generateHighlightedComment.tsx
  • updateHighlightedCommentIconPosition: when a scroll/resize position
    update is attempted for an icon whose [data-cslp="${path}"] element
    is no longer found, the icon is removed from the DOM instead of being
    silently skipped.  Without this, an icon mounted just before the
    variant re-render would drift to the wrong fixed position and stop
    tracking scroll permanently.  The visual editor will re-mount it
    correctly once CSLP_TAGS_UPDATED arrives.
  • Removed accidental console.log left from debugging.

src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts
  • Four new tests in the SSR-handling describe block covering:
    1. CSLP_TAGS_UPDATED sent immediately for SSR + variant provided.
    2. CSLP_TAGS_UPDATED sent immediately for SSR + null variant.
    3. updateVariantClasses called with an onSettled callback (CSR).
    4. CSLP_TAGS_UPDATED sent when the onSettled callback fires (CSR).

src/visualBuilder/eventManager/__test__/useRecalculateVariantDataCSLPValues.spec.ts (new)
  • Four tests for the onSettled callback logic:
    1. Fires via the 1500 ms fallback when no mutations occur.
    2. Does not fire before 1499 ms.
    3. Fires exactly once even if the fallback would fire again.
    4. Does not throw when no callback is provided.

src/visualBuilder/generators/__test__/generateHighlightedComment.test.ts (new)
  • Five tests for updateHighlightedCommentIconPosition and
    removeAllHighlightedCommentIcons covering: position update when target
    exists, orphan removal when target is gone, mixed-case handling, empty
    DOM safety, and full bulk removal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: karancs06

src/visualBuilder/eventManager/__test__/useRecalculateVariantDataCSLPValues.spec.ts

@@ -0,0 +1,85 @@
+import { vi, describe, it, expect, beforeEach, afterEach } from "vitest";
+
+vi.mock("../../../visualBuilder", () => ({
+    VisualBuilder: {
+        VisualBuilderGlobalState: {
+            value: {
+                highlightVariantFields: false,
+                variant: null,
+            },
+        },
+    },
+}));
+
+vi.mock("../../../visualBuilder/visualBuilder.style", () => ({
+    visualBuilderStyles: () => ({
+        "visual-builder__variant-field-outline": "vb-variant-outline",
+    }),
+}));
+
+vi.mock("../../../cslp/cslpdata", () => ({
+    isValidCslp: vi.fn((v: string | null) => !!v),
+}));
+
+// useVariantsPostMessageEvent imports setHighlightVariantFields from here;
+// provide a no-op so the import resolves without side effects.
+vi.mock("../../../visualBuilder/eventManager/useVariantsPostMessageEvent", () => ({
+    setHighlightVariantFields: vi.fn(),
+}));
+
+import { updateVariantClasses } from "../useRecalculateVariantDataCSLPValues";
+
+describe("updateVariantClasses — onSettled callback", () => {
+    beforeEach(() => {
+        vi.useFakeTimers();
+    });
+
+    afterEach(() => {
+        vi.useRealTimers();
+    });
+
+    it("fires onSettled via the fallback timeout when no mutations occur", () => {
+        const onSettled = vi.fn();
+
+        updateVariantClasses(onSettled);
+
+        expect(onSettled).not.toHaveBeenCalled();
+
+        // Advance past the 1500 ms fallback (CSLP_SETTLED_FALLBACK_MS)
+        vi.advanceTimersByTime(1500);
+
+        expect(onSettled).toHaveBeenCalledTimes(1);
+    });
+
+    it("does not call onSettled before the fallback timeout elapses", () => {
+        const onSettled = vi.fn();
+
+        updateVariantClasses(onSettled);
+
+        vi.advanceTimersByTime(1499);
+        expect(onSettled).not.toHaveBeenCalled();
+    });
+
+    it("calls onSettled only once even if the fallback fires after a mutation already settled it", () => {
+        const onSettled = vi.fn();
+
+        updateVariantClasses(onSettled);
+
+        // Simulate a mutation settling: fire the debounce (200 ms) manually
+        // by advancing timers just past CSLP_SETTLED_DEBOUNCE_MS.
+        // In a real DOM environment the MutationObserver would call scheduleSettled;
+        // here we rely on the fallback path which exercises the same idempotency
+        // guard (hasSettled flag).
+        vi.advanceTimersByTime(1500); // first call via fallback
+        vi.advanceTimersByTime(1500); // would be a second call if guard is missing
+
+        expect(onSettled).toHaveBeenCalledTimes(1);
+    });
+
+    it("works without an onSettled callback (no-op when undefined)", () => {
+        expect(() => {
+            updateVariantClasses(); // no callback
+            vi.advanceTimersByTime(1500);
+        }).not.toThrow();
+    });
+});

src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts

@@ -669,4 +669,74 @@ describe("useVariantFieldsPostMessageEvent SSR handling", () => {
         expect(mockQuerySelectorAll).not.toHaveBeenCalled();
         expect(updateVariantClasses).not.toHaveBeenCalled();
     });
+
+    it("sends CSLP_TAGS_UPDATED immediately when isSSR is true and variant is provided", () => {
+        useVariantFieldsPostMessageEvent({ isSSR: true });
+
+        const call = mockVisualBuilderPostMessage.on.mock.calls.find(
+            (call: any[]) =>
+                call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID
+        );
+        const handler = call![1];
+
+        vi.clearAllMocks();
+        handler({ data: { variant: "variant-123" } });
+
+        expect(mockVisualBuilderPostMessage.send).toHaveBeenCalledWith(
+            VisualBuilderPostMessageEvents.CSLP_TAGS_UPDATED
+        );
+    });
+
+    it("sends CSLP_TAGS_UPDATED immediately when isSSR is true and variant is null", () => {
+        useVariantFieldsPostMessageEvent({ isSSR: true });
+
+        const call = mockVisualBuilderPostMessage.on.mock.calls.find(
+            (call: any[]) =>
+                call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID
+        );
+        const handler = call![1];
+
+        vi.clearAllMocks();
+        handler({ data: { variant: null } });
+
+        expect(mockVisualBuilderPostMessage.send).toHaveBeenCalledWith(
+            VisualBuilderPostMessageEvents.CSLP_TAGS_UPDATED
+        );
+    });
+
+    it("passes an onSettled callback to updateVariantClasses when isSSR is false", () => {
+        useVariantFieldsPostMessageEvent({ isSSR: false });
+
+        const call = mockVisualBuilderPostMessage.on.mock.calls.find(
+            (call: any[]) =>
+                call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID
+        );
+        const handler = call![1];
+
+        vi.clearAllMocks();
+        handler({ data: { variant: "variant-123" } });
+
+        expect(updateVariantClasses).toHaveBeenCalledWith(expect.any(Function));
+    });
+
+    it("sends CSLP_TAGS_UPDATED when the onSettled callback from updateVariantClasses is invoked (CSR)", () => {
+        useVariantFieldsPostMessageEvent({ isSSR: false });
+
+        const call = mockVisualBuilderPostMessage.on.mock.calls.find(
+            (call: any[]) =>
+                call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID
+        );
+        const handler = call![1];
+
+        vi.clearAllMocks();
+        handler({ data: { variant: "variant-123" } });
+
+        // Retrieve the onSettled callback passed to updateVariantClasses and invoke it
+        const onSettled = (updateVariantClasses as vi.Mock).mock.calls[0][0] as () => void;
+        onSettled();
+
+        expect(mockVisualBuilderPostMessage.send).toHaveBeenCalledWith(
+            VisualBuilderPostMessageEvents.CSLP_TAGS_UPDATED
+        );
+    });
 });

src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts

@@ -7,6 +7,11 @@ import { isValidCslp } from "../../cslp/cslpdata";
 import { setHighlightVariantFields } from "./useVariantsPostMessageEvent";
 
 const VARIANT_UPDATE_DELAY_MS: Readonly<number> = 8000;
+// How long to wait after the last observed mutation before considering the DOM settled.
+const CSLP_SETTLED_DEBOUNCE_MS: Readonly<number> = 200;
+// Fallback: if no mutations are observed at all (e.g. no variant overrides on this entry),
+// treat the DOM as settled after this delay so the visual editor is never left waiting.
+const CSLP_SETTLED_FALLBACK_MS: Readonly<number> = 1500;
 
 type OnAudienceModeVariantPatchUpdate = {
     highlightVariantFields: boolean;
@@ -27,11 +32,35 @@ export function useRecalculateVariantDataCSLPValues(): void {
         }
     );
 }
-export function updateVariantClasses(): void {
+export function updateVariantClasses(onSettled?: () => void): void {
     const highlightVariantFields = VisualBuilder.VisualBuilderGlobalState.value.highlightVariantFields;
     const variant = VisualBuilder.VisualBuilderGlobalState.value.variant;
     const observers: MutationObserver[] = [];
 
+    // --- settled notification logic ---
+    let settledTimer: ReturnType<typeof setTimeout> | null = null;
+    let hasSettled = false;
+
+    const fireSettled = () => {
+        if (hasSettled) return;
+        hasSettled = true;
+        if (settledTimer !== null) {
+            clearTimeout(settledTimer);
+            settledTimer = null;
+        }
+        onSettled?.();
+    };
+
+    const scheduleSettled = () => {
+        if (hasSettled) return;
+        if (settledTimer !== null) clearTimeout(settledTimer);
+        settledTimer = setTimeout(fireSettled, CSLP_SETTLED_DEBOUNCE_MS);
+    };
+
+    // Fallback: fire even if no mutations were observed (e.g. no variant overrides)
+    setTimeout(fireSettled, CSLP_SETTLED_FALLBACK_MS);
+    // ----------------------------------
+
     // Helper function to update element classes
     const updateElementClasses = (
         element: HTMLElement,
@@ -156,6 +185,7 @@ export function updateVariantClasses(): void {
                         DATA_CSLP_ATTR_SELECTOR
                     );
                     updateElementClasses(element, dataCslp || "", observer);
+                    scheduleSettled();
                 }
             });
         });

src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts

@@ -40,13 +40,13 @@ interface LocaleEvent {
 }
 
 function isLowerOrderVariant(variant_uid: string, dataCslp: string, variantOrder: string[]): boolean {
-    if(!variantOrder || variantOrder.length === 0) {
+    if (!variantOrder || variantOrder.length === 0) {
         return false;
     }
-    const {variant: cslpVariant} = extractDetailsFromCslp(dataCslp);
+    const { variant: cslpVariant } = extractDetailsFromCslp(dataCslp);
     const indexOfCmsVariant = variantOrder.lastIndexOf(variant_uid);
     const indexOfCslpVariant = variantOrder.lastIndexOf(cslpVariant || "");
-    if(indexOfCslpVariant < 0) {
+    if (indexOfCslpVariant < 0) {
         return false;
     }
     return indexOfCslpVariant < indexOfCmsVariant;
@@ -72,7 +72,7 @@ export function addVariantFieldClass(
             }
         } else if (!dataCslp.startsWith("v2:")) {
             element.classList.add("visual-builder__base-field");
-        } 
+        }
         else if (isLowerOrderVariant(variant_uid, dataCslp, variantOrder)) {
             element.classList.add("visual-builder__variant-field", "visual-builder__lower-order-variant-field");
         }
@@ -102,7 +102,7 @@ export function removeVariantFieldClass(
         });
     } else {
         const variantAndBaseFieldElements = document.querySelectorAll(
-            ".visual-builder__disabled-variant-field, .visual-builder__variant-field, .visual-builder__base-field, .visual-builder__lower-order-variant-field" 
+            ".visual-builder__disabled-variant-field, .visual-builder__variant-field, .visual-builder__base-field, .visual-builder__lower-order-variant-field"
         );
         variantAndBaseFieldElements.forEach((element) => {
             element.classList.remove(
@@ -139,7 +139,7 @@ export async function getHighlightVariantFieldsStatus(): Promise<GetHighlightVar
     try {
         const result = await visualBuilderPostMessage?.send<GetHighlightVariantFieldsStatusResponse>(
             VisualBuilderPostMessageEvents.GET_HIGHLIGHT_VARIANT_FIELDS_STATUS
-        );   
+        );
         return result ?? {
             highlightVariantFields: false,
         };
@@ -167,9 +167,22 @@ export function useVariantFieldsPostMessageEvent({ isSSR }: { isSSR: boolean }):
                 if (selectedVariant) {
                     addVariantFieldClass(selectedVariant);
                 }
+                // For SSR the classes are applied synchronously, so the DOM is
+                // already in its final state — notify the visual editor immediately.
+                visualBuilderPostMessage?.send(
+                    VisualBuilderPostMessageEvents.CSLP_TAGS_UPDATED
+                );
             } else {
-                // recalculate and apply classes
-                updateVariantClasses();
+                // For CSR apps the user's framework re-renders asynchronously after
+                // receiving the new variant, mutating data-cslp attributes on the
+                // way.  updateVariantClasses watches for those mutations; once they
+                // settle (or after a fallback timeout) it fires onSettled, which
+                // tells the visual editor it is safe to re-send comment highlights.
+                updateVariantClasses(() => {
+                    visualBuilderPostMessage?.send(
+                        VisualBuilderPostMessageEvents.CSLP_TAGS_UPDATED
+                    );
+                });
             }
         }
     );

src/visualBuilder/generators/__test__/generateHighlightedComment.test.ts

@@ -0,0 +1,108 @@
+import { vi, describe, it, expect, beforeEach, afterEach } from "vitest";
+import {
+    updateHighlightedCommentIconPosition,
+    removeAllHighlightedCommentIcons,
+} from "../generateHighlightedComment";
+
+/**
+ * Tests for generateHighlightedComment.ts
+ *
+ * Covers the orphaned-icon cleanup introduced to fix the scroll-drift bug that
+ * occurred after a variant switch changed data-cslp attribute values.
+ */
+describe("updateHighlightedCommentIconPosition", () => {
+    let container: HTMLDivElement;
+
+    beforeEach(() => {
+        container = document.createElement("div");
+        container.className = "visual-builder__container";
+        document.body.appendChild(container);
+    });
+
+    afterEach(() => {
+        document.body.removeChild(container);
+    });
+
+    const makeIcon = (fieldPath: string): HTMLDivElement => {
+        const icon = document.createElement("div");
+        icon.className = "highlighted-comment collab-icon";
+        icon.setAttribute("field-path", fieldPath);
+        icon.style.position = "fixed";
+        icon.style.top = "100px";
+        icon.style.left = "200px";
+        container.appendChild(icon);
+        return icon;
+    };
+
+    const makeTarget = (cslpValue: string): HTMLDivElement => {
+        const el = document.createElement("div");
+        el.setAttribute("data-cslp", cslpValue);
+        container.appendChild(el);
+        return el;
+    };
+
+    it("updates position of an icon whose target element is still present", () => {
+        const cslp = "content_type.entry.en-us.field";
+        makeTarget(cslp);
+        const icon = makeIcon(cslp);
+
+        // jsdom returns zero for getBoundingClientRect by default; just verify
+        // the update runs without error and the icon remains in the DOM.
+        updateHighlightedCommentIconPosition();
+
+        expect(document.querySelector(`.highlighted-comment[field-path="${cslp}"]`)).not.toBeNull();
+    });
+
+    it("removes an orphaned icon when its target element is no longer found", () => {
+        const staleCslp = "content_type.entry.en-us.old_field";
+        // Do NOT add a matching [data-cslp] element — simulates the element
+        // having been updated to a variant CSLP value after a re-render.
+        makeIcon(staleCslp);
+
+        updateHighlightedCommentIconPosition();
+
+        // Icon must be removed from the DOM
+        expect(
+            document.querySelector(`.highlighted-comment[field-path="${staleCslp}"]`)
+        ).toBeNull();
+    });
+
+    it("removes only orphaned icons and keeps icons whose targets still exist", () => {
+        const validCslp = "content_type.entry.en-us.valid_field";
+        const staleCslp = "content_type.entry.en-us.stale_field";
+
+        makeTarget(validCslp);
+        makeIcon(validCslp);
+        makeIcon(staleCslp); // no matching element
+
+        updateHighlightedCommentIconPosition();
+
+        expect(
+            document.querySelector(`.highlighted-comment[field-path="${validCslp}"]`)
+        ).not.toBeNull();
+        expect(
+            document.querySelector(`.highlighted-comment[field-path="${staleCslp}"]`)
+        ).toBeNull();
+    });
+
+    it("does not throw when there are no highlighted-comment icons", () => {
+        expect(() => updateHighlightedCommentIconPosition()).not.toThrow();
+    });
+});
+
+describe("removeAllHighlightedCommentIcons", () => {
+    it("removes every .highlighted-comment element from the DOM", () => {
+        ["cslp-1", "cslp-2", "cslp-3"].forEach((cslp) => {
+            const icon = document.createElement("div");
+            icon.className = "highlighted-comment collab-icon";
+            icon.setAttribute("field-path", cslp);
+            document.body.appendChild(icon);
+        });
+
+        expect(document.querySelectorAll(".highlighted-comment").length).toBe(3);
+
+        removeAllHighlightedCommentIcons();
+
+        expect(document.querySelectorAll(".highlighted-comment").length).toBe(0);
+    });
+});