From c0c5b23004164292878b28b99badd39156feec56 Mon Sep 17 00:00:00 2001 From: Pavel Denisjuk Date: Fri, 8 Mar 2024 14:03:56 +0100 Subject: [PATCH] fix(app-page-builder): optimize Image element renderer (#3984) --- .../src/components/Elements.tsx | 12 +++-- .../src/renderers/image.tsx | 39 +++++++++------- .../ElementControlHorizontalDropZones.tsx | 2 +- .../editor/plugins/elements/image/PeImage.tsx | 46 ++++++------------- 4 files changed, 47 insertions(+), 52 deletions(-) diff --git a/packages/app-page-builder-elements/src/components/Elements.tsx b/packages/app-page-builder-elements/src/components/Elements.tsx index ffa3f5f3d27..87498f5caf9 100644 --- a/packages/app-page-builder-elements/src/components/Elements.tsx +++ b/packages/app-page-builder-elements/src/components/Elements.tsx @@ -13,10 +13,15 @@ const getElementKey = ( elementIndex: number, parentBlockElement?: ElementType ) => { + let parts: string[] = [element.id]; + if (parentBlockElement) { - return `${parentBlockElement.id}-${elementIndex}`; + parts = [parentBlockElement.id, elementIndex.toString()]; } - return element.id; + // Add element type for easier debugging and more clarity in the profiler. + parts.push(element.type); + + return parts.join("-"); }; export interface ElementsProps { @@ -68,8 +73,7 @@ export const Elements = (props: ElementsProps) => { parentDocumentElement, isFirstElement: index === 0, isLastElement: index === elements.length - 1, - elementIndex: index, - collection: elements + elementIndex: index }} /> ); diff --git a/packages/app-page-builder-elements/src/renderers/image.tsx b/packages/app-page-builder-elements/src/renderers/image.tsx index ffcaf2ab1f0..849e4606b0f 100644 --- a/packages/app-page-builder-elements/src/renderers/image.tsx +++ b/packages/app-page-builder-elements/src/renderers/image.tsx @@ -24,6 +24,12 @@ export interface ImageRendererComponentProps extends Props, CreateImageParams {} const SUPPORTED_IMAGE_RESIZE_WIDTHS = [100, 300, 500, 750, 1000, 1500, 2500]; +const PbImg = styled.img` + max-width: 100%; + width: ${props => props.width}; + height: ${props => props.height}; +`; + export const ImageRendererComponent = ({ onClick, renderEmpty, @@ -32,31 +38,21 @@ export const ImageRendererComponent = ({ linkComponent }: ImageRendererComponentProps) => { const LinkComponent = linkComponent || DefaultLinkComponent; - const { getElement } = useRenderer(); - const element = getElement(); let content; if (element.data?.image?.file?.src) { - // Image has its width / height set from its own settings. - const PbImg = styled.img({ - width: element.data.image.width, - height: element.data.image.height, - maxWidth: "100%" - }); - - const { title } = element.data.image; - const { src } = value || element.data?.image?.file; + const { title, width, height, file } = element.data.image; + const { src } = value || file; // If a fixed image width in pixels was set, let's filter out unneeded // image resize widths. For example, if 155px was set as the fixed image // width, then we want the `srcset` attribute to only contain 100w and 300w. let srcSetWidths: number[] = []; - const imageWidth = element.data.image.width; - if (imageWidth && imageWidth.endsWith("px")) { - const imageWidthInt = parseInt(imageWidth); + if (width && width.endsWith("px")) { + const imageWidthInt = parseInt(width); for (let i = 0; i < SUPPORTED_IMAGE_RESIZE_WIDTHS.length; i++) { const supportedResizeWidth = SUPPORTED_IMAGE_RESIZE_WIDTHS[i]; if (imageWidthInt > supportedResizeWidth) { @@ -78,7 +74,18 @@ export const ImageRendererComponent = ({ }) .join(", "); - content = ; + content = ( + + ); } else { content = renderEmpty || null; } @@ -108,7 +115,7 @@ export const imageRendererOptions: CreateRendererOptions = { export type ImageRenderer = ReturnType; interface Props { - onClick?: React.MouseEventHandler; + onClick?: () => void; renderEmpty?: React.ReactNode; value?: { id: string; src: string }; link?: { href: string; newTab?: boolean }; diff --git a/packages/app-page-builder/src/editor/contexts/EditorPageElementsProvider/ElementControlHorizontalDropZones.tsx b/packages/app-page-builder/src/editor/contexts/EditorPageElementsProvider/ElementControlHorizontalDropZones.tsx index c0bc5e1e128..405cff5f941 100644 --- a/packages/app-page-builder/src/editor/contexts/EditorPageElementsProvider/ElementControlHorizontalDropZones.tsx +++ b/packages/app-page-builder/src/editor/contexts/EditorPageElementsProvider/ElementControlHorizontalDropZones.tsx @@ -129,7 +129,7 @@ export const ElementControlHorizontalDropZones = () => { {meta.isLastElement && ( true} - onDrop={source => dropElementAction(source, meta.collection?.length)} + onDrop={source => dropElementAction(source, meta.elementIndex + 1)} type={type} > {({ drop, isOver }) => ( diff --git a/packages/app-page-builder/src/editor/plugins/elements/image/PeImage.tsx b/packages/app-page-builder/src/editor/plugins/elements/image/PeImage.tsx index e10d0162e1d..454676cd0b9 100644 --- a/packages/app-page-builder/src/editor/plugins/elements/image/PeImage.tsx +++ b/packages/app-page-builder/src/editor/plugins/elements/image/PeImage.tsx @@ -9,7 +9,6 @@ import { ReactComponent as AddImageIcon } from "@webiny/ui/ImageUpload/icons/rou import { Typography } from "@webiny/ui/Typography"; import { useElementVariableValue } from "~/editor/hooks/useElementVariableValue"; import { createRenderer, useRenderer } from "@webiny/app-page-builder-elements"; -import { useActiveElementId } from "~/editor/hooks/useActiveElementId"; const RenderBlank = (props: { onClick?: () => void }) => { return ( @@ -22,14 +21,12 @@ const RenderBlank = (props: { onClick?: () => void }) => { ); }; +const emptyLink = { href: "" }; + const PeImage = createRenderer(() => { const { getElement } = useRenderer(); const element = getElement(); const variableValue = useElementVariableValue(element); - - const [activeElementId] = useActiveElementId(); - const isActive = activeElementId === element.id; - const handler = useEventActionHandler(); const id = element?.id; @@ -55,33 +52,20 @@ const PeImage = createRenderer(() => { [id] ); - if (isActive) { - return ( - ( - showFileManager()} - renderEmpty={} - value={variableValue} - // Even if the link might've been applied via the right sidebar, we still don't - // want to have it rendered in the editor. Because, otherwise, user wouldn't be - // able to click again on the component and bring back the file manager overlay. - link={{ href: "" }} - /> - )} - /> - ); - } - return ( - } - value={variableValue} - // Even if the link might've been applied via the right sidebar, we still don't - // want to have it rendered in the editor. Because, otherwise, user wouldn't be - // able to click again on the component and bring back the file manager overlay. - link={{ href: "" }} + ( + } + value={variableValue} + // Even if the link might've been applied via the right sidebar, we still don't + // want to have it rendered in the editor. Because, otherwise, user wouldn't be + // able to click again on the component and bring back the file manager overlay. + link={emptyLink} + /> + )} /> ); });