Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix inconsistent pb element save-behavior in editor #3540

Open
wants to merge 7 commits into
base: next
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from "react";
import styled from "@emotion/styled";

const EmptyElementWrapper = styled.div`
display: flex;
align-items: center;
justify-content: center;
height: 100%;
min-height: 100px;
${props => props.theme.styles.typography.paragraphs.stylesById("paragraph1")};
color: ${props => props.theme.styles.colors["color4"]};
`;

type EmptyElementProps = {
message: string;
};

export const EmptyElement = ({ message }: EmptyElementProps) => {
return <EmptyElementWrapper>{message}</EmptyElementWrapper>;
};
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./DefaultLinkComponent";
export * from "./EmptyElement";
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React, { useEffect, useCallback } from "react";
import { Element } from "~/types";
import styled from "@emotion/styled";

import { EmptyElement } from "~/renderers/components";
import { Element } from "~/types";

export interface OEmbedPropsInitCallableParams {
props: OEmbedProps;
node: HTMLElement;
Expand Down Expand Up @@ -74,5 +76,9 @@ export const OEmbed = (props: OEmbedProps) => {
appendSDK(props).then(() => initEmbed(props));
});

return url ? renderer() : null;
if (!url) {
return <EmptyElement message="Please provide a link for this embed element." />;
}

return renderer();
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import { createRenderer } from "~/createRenderer";
import { useRenderer } from "~/hooks/useRenderer";
import { EmptyElement } from "~/renderers/components";

export interface IFrameElementData {
iframe: {
Expand All @@ -13,6 +14,11 @@ export const createIFrame = () =>
const { getElement } = useRenderer();

const element = getElement<IFrameElementData>();
const url = element.data.iframe.url;

return <iframe src={element.data.iframe.url} width="100%" height="100%" />;
if (!url) {
return <EmptyElement message="Please provide a link for this iframe element." />;
}

return <iframe src={url} width="100%" height="100%" />;
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useEffect } from "react";
import { Element, Renderer } from "~/types";
import { createRenderer } from "~/createRenderer";
import { useRenderer } from "~/hooks/useRenderer";
import { EmptyElement } from "~/renderers/components";

declare global {
interface Window {
Expand Down Expand Up @@ -50,7 +51,7 @@ const Pinterest: PinterestComponent = ({ element }) => {
return <a data-pin-do="embedPin" data-pin-width={data.size || "small"} href={data.url} />;
}

return null;
return <EmptyElement message="Please provide a link for this embed element." />;
};

export const createPinterest = () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/app-page-builder-elements/src/renderers/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import styled from "@emotion/styled";
import { createRenderer, CreateRendererOptions } from "~/createRenderer";
import { useRenderer } from "~/hooks/useRenderer";
import { LinkComponent as LinkComponentType } from "~/types";
import { DefaultLinkComponent } from "~/renderers/components";
import { DefaultLinkComponent, EmptyElement } from "~/renderers/components";

declare global {
// eslint-disable-next-line
Expand Down Expand Up @@ -128,7 +128,7 @@ export const ImageRendererComponent = ({
);
}
} else {
content = renderEmpty || null;
content = renderEmpty || <EmptyElement message="Please select an image." />;
}

const linkProps = link || element.data?.link;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React, { useReducer } from "react";
import { ImagesListComponent } from "../types";
import { EmptyElement } from "~/renderers/components";

/**
* Package react-columned does not have types.
*/
// @ts-expect-error
import Columned from "react-columned";
import Lightbox from "react-images";
import styled from "@emotion/styled";

const COLUMNS = { 320: 1, 480: 2, 800: 3, 1366: 4 };

Expand Down Expand Up @@ -57,17 +57,6 @@ const useLightbox = () => {
};
};

const NoImagesToDisplay = styled.div`
display: flex;
justify-content: center;
align-items: center;
height: 100px;
width: 100%;
border: 1px dashed var(--mdc-theme-secondary);
color: var(--mdc-theme-text-secondary-on-background);
pointer-events: none;
`;

export const createDefaultImagesListComponent = (): ImagesListComponent =>
function DefaultImagesListComponent(props) {
const { opened, open, close, next, prev, currentIndex } = useLightbox();
Expand Down Expand Up @@ -99,5 +88,5 @@ export const createDefaultImagesListComponent = (): ImagesListComponent =>
);
}

return <NoImagesToDisplay>No images to display.</NoImagesToDisplay>;
return <EmptyElement message="No images to display." />;
};
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { UpdateDocumentActionEvent } from "~/editor/recoil/actions/updateDocument";
import { CreateElementEventActionArgsType } from "./types";
import { EventActionCallable } from "~/types";

export const createElementAction: EventActionCallable<CreateElementEventActionArgsType> = () => {
return {
actions: []
actions: [new UpdateDocumentActionEvent()]
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a comment why this step please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did some more investigation on this.

Currently, Page Builder does not send update document requests to API when an element is added to a page. However, based on my testing, some elements (such as Heading, Paragraph, Image, Button, Pages List and Form) do trigger a document update.

For instance, adding a button element triggers this updateSettings function, when added to the page. Elements utilizing the Lexical Editor (like Heading and Paragraph) are triggering this onChange function.

By adding new UpdateDocumentActionEvent() to the createElementAction we ensure the document is updated after adding a new element to the page. But since the elements mentioned already trigger a document update on their own, this addition now results in two separate API calls when they are added.

I've tried to fix this for the Button, but the fix didn't work as expected, so I reverted these Button changes.

};
};