Skip to content
Open
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
21 changes: 20 additions & 1 deletion src/visualBuilder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import {
import { generateStartEditingButton } from "./generators/generateStartEditingButton";

import { addFocusOverlay } from "./generators/generateOverlay";
import { getEntryIdentifiersInCurrentPage } from "./utils/getEntryIdentifiersInCurrentPage";
import {
getEntryIdentifiersInCurrentPage,
getEntryIdentifiersSignature,
} from "./utils/getEntryIdentifiersInCurrentPage";
import { resolvePageContext } from "./utils/resolvePageContext";
import visualBuilderPostMessage from "./utils/visualBuilderPostMessage";
import { VisualBuilderPostMessageEvents } from "./utils/types/postMessage.types";
Expand Down Expand Up @@ -206,6 +209,20 @@ export class VisualBuilder {
});
});

private lastEntriesSignature = "";

/** Tell the editor which entries are on the page, only when the set changed. */
private notifyEntriesInPageIfChanged = (): void => {
const { entriesInCurrentPage } = getEntryIdentifiersInCurrentPage();
const signature = getEntryIdentifiersSignature(entriesInCurrentPage);
if (signature === this.lastEntriesSignature) return;
this.lastEntriesSignature = signature;
visualBuilderPostMessage?.send(
VisualBuilderPostMessageEvents.ENTRIES_IN_PAGE_CHANGED,
{ entriesInCurrentPage }
);
};

private mutationObserver = new MutationObserver(
debounce(
async () => {
Expand All @@ -215,6 +232,7 @@ export class VisualBuilder {
this.visualBuilderContainer,
this.resizeObserver
);
this.notifyEntriesInPageIfChanged();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer is { childList: true, subtree: true }, so attribute-only changes never reach notifyEntriesInPageIfChanged.

useRecalculateVariantDataCSLPValues rewrites data-cslp in place on a variant switch. Same nodes, different entry uids, no mutation record, no push. The editor would keep the old entries' channels and miss the new ones.

Either add attributes: true, attributeFilter: ["data-cslp"] to the observe call, or call notifyEntriesInPageIfChanged() at the end of the recalculation path.


const emptyBlockParents = Array.from(
document.querySelectorAll(`.${VB_EmptyBlockParentClass}`)
Expand Down Expand Up @@ -378,6 +396,7 @@ export class VisualBuilder {
VisualBuilderPostMessageEvents.GET_ALL_ENTRIES_IN_CURRENT_PAGE,
getEntryIdentifiersInCurrentPage
);
this.notifyEntriesInPageIfChanged();
visualBuilderPostMessage?.send(
VisualBuilderPostMessageEvents.SEND_VARIANT_AND_LOCALE
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { JSDOM } from "jsdom";
import { getEntryIdentifiersInCurrentPage } from "../getEntryIdentifiersInCurrentPage";
import {
getEntryIdentifiersInCurrentPage,
getEntryIdentifiersSignature,
} from "../getEntryIdentifiersInCurrentPage";

const dom = new JSDOM(`
<div>
Expand All @@ -23,8 +26,8 @@ const domWithNoCslp = new JSDOM(`
`);

describe("getEntryIdentifiersInCurrentPage", () => {
test('should return an empty array if no elements with data-cslp attribute are found', () => {
document.body.innerHTML = '';
test("should return an empty array if no elements with data-cslp attribute are found", () => {
document.body.innerHTML = "";
const result = getEntryIdentifiersInCurrentPage();
expect(result.entriesInCurrentPage).toEqual([]);
});
Expand Down Expand Up @@ -58,6 +61,38 @@ describe("getEntryIdentifiersInCurrentPage", () => {
`;
const { entriesInCurrentPage } = getEntryIdentifiersInCurrentPage();
expect(entriesInCurrentPage.length).toBe(1);
expect(entriesInCurrentPage[0].entryUid).toBe('bltf5bb5f8fb088a332');
expect(entriesInCurrentPage[0].entryUid).toBe("bltf5bb5f8fb088a332");
});
});

describe("getEntryIdentifiersInCurrentPage locale handling", () => {
test("should keep the same entry in two locales as two results", () => {
document.body.innerHTML = `
<h1 data-cslp="page.blt1.en-us.title">EN</h1>
<h1 data-cslp="page.blt1.fr-fr.title">FR</h1>
`;
const { entriesInCurrentPage } = getEntryIdentifiersInCurrentPage();
expect(entriesInCurrentPage).toEqual([
{ entryUid: "blt1", contentTypeUid: "page", locale: "en-us" },
{ entryUid: "blt1", contentTypeUid: "page", locale: "fr-fr" },
]);
});
});

describe("getEntryIdentifiersSignature", () => {
test("should be order independent and change with the set", () => {
const a = { entryUid: "blt1", contentTypeUid: "page", locale: "en-us" };
const b = {
entryUid: "blt2",
contentTypeUid: "header",
locale: "en-us",
};
expect(getEntryIdentifiersSignature([a, b])).toBe(
getEntryIdentifiersSignature([b, a])
);
expect(getEntryIdentifiersSignature([a])).not.toBe(
getEntryIdentifiersSignature([a, b])
);
expect(getEntryIdentifiersSignature([])).toBe("");
});
});
48 changes: 30 additions & 18 deletions src/visualBuilder/utils/getEntryIdentifiersInCurrentPage.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,46 @@
import { extractDetailsFromCslp, isValidCslp } from "../../cslp/cslpdata";

type EntryIdentifiers = {
entriesInCurrentPage: {
entryUid: string;
contentTypeUid: string;
locale: string;
}[];
}
export type EntryIdentifier = {
entryUid: string;
contentTypeUid: string;
locale: string;
};

export type EntryIdentifiers = {
entriesInCurrentPage: EntryIdentifier[];
};

/**
* Distinct entries rendered on the page, read from `data-cslp`. The same entry
* in two locales is two results: the editor keeps one channel per entry+locale.
*/
export function getEntryIdentifiersInCurrentPage(): EntryIdentifiers {
const elementsWithCslp = Array.from(
document.querySelectorAll("[data-cslp]")
);
const uniqueEntriesMap = new Map<string, { entryUid: string, contentTypeUid: string, locale: string}>();
const uniqueEntriesMap = new Map<string, EntryIdentifier>();
elementsWithCslp.forEach((element) => {
const cslpValue = element.getAttribute("data-cslp");
if (!isValidCslp(cslpValue)) return;
const cslpData = extractDetailsFromCslp(cslpValue);
uniqueEntriesMap.set(cslpData.entry_uid,
{
entryUid: cslpData.entry_uid,
contentTypeUid: cslpData.content_type_uid,
locale: cslpData.locale
}
);
uniqueEntriesMap.set(`${cslpData.entry_uid}.${cslpData.locale}`, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dedup key moved from entry_uid to entry_uid.locale, which changes what the existing get-entries-in-current-page response returns, not just the new event. On a page rendering one entry in two locales, every current caller now gets that entry twice.

Correct for channel subscriptions, but the callers were written against the old shape. Worth a check on EntryAccordionList and ContentPublishModal before this ships, since a duplicate in the publish list is the expensive kind of surprise. If they are not locale-aware, the signature could key on entry+locale while the response stays deduped by uid.

entryUid: cslpData.entry_uid,
contentTypeUid: cslpData.content_type_uid,
locale: cslpData.locale,
});
});

const uniqueEntriesArray = Array.from(uniqueEntriesMap.values());

return {
entriesInCurrentPage: uniqueEntriesArray,
entriesInCurrentPage: Array.from(uniqueEntriesMap.values()),
};
}

/** Order-independent fingerprint of the entry set, used to notify only on change. */
export function getEntryIdentifiersSignature(
entries: EntryIdentifier[]
): string {
return entries
.map((entry) => `${entry.entryUid}.${entry.locale}`)
.sort()
.join("|");
}
1 change: 1 addition & 0 deletions src/visualBuilder/utils/types/postMessage.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export enum VisualBuilderPostMessageEvents {

// FROM visual builder
GET_ALL_ENTRIES_IN_CURRENT_PAGE = "get-entries-in-current-page",
ENTRIES_IN_PAGE_CHANGED = "entries-in-page-changed",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this entries-in-current-page-changed?

get- is the request/response prefix everywhere in this enum, and each of those is a .send() that awaits a value. Push notifications use the unprefixed past-tense form (composition-saved, primary-composition-detected). Matching get-entries-in-current-page on the noun also makes the pair obvious when reading the enum.

Worth settling now: this string is the wire contract on an SDK that customers pin, so renaming after release means supporting both forever.

HIDE_FOCUS_OVERLAY = "hide-focus-overlay",
SHOW_DRAFT_FIELDS = "show-draft-fields",
REMOVE_DRAFT_FIELDS = "remove-draft-fields",
Expand Down
Loading