diff --git a/builder/builder/doctype/builder_component/builder_component.py b/builder/builder/doctype/builder_component/builder_component.py index 6429f0749..fc208e8f0 100644 --- a/builder/builder/doctype/builder_component/builder_component.py +++ b/builder/builder/doctype/builder_component/builder_component.py @@ -38,6 +38,9 @@ def before_insert(self): self.component_id = frappe.generate_hash(length=16) capture("builder_component_created", "builder") + def validate(self): + validate_component_slots(frappe.parse_json(self.block or "{}")) + def on_update(self): # Skip the background cache-clear and version snapshot during bulk imports # (install / migrate / import_doc). queue_action enqueues a job AND locks the @@ -93,6 +96,51 @@ def update_exported_component(self): ) +def validate_component_slots(block: dict) -> None: + slot_names: set[str] = set() + + def visit( + node: dict, + inside_slot: bool = False, + inside_repeater: bool = False, + is_root: bool = False, + ) -> None: + if not is_root and node.get("extendedFromComponent"): + return + raw_slot_name = node.get("slotName") + if raw_slot_name is not None and ( + not isinstance(raw_slot_name, str) + or not raw_slot_name.strip() + or raw_slot_name != raw_slot_name.strip() + ): + frappe.throw(frappe._("Component slot names must be non-empty and trimmed")) + slot_name = raw_slot_name or "" + is_slot = bool(slot_name) + if is_slot: + if node.get("element") not in ("div", "section"): + frappe.throw(frappe._("Component slots must be container blocks")) + if inside_slot: + frappe.throw(frappe._("Component slots cannot be nested")) + if inside_repeater or node.get("isRepeaterBlock"): + frappe.throw(frappe._("Component slots cannot be placed inside collections")) + if slot_name in slot_names: + frappe.throw( + frappe._("Component slot names must be unique. Duplicate slot: {0}").format(slot_name) + ) + slot_names.add(slot_name) + node["slotName"] = slot_name + + for child in node.get("children") or []: + visit( + child, + inside_slot=inside_slot or is_slot, + inside_repeater=inside_repeater or bool(node.get("isRepeaterBlock")), + ) + + if isinstance(block, dict): + visit(block, is_root=True) + + class ComponentSyncer: def __init__(self, page_doc) -> None: self.page_doc = page_doc @@ -117,24 +165,84 @@ def sync_blocks(self, blocks: str | list[Block], component) -> str: continue if block.extendedFromComponent == component.component_id: component_block = Block(**frappe.parse_json(component.block)) - self.sync_single_block(block, component.name, component_block.children or []) + self.sync_single_block(block, component.name, component_block) else: self.sync_blocks(block.children or [], component) blocks_dict = [block.as_dict() if isinstance(block, Block) else block for block in blocks_list] return compact_json(blocks_dict) - def sync_single_block(self, target_block: Block, component_name: str, component_children: list[Block]): + def sync_single_block( + self, + target_block: Block, + component_name: str, + component_block: Block, + filled_slots: dict[str, Block] | None = None, + ): """Sync a single block with its component template""" + if filled_slots is None: + filled_slots = self.collect_filled_slots(target_block) + target_block.slotName = component_block.slotName + root_slot = filled_slots.get(component_block.slotName) if component_block.slotName else None + if root_slot: + target_block.slotFilled = True + target_block.children = copy.deepcopy(root_slot.children or []) + return + target_block.slotFilled = False + if component_block.slotName: + target_block.children = self.create_detached_slot_content(component_block.children or []) + target_block.slotFilled = bool(target_block.children) + return target_children = target_block.children or [] target_block.children = [] - for index, component_child in enumerate(component_children): + for index, component_child in enumerate(component_block.children or []): block_component = self.find_component_block(component_child.blockId, target_children) - if block_component: - self.sync_single_block(block_component, component_name, component_child.children or []) - else: + if not block_component: block_component = self.create_component_block(component_child, component_name) + block_component.slotName = component_child.slotName + filled_slot = filled_slots.get(component_child.slotName) if component_child.slotName else None + if filled_slot: + block_component.slotFilled = True + block_component.children = copy.deepcopy(filled_slot.children or []) + else: + block_component.slotFilled = False + self.sync_single_block(block_component, component_name, component_child, filled_slots) target_block.children.insert(index, block_component) + @staticmethod + def collect_filled_slots(root: Block) -> dict[str, Block]: + slots: dict[str, Block] = {} + + def visit(block: Block, is_root: bool = False) -> None: + if not is_root and block.extendedFromComponent: + return + if block.slotName and block.slotFilled: + slots[block.slotName] = block + return + for child in block.children or []: + visit(child) + + visit(root, True) + return slots + + @staticmethod + def create_detached_slot_content(component_children: list[Block]) -> list[Block]: + children = copy.deepcopy(component_children) + + def detach(block: Block, inside_nested_component: bool = False) -> None: + block.blockId = frappe.generate_hash(length=8) + if not inside_nested_component: + block.isChildOfComponent = None + block.referenceBlockId = None + if not block.extendedFromComponent: + block.componentVersion = None + inside_nested_component = inside_nested_component or bool(block.extendedFromComponent) + for child in block.children or []: + detach(child, inside_nested_component) + + for child in children: + detach(child) + return children + @staticmethod def parse_blocks(blocks: str) -> list[Block]: """Parse blocks JSON into Block objects""" diff --git a/builder/builder/doctype/builder_component/test_builder_component.py b/builder/builder/doctype/builder_component/test_builder_component.py index 8ab83653d..0222c12f5 100644 --- a/builder/builder/doctype/builder_component/test_builder_component.py +++ b/builder/builder/doctype/builder_component/test_builder_component.py @@ -1,9 +1,138 @@ # Copyright (c) 2023, asdf and Contributors # See license.txt -# import frappe +import copy + +import frappe from frappe.tests.utils import FrappeTestCase +from builder.builder.doctype.builder_component.builder_component import ( + ComponentSyncer, + validate_component_slots, +) +from builder.builder.doctype.builder_page.builder_page import extend_block +from builder.utils import Block + class TestBuilderComponent(FrappeTestCase): - pass + def test_component_slot_names_must_be_unique(self): + block = { + "element": "div", + "children": [ + {"element": "div", "slotName": "content"}, + {"element": "section", "slotName": "content"}, + ], + } + + with self.assertRaises(frappe.ValidationError): + validate_component_slots(block) + + def test_component_slots_cannot_be_nested(self): + block = { + "element": "div", + "slotName": "outer", + "children": [{"element": "div", "slotName": "inner"}], + } + + with self.assertRaises(frappe.ValidationError): + validate_component_slots(block) + + def test_filled_slot_replaces_fallback_when_rendering(self): + component_slot = { + "element": "div", + "slotName": "content", + "children": [{"element": "p", "innerHTML": "Fallback"}], + } + instance_slot = { + "referenceBlockId": "slot", + "slotName": "content", + "slotFilled": True, + "children": [{"element": "h2", "innerHTML": "Custom"}], + } + + result = extend_block(copy.deepcopy(component_slot), instance_slot) + + self.assertEqual(result["children"], instance_slot["children"]) + self.assertNotIn("Fallback", frappe.as_json(result)) + + def test_empty_filled_slot_does_not_render_fallback(self): + component_slot = { + "element": "div", + "slotName": "content", + "children": [{"element": "p", "innerHTML": "Fallback"}], + } + + result = extend_block( + copy.deepcopy(component_slot), + {"slotName": "content", "slotFilled": True, "children": []}, + ) + + self.assertEqual(result["children"], []) + + def test_sync_preserves_filled_slot_by_name_after_move(self): + custom = Block(element="h2", innerHTML="Custom") + old_slot = Block( + blockId="instance-slot", + referenceBlockId="old-slot", + slotName="content", + slotFilled=True, + isChildOfComponent="component", + children=[custom], + ) + target = Block( + extendedFromComponent="component", + children=[Block(referenceBlockId="old-wrapper", children=[old_slot])], + ) + new_component = Block( + blockId="component-root", + element="div", + children=[ + Block( + blockId="new-wrapper", + element="section", + children=[ + Block( + blockId="new-slot", + element="div", + slotName="content", + children=[Block(blockId="fallback", element="p", innerHTML="Fallback")], + ) + ], + ) + ], + ) + + ComponentSyncer(None).sync_single_block(target, "component", new_component) + + synced_slot = target.children[0].children[0] + self.assertEqual(synced_slot.slotName, "content") + self.assertTrue(synced_slot.slotFilled) + self.assertEqual(synced_slot.children[0].innerHTML, "Custom") + + def test_sync_materializes_slot_defaults_as_detached_content(self): + target = Block( + extendedFromComponent="component", + slotName="content", + children=[], + ) + component_slot = Block( + blockId="component-slot", + element="div", + slotName="content", + children=[ + Block( + blockId="fallback", + element="p", + innerHTML="Fallback", + isChildOfComponent="component", + referenceBlockId="fallback", + ) + ], + ) + + ComponentSyncer(None).sync_single_block(target, "component", component_slot) + + self.assertTrue(target.slotFilled) + self.assertEqual(target.children[0].innerHTML, "Fallback") + self.assertIsNone(target.children[0].isChildOfComponent) + self.assertIsNone(target.children[0].referenceBlockId) diff --git a/builder/builder/doctype/builder_page/builder_page.py b/builder/builder/doctype/builder_page/builder_page.py index 708770d08..7c2531bb4 100644 --- a/builder/builder/doctype/builder_page/builder_page.py +++ b/builder/builder/doctype/builder_page/builder_page.py @@ -1489,6 +1489,10 @@ def extend_block(block, overridden_block): block["innerHTML"] = overridden_block["innerHTML"] component_children = block.get("children", []) or [] overridden_children = overridden_block.get("children", []) or [] + if block.get("slotName") and overridden_block.get("slotFilled"): + block["children"] = copy.deepcopy(overridden_children) + block["slotFilled"] = True + return block extended_children = [] for overridden_child in overridden_children: component_child = next( @@ -1553,6 +1557,10 @@ def resolve_path(path): def reset_with_component(block, extended_with_component, component_children): reset_block(block) block["children"] = [] + if block.get("slotName"): + block["children"] = create_detached_slot_content(component_children) + block["slotFilled"] = bool(block["children"]) + return for component_child in component_children: component_child_id = component_child.get("blockId") child_block = reset_block(component_child) @@ -1574,6 +1582,27 @@ def reset_with_component(block, extended_with_component, component_children): reset_with_component(child_block, extended_with_component, child_block.get("children")) +def create_detached_slot_content(component_children): + children = copy.deepcopy(component_children or []) + + def detach(block, inside_nested_component=False): + block["blockId"] = frappe.generate_hash(length=8) + if not inside_nested_component: + block.pop("isChildOfComponent", None) + block.pop("referenceBlockId", None) + if not block.get("extendedFromComponent"): + block.pop("componentVersion", None) + inside_nested_component = inside_nested_component or bool( + block.get("extendedFromComponent") + ) + for child in block.get("children") or []: + detach(child, inside_nested_component) + + for child in children: + detach(child) + return children + + def reset_block(block): block["blockId"] = frappe.generate_hash(length=8) block["innerHTML"] = None @@ -1588,6 +1617,8 @@ def reset_block(block): block["dataKey"] = {} block["props"] = {} block["dynamicValues"] = [] + if block.get("slotName"): + block["slotFilled"] = False return block diff --git a/builder/utils.py b/builder/utils.py index 19dabfe20..cf546621d 100644 --- a/builder/utils.py +++ b/builder/utils.py @@ -102,6 +102,8 @@ class Block: originalElement: str | None = None isChildOfComponent: str | None = None referenceBlockId: str | None = None + slotName: str | None = None + slotFilled: bool = False isRepeaterBlock: bool = False visibilityCondition: str | VisibilityCondition | None = None elementBeforeConversion: str | None = None @@ -167,6 +169,8 @@ def as_dict(self): "originalElement": self.originalElement, "isChildOfComponent": self.isChildOfComponent, "referenceBlockId": self.referenceBlockId, + "slotName": self.slotName, + "slotFilled": self.slotFilled, "isRepeaterBlock": self.isRepeaterBlock, "visibilityCondition": self.visibilityCondition, "elementBeforeConversion": self.elementBeforeConversion, diff --git a/frontend/src/block.ts b/frontend/src/block.ts index 7f42b8d79..e0287998b 100644 --- a/frontend/src/block.ts +++ b/frontend/src/block.ts @@ -71,6 +71,8 @@ class Block implements BlockOptions { originalElement?: string | undefined; isChildOfComponent?: string; referenceBlockId?: string; + slotName?: string; + slotFilled?: boolean; isRepeaterBlock?: boolean; visibilityCondition?: BlockVisibilityCondition; elementBeforeConversion?: string; @@ -91,6 +93,8 @@ class Block implements BlockOptions { this.isRepeaterBlock = options.isRepeaterBlock; this.isChildOfComponent = options.isChildOfComponent; this.referenceBlockId = options.referenceBlockId; + this.slotName = options.slotName; + this.slotFilled = options.slotFilled; this.parentBlock = options.parentBlock || null; if (this.extendedFromComponent) { if (this.componentVersion) { @@ -128,7 +132,7 @@ class Block implements BlockOptions { // falling back to the live component const componentBlock = this.componentVersion ? componentStore.getComponentVersionBlock(this.componentVersion as string) || - componentStore.getComponentBlock(this.isChildOfComponent as string) + componentStore.getComponentBlock(this.isChildOfComponent as string) : componentStore.getComponentBlock(this.isChildOfComponent as string); return findBlockInTree(this.referenceBlockId as string, [componentBlock]); } @@ -314,6 +318,9 @@ class Block implements BlockOptions { return visibilityCondition; } getBlockDescription() { + if (this.slotName) { + return this.slotName; + } if (this.extendedFromComponent) { return this.getComponentBlockDescription() || ""; } @@ -562,6 +569,9 @@ class Block implements BlockOptions { } } addChild(child: BlockOptions, index?: number | null, select: boolean = true) { + if (select) { + this.prepareSlotForContent(); + } if (index === undefined || index === null) { index = this.children.length; } @@ -588,6 +598,7 @@ class Block implements BlockOptions { removeChild(child: Block) { const index = this.getChildIndex(child); if (index > -1) { + this.markSlotContentChanged(); this.children.splice(index, 1); } } @@ -681,7 +692,7 @@ class Block implements BlockOptions { this.isVideo() || (this.isText() && !this.isLink()) || this.isHTML() || - this.isExtendedFromComponent() + (this.isExtendedFromComponent() && !this.isInstanceSlot()) ); } updateStyles(styles: BlockStyleObjects) { @@ -784,6 +795,7 @@ class Block implements BlockOptions { moveChild(child: Block, index: number) { const childIndex = this.children.findIndex((block) => block.blockId === child.blockId); if (childIndex > -1) { + this.markSlotContentChanged(); this.children.splice(childIndex, 1); this.children.splice(index, 0, child); } @@ -854,6 +866,41 @@ class Block implements BlockOptions { isChildOfComponentBlock() { return Boolean(this.isChildOfComponent); } + isSlot() { + return Boolean(this.slotName?.trim()); + } + isInstanceSlot() { + return this.isSlot() && this.isExtendedFromComponent() && useCanvasStore().editingMode === "page"; + } + prepareSlotForContent() { + if (!this.isInstanceSlot() || this.slotFilled) return; + this.children.splice(0, this.children.length); + this.slotFilled = true; + } + markSlotContentChanged() { + if (this.isInstanceSlot()) { + this.slotFilled = true; + } + } + isDetachedSlotContent() { + if (this.isChildOfComponentBlock()) return false; + let parent = this.getParentBlock(); + while (parent) { + if (parent.isInstanceSlot()) return true; + if (parent.extendedFromComponent) return false; + parent = parent.getParentBlock(); + } + return false; + } + resetSlotContent() { + if (!this.isInstanceSlot()) return; + const componentId = this.extendedFromComponent || this.isChildOfComponent; + const componentSlot = this.referenceComponent; + if (!componentId || !componentSlot) return; + this.children.splice(0, this.children.length); + this.slotFilled = false; + populateSlotFallback(this, componentSlot.children); + } resetWithComponent() { const component = this.referenceComponent; if (component) { @@ -953,13 +1000,14 @@ class Block implements BlockOptions { return this.isFlex() && this.getStyle("flexDirection") === "column"; } duplicateBlock() { - if (this.isRoot()) { + if (this.isRoot() || this.isChildOfComponentBlock()) { return; } const canvasStore = useCanvasStore(); const pauseId = canvasStore.activeCanvas?.history?.pause(); const blockCopy = getBlockCopy(this); const parentBlock = this.getParentBlock(); + parentBlock?.markSlotContentChanged(); if (blockCopy.getStyle("position") === "absolute") { // shift the block a bit diff --git a/frontend/src/builder.d.ts b/frontend/src/builder.d.ts index c4e44f524..ff54ded84 100644 --- a/frontend/src/builder.d.ts +++ b/frontend/src/builder.d.ts @@ -55,6 +55,8 @@ declare interface BlockOptions { draggable?: boolean; editorConfig?: BlockEditorConfig; componentVersion?: string; + slotName?: string; + slotFilled?: boolean; [key: string]: any; } diff --git a/frontend/src/components/BlockContextMenu.vue b/frontend/src/components/BlockContextMenu.vue index 6b476e875..c19ac9db6 100644 --- a/frontend/src/components/BlockContextMenu.vue +++ b/frontend/src/components/BlockContextMenu.vue @@ -109,6 +109,7 @@ const contextMenuOptions: ContextMenuOption[] = [ { label: "Duplicate", action: duplicateBlock, + condition: () => !block.value.isChildOfComponentBlock(), disabled: () => builderStore.readOnlyMode, }, { @@ -142,6 +143,9 @@ const contextMenuOptions: ContextMenuOption[] = [ if (!parentBlock) return; const selectedBlocks = canvasStore.activeCanvas?.selectedBlocks || []; + if (selectedBlocks.every((selectedBlock: Block) => selectedBlock.getParentBlock() === parentBlock)) { + parentBlock.markSlotContentChanged(); + } const blockPosition = Math.min(...selectedBlocks.map(parentBlock.getChildIndex.bind(parentBlock))); const newBlock = parentBlock?.addChild(newBlockObj, blockPosition); @@ -171,13 +175,15 @@ const contextMenuOptions: ContextMenuOption[] = [ }); }, condition: () => { - if (block.value.isRoot()) return false; + if (block.value.isRoot() || block.value.isChildOfComponentBlock()) return false; if (canvasStore.activeCanvas?.selectedBlocks.length === 1) return true; // check if all selected blocks are siblings const parentBlock = block.value.getParentBlock(); if (!parentBlock) return false; const selectedBlocks = canvasStore.activeCanvas?.selectedBlocks || []; - return selectedBlocks.every((block: Block) => block.getParentBlock() === parentBlock); + return selectedBlocks.every( + (block: Block) => !block.isChildOfComponentBlock() && block.getParentBlock() === parentBlock, + ); }, disabled: () => builderStore.readOnlyMode, }, @@ -241,6 +247,16 @@ const contextMenuOptions: ContextMenuOption[] = [ }, disabled: () => builderStore.readOnlyMode, }, + { + label: "Reset Slot to Fallback", + condition: () => block.value.isInstanceSlot() && Boolean(block.value.slotFilled), + action: () => { + confirm("Discard this slot's custom content and restore its fallback?").then((confirmed) => { + if (confirmed) block.value.resetSlotContent(); + }); + }, + disabled: () => builderStore.readOnlyMode, + }, { label: "Update to Latest Component", action: () => { diff --git a/frontend/src/components/BlockLayers.vue b/frontend/src/components/BlockLayers.vue index d48e4c00c..348469384 100644 --- a/frontend/src/components/BlockLayers.vue +++ b/frontend/src/components/BlockLayers.vue @@ -56,22 +56,21 @@ @click="toggleExpanded(element)" />