diff --git a/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts b/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts index 78b25bb3..2558c4e7 100644 --- a/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts +++ b/src/visualBuilder/eventManager/__test__/useVariantsPostMessageEvent.spec.ts @@ -669,4 +669,62 @@ describe("useVariantFieldsPostMessageEvent SSR handling", () => { expect(mockQuerySelectorAll).not.toHaveBeenCalled(); expect(updateVariantClasses).not.toHaveBeenCalled(); }); + + it("sends REQUEST_DISCUSSION_HIGHLIGHTS 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 ? call[1] : null; + + vi.clearAllMocks(); + handler!({ data: { variant: "variant-123" } }); + + // SSR DOM is already in its final state — the observer would never + // fire, so we must ask the visual editor for a fresh highlight list. + expect(mockVisualBuilderPostMessage.send).toHaveBeenCalledWith( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + }); + + it("sends REQUEST_DISCUSSION_HIGHLIGHTS immediately when isSSR is true even if variant is null", () => { + useVariantFieldsPostMessageEvent({ isSSR: true }); + + const call = mockVisualBuilderPostMessage.on.mock.calls.find( + (call: any[]) => + call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID + ); + const handler = call ? call[1] : null; + + vi.clearAllMocks(); + handler!({ data: { variant: null } }); + + // Switching back to base is still a CSLP-relevant change — VB needs to + // re-compute and re-send highlights against the new (base) CSLPs. + expect(mockVisualBuilderPostMessage.send).toHaveBeenCalledWith( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + }); + + it("does not send REQUEST_DISCUSSION_HIGHLIGHTS directly when isSSR is false (observer handles it)", () => { + useVariantFieldsPostMessageEvent({ isSSR: false }); + + const call = mockVisualBuilderPostMessage.on.mock.calls.find( + (call: any[]) => + call[0] === VisualBuilderPostMessageEvents.GET_VARIANT_ID + ); + const handler = call ? call[1] : null; + + vi.clearAllMocks(); + handler!({ data: { variant: "variant-123" } }); + + // For CSR apps the event is emitted by the MutationObserver inside + // updateVariantClasses, not synchronously from the handler. + expect(mockVisualBuilderPostMessage.send).not.toHaveBeenCalledWith( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + expect(updateVariantClasses).toHaveBeenCalled(); + }); }); diff --git a/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts b/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts index 7c4f1ba2..180696be 100644 --- a/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts +++ b/src/visualBuilder/eventManager/useRecalculateVariantDataCSLPValues.ts @@ -5,8 +5,14 @@ import { DATA_CSLP_ATTR_SELECTOR } from "../utils/constants"; import { visualBuilderStyles } from "../visualBuilder.style"; import { isValidCslp } from "../../cslp/cslpdata"; import { setHighlightVariantFields } from "./useVariantsPostMessageEvent"; +import visualBuilderPostMessage from "../utils/visualBuilderPostMessage"; +import { VisualBuilderPostMessageEvents } from "../utils/types/postMessage.types"; const VARIANT_UPDATE_DELAY_MS: Readonly = 8000; +// Debounce window after the last observed mutation before asking the visual +// editor to re-send discussion highlights. Short enough to feel responsive; +// long enough to coalesce a burst of mutations into a single request. +const DISCUSSION_HIGHLIGHTS_DEBOUNCE_MS: Readonly = 200; type OnAudienceModeVariantPatchUpdate = { highlightVariantFields: boolean; @@ -32,6 +38,21 @@ export function updateVariantClasses(): void { const variant = VisualBuilder.VisualBuilderGlobalState.value.variant; const observers: MutationObserver[] = []; + // Ask the visual editor to re-send discussion highlights once the current + // burst of data-cslp mutations has settled. The VB owns the field-path list + // (it can be per-variant) so the iframe cannot re-mount comment icons on + // its own — it can only request a refresh. + let highlightsRequestTimer: ReturnType | null = null; + const requestDiscussionHighlights = () => { + if (highlightsRequestTimer !== null) clearTimeout(highlightsRequestTimer); + highlightsRequestTimer = setTimeout(() => { + highlightsRequestTimer = null; + visualBuilderPostMessage?.send( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); + }, DISCUSSION_HIGHLIGHTS_DEBOUNCE_MS); + }; + // Helper function to update element classes const updateElementClasses = ( element: HTMLElement, @@ -156,6 +177,7 @@ export function updateVariantClasses(): void { DATA_CSLP_ATTR_SELECTOR ); updateElementClasses(element, dataCslp || "", observer); + requestDiscussionHighlights(); } }); }); diff --git a/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts b/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts index ba02c539..d21c1068 100644 --- a/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts +++ b/src/visualBuilder/eventManager/useVariantsPostMessageEvent.ts @@ -167,8 +167,18 @@ export function useVariantFieldsPostMessageEvent({ isSSR }: { isSSR: boolean }): if (selectedVariant) { addVariantFieldClass(selectedVariant); } + // SSR DOM is already in its final state (no async re-render) — + // the MutationObserver in updateVariantClasses will never fire, + // so ask the visual editor to re-send discussion highlights now + // that the classes are applied against the final CSLP values. + visualBuilderPostMessage?.send( + VisualBuilderPostMessageEvents.REQUEST_DISCUSSION_HIGHLIGHTS + ); } else { - // recalculate and apply classes + // For CSR apps the framework re-renders asynchronously after + // receiving the new variant, mutating data-cslp attributes. + // updateVariantClasses installs a MutationObserver that emits + // REQUEST_DISCUSSION_HIGHLIGHTS once those mutations settle. updateVariantClasses(); } } diff --git a/src/visualBuilder/generators/__test__/generateHighlightedComment.test.ts b/src/visualBuilder/generators/__test__/generateHighlightedComment.test.ts new file mode 100644 index 00000000..e83529b0 --- /dev/null +++ b/src/visualBuilder/generators/__test__/generateHighlightedComment.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { + updateHighlightedCommentIconPosition, + removeAllHighlightedCommentIcons, +} from "../generateHighlightedComment"; + +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("keeps an icon whose target element is still present", () => { + const cslp = "content.entry.en-us.field"; + makeTarget(cslp); + makeIcon(cslp); + + updateHighlightedCommentIconPosition(); + + expect( + document.querySelector(`.highlighted-comment[field-path="${cslp}"]`), + ).not.toBeNull(); + }); + + it("removes an orphaned icon when its target element is no longer found", () => { + // Stale CSLP with no matching element — simulates the element having + // been mutated to a variant CSLP value. + const staleCslp = "content.entry.en-us.old_field"; + makeIcon(staleCslp); + + updateHighlightedCommentIconPosition(); + + expect( + document.querySelector(`.highlighted-comment[field-path="${staleCslp}"]`), + ).toBeNull(); + }); + + it("removes only orphaned icons and keeps valid ones", () => { + const validCslp = "content.entry.en-us.valid"; + const staleCslp = "content.entry.en-us.stale"; + + makeTarget(validCslp); + makeIcon(validCslp); + makeIcon(staleCslp); + + 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 icons", () => { + expect(() => updateHighlightedCommentIconPosition()).not.toThrow(); + }); +}); + +describe("removeAllHighlightedCommentIcons", () => { + it("removes every .highlighted-comment element", () => { + ["a", "b", "c"].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); + }); +}); diff --git a/src/visualBuilder/generators/generateHighlightedComment.tsx b/src/visualBuilder/generators/generateHighlightedComment.tsx index 7fa8c7c9..ed16c662 100644 --- a/src/visualBuilder/generators/generateHighlightedComment.tsx +++ b/src/visualBuilder/generators/generateHighlightedComment.tsx @@ -95,6 +95,12 @@ export function updateHighlightedCommentIconPosition() { // Update the position of the icon container icon.style.top = `${top - highlighCommentOffset}px`; // Adjust based on the target element's top icon.style.left = `${left - highlighCommentOffset}px`; // Adjust based on the target element's left + } else { + // Target element is gone (e.g. data-cslp mutated after a + // variant switch). Remove the orphaned icon instead of + // silently leaving it drifted — the visual editor will + // re-mount a fresh set via REQUEST_DISCUSSION_HIGHLIGHTS. + icon.remove(); } } } diff --git a/src/visualBuilder/utils/types/postMessage.types.ts b/src/visualBuilder/utils/types/postMessage.types.ts index e26e7df2..a95b207b 100644 --- a/src/visualBuilder/utils/types/postMessage.types.ts +++ b/src/visualBuilder/utils/types/postMessage.types.ts @@ -59,4 +59,5 @@ export enum VisualBuilderPostMessageEvents { COLLAB_THREAD_REOPEN = "collab-thread-reopen", COLLAB_THREAD_HIGHLIGHT = "collab-thread-highlight", TOGGLE_SCROLL = "toggle-scroll", + REQUEST_DISCUSSION_HIGHLIGHTS = "request-discussion-highlights", }