Skip to content

Commit

Permalink
feat: add circularity error message, wrappingPatternIds and handle en…
Browse files Browse the repository at this point in the history
…tryId as rootBlockId
  • Loading branch information
Chaoste committed Jan 22, 2025
1 parent 44f1431 commit 73c82f5
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 13 deletions.
23 changes: 21 additions & 2 deletions packages/visual-editor/src/components/DraggableBlock/Dropzone.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type DropzoneProps = {
className?: string;
WrapperComponent?: ElementType | string;
dragProps?: DragWrapperProps;
wrappingPatternIds?: Set<string>;
};

export function Dropzone({
Expand All @@ -40,6 +41,7 @@ export function Dropzone({
className,
WrapperComponent = 'div',
dragProps,
wrappingPatternIds: parentWrappingPatternIds = new Set(),
...rest
}: DropzoneProps) {
const userIsDragging = useDraggedItemStore((state) => state.isDraggingOnCanvas);
Expand All @@ -66,6 +68,19 @@ export function Dropzone({
const isRootAssembly = node?.type === ASSEMBLY_NODE_TYPE;
const htmlDraggableProps = getHtmlDragProps(dragProps);
const htmlProps = getHtmlComponentProps(rest);

const wrappingPatternIds = useMemo(() => {
// On the top level, the node is not defined. If the root blockId is not the default string,
// we assume that it is the entry ID of the experience/ pattern to properly detect circular dependencies
if (!node && tree.root.data.blockId && tree.root.data.blockId !== ROOT_ID) {
return new Set([tree.root.data.blockId, ...parentWrappingPatternIds]);
}
if (isRootAssembly && node?.data.blockId) {
return new Set([node.data.blockId, ...parentWrappingPatternIds]);
}
return parentWrappingPatternIds;
}, [isRootAssembly, node, parentWrappingPatternIds, tree.root.data.blockId]);

// To avoid a circular dependency, we create the recursive rendering function here and trickle it down
const renderDropzone: RenderDropzoneFunction = useCallback(
(node, props) => {
Expand All @@ -74,11 +89,12 @@ export function Dropzone({
zoneId={node.data.id}
node={node}
resolveDesignValue={resolveDesignValue}
wrappingPatternIds={wrappingPatternIds}
{...props}
/>
);
},
[resolveDesignValue],
[wrappingPatternIds, resolveDesignValue],
);

const renderClonedDropzone: RenderDropzoneFunction = useCallback(
Expand All @@ -89,11 +105,12 @@ export function Dropzone({
node={node}
resolveDesignValue={resolveDesignValue}
renderDropzone={renderClonedDropzone}
wrappingPatternIds={wrappingPatternIds}
{...props}
/>
);
},
[resolveDesignValue],
[resolveDesignValue, wrappingPatternIds],
);

const isDropzoneEnabled = useMemo(() => {
Expand Down Expand Up @@ -152,6 +169,7 @@ export function Dropzone({
provided={provided}
snapshot={snapshot}
renderDropzone={renderClonedDropzone}
wrappingPatternIds={wrappingPatternIds}
/>
)}>
{(provided, snapshot) => {
Expand Down Expand Up @@ -200,6 +218,7 @@ export function Dropzone({
node={item}
resolveDesignValue={resolveDesignValue}
renderDropzone={renderDropzone}
wrappingPatternIds={wrappingPatternIds}
/>
))
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type DropzoneProps = {
WrapperComponent?: ElementType | string;
renderDropzone: RenderDropzoneFunction;
dragProps?: DragWrapperProps;
wrappingPatternIds: Set<string>;
};

export function DropzoneClone({
Expand All @@ -27,6 +28,7 @@ export function DropzoneClone({
WrapperComponent = 'div',
renderDropzone,
dragProps,
wrappingPatternIds,
...rest
}: DropzoneProps) {
const tree = useTreeStore((state) => state.tree);
Expand Down Expand Up @@ -72,6 +74,7 @@ export function DropzoneClone({
node={item}
resolveDesignValue={resolveDesignValue}
renderDropzone={renderDropzone}
wrappingPatternIds={wrappingPatternIds}
/>
);
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type EditorBlockProps = {
resolveDesignValue: ResolveDesignValueType;
renderDropzone: RenderDropzoneFunction;
zoneId: string;
wrappingPatternIds: Set<string>;
};

export const EditorBlock: React.FC<EditorBlockProps> = ({
Expand All @@ -51,6 +52,7 @@ export const EditorBlock: React.FC<EditorBlockProps> = ({
zoneId,
userIsDragging,
placeholder,
wrappingPatternIds,
}) => {
const { slotId } = parseZoneId(zoneId);
const ref = useRef<HTMLElement | null>(null);
Expand All @@ -69,6 +71,7 @@ export const EditorBlock: React.FC<EditorBlockProps> = ({
resolveDesignValue,
renderDropzone,
userIsDragging,
wrappingPatternIds,
});
const { isSingleColumn, isWrapped } = useSingleColumn(node, resolveDesignValue);
const setDomRect = useDraggedItemStore((state) => state.setDomRect);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type EditorBlockCloneProps = {
provided?: DraggableProvided;
snapshot?: DraggableStateSnapshot;
renderDropzone: RenderDropzoneFunction;
wrappingPatternIds: Set<string>;
};

export const EditorBlockClone: React.FC<EditorBlockCloneProps> = ({
Expand All @@ -37,6 +38,7 @@ export const EditorBlockClone: React.FC<EditorBlockCloneProps> = ({
snapshot,
provided,
renderDropzone,
wrappingPatternIds,
}) => {
const userIsDragging = useDraggedItemStore((state) => state.isDraggingOnCanvas);

Expand All @@ -45,6 +47,7 @@ export const EditorBlockClone: React.FC<EditorBlockCloneProps> = ({
resolveDesignValue,
renderDropzone,
userIsDragging,
wrappingPatternIds,
});

const isAssemblyBlock = node.type === ASSEMBLY_BLOCK_NODE_TYPE;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { useEntityStore } from '@/store/entityStore';
import React, { forwardRef, HTMLAttributes } from 'react';

type CircularDependencyErrorPlaceholderProperties = HTMLAttributes<HTMLDivElement> & {
wrappingPatternIds: Set<string>;
};

export const CircularDependencyErrorPlaceholder = forwardRef<
HTMLDivElement,
CircularDependencyErrorPlaceholderProperties
>(({ wrappingPatternIds, ...props }, ref) => {
const entityStore = useEntityStore((state) => state.entityStore);

return (
<div
{...props}
// Pass through ref to avoid DND errors being logged
ref={ref}
data-cf-node-error="circular-pattern-dependency"
style={{
border: '1px solid red',
background: 'rgba(255, 0, 0, 0.1)',
padding: '1rem 1rem 0 1rem',
width: '100%',
height: '100%',
}}>
Circular usage of patterns detected:
<ul>
{Array.from(wrappingPatternIds).map((patternId) => {
const entryLink = { sys: { type: 'Link', linkType: 'Entry', id: patternId } } as const;
const entry = entityStore.getEntityFromLink(entryLink);
const entryTitle = entry?.fields?.title;
const text = entryTitle ? `${entryTitle} (${patternId})` : patternId;
return <li key={patternId}>{text}</li>;
})}
</ul>
</div>
);
});

CircularDependencyErrorPlaceholder.displayName = 'CircularDependencyErrorPlaceholder';
1 change: 1 addition & 0 deletions packages/visual-editor/src/hooks/useComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe('useComponent', () => {
resolveDesignValue,
renderDropzone,
userIsDragging,
wrappingPatternIds: new Set(),
}),
);

Expand Down
37 changes: 26 additions & 11 deletions packages/visual-editor/src/hooks/useComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,22 @@ import { isContentfulStructureComponent } from '@contentful/experiences-core';
import { MissingComponentPlaceholder } from '@components/DraggableHelpers/MissingComponentPlaceholder';
import { useTreeStore } from '@/store/tree';
import { getItem } from '@/utils/getItem';
import { CircularDependencyErrorPlaceholder } from '@components/DraggableHelpers/CircularDependencyErrorPlaceholder';

type UseComponentProps = {
node: ExperienceTreeNode;
resolveDesignValue: ResolveDesignValueType;
renderDropzone: RenderDropzoneFunction;
userIsDragging: boolean;
wrappingPatternIds: Set<string>;
};

export const useComponent = ({
node,
resolveDesignValue,
renderDropzone,
userIsDragging,
wrappingPatternIds,
}: UseComponentProps) => {
const areEntitiesFetched = useEntityStore((state) => state.areEntitiesFetched);
const tree = useTreeStore((state) => state.tree);
Expand Down Expand Up @@ -79,11 +82,32 @@ export const useComponent = ({
});

const elementToRender = (props?: { dragProps?: DragWrapperProps; rest?: unknown }) => {
const { dragProps = {} } = props || {};
const { children, innerRef, Tag = 'div', ToolTipAndPlaceholder, style, ...rest } = dragProps;
const {
'data-cf-node-block-id': dataCfNodeBlockId,
'data-cf-node-block-type': dataCfNodeBlockType,
'data-cf-node-id': dataCfNodeId,
} = componentProps;
const refCallback = (refNode: HTMLElement | null) => {
if (innerRef && refNode) innerRef(refNode);
};

if (!componentRegistration) {
return <MissingComponentPlaceholder blockId={node.data.blockId} />;
}

const { dragProps = {} } = props || {};
if (node.data.blockId && wrappingPatternIds.has(node.data.blockId)) {
return (
<CircularDependencyErrorPlaceholder
ref={refCallback}
data-cf-node-id={dataCfNodeId}
data-cf-node-block-id={dataCfNodeBlockId}
data-cf-node-block-type={dataCfNodeBlockType}
wrappingPatternIds={wrappingPatternIds}
/>
);
}

const element = React.createElement(
ImportedComponentErrorBoundary,
Expand All @@ -98,20 +122,11 @@ export const useComponent = ({
return element;
}

const { children, innerRef, Tag = 'div', ToolTipAndPlaceholder, style, ...rest } = dragProps;
const {
'data-cf-node-block-id': dataCfNodeBlockId,
'data-cf-node-block-type': dataCfNodeBlockType,
'data-cf-node-id': dataCfNodeId,
} = componentProps;

return (
<Tag
{...rest}
style={{ ...style, ...wrapperStyles }}
ref={(refNode: HTMLElement | null) => {
if (innerRef && refNode) innerRef(refNode);
}}
ref={refCallback}
data-cf-node-id={dataCfNodeId}
data-cf-node-block-id={dataCfNodeBlockId}
data-cf-node-block-type={dataCfNodeBlockType}>
Expand Down

0 comments on commit 73c82f5

Please sign in to comment.