Skip to content
Merged
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
7 changes: 7 additions & 0 deletions .changeset/product-media-render-loop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"saleor-dashboard": patch
---

Fixed the product page freezing while it loads. The media card re-rendered
itself in a loop until the product query resolved, which could lock up the tab
on slower connections.
14 changes: 14 additions & 0 deletions .changeset/rich-text-editor-images.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"saleor-dashboard": patch
---

Rich text fields (product, category and collection descriptions, CMS pages, and
rich text attributes) now support images. Pick "Image" from the editor toolbar
and paste a link to an externally hosted image, or paste the link straight into
the editor.

Uploading files to Saleor media storage is not supported yet, so dragging,
dropping or pasting an image file does nothing.

Note: storefronts and other API clients that render rich text content need to
handle the `image` block to display these images.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"@dnd-kit/utilities": "^3.2.2",
"@editorjs/editorjs": "2.30.7",
"@editorjs/header": "2.8.8",
"@editorjs/image": "^2.10.3",
"@editorjs/list": "2.0.8",
"@editorjs/paragraph": "2.11.7",
"@editorjs/quote": "2.7.6",
Expand Down
13 changes: 13 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 67 additions & 0 deletions src/components/RichTextEditor/consts.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { type ToolSettings } from "@editorjs/editorjs";

import { tools } from "./consts";

const imageTool = tools.image as ToolSettings;
const uploader = imageTool.config!.uploader;

describe("rich text image tool", () => {
it("is offered in the toolbox", () => {
// Assert
expect(imageTool.toolbox).toBeUndefined();
});

it("asks for a url instead of opening a file picker", () => {
// Arrange
const ImageTool = imageTool.class as unknown as new (...args: never[]) => {
askForUrl(): void;
uploadUrl: jest.Mock;
};
const tool = Object.create(ImageTool.prototype);

tool.uploadUrl = jest.fn();
jest.spyOn(window, "prompt").mockReturnValue(" https://example.com/cat.png ");

// Act
tool.askForUrl();

// Assert
expect(tool.uploadUrl).toHaveBeenCalledWith("https://example.com/cat.png");
});

it("rejects file uploads", async () => {
// Act
const result = await uploader.uploadByFile(new Blob([], { type: "image/png" }));

// Assert
expect(result).toEqual({ success: 0, file: { url: "" } });
});

it("accepts an externally hosted image url", async () => {
// Act
const result = await uploader.uploadByUrl("https://example.com/cat.png");

// Assert
expect(result).toEqual({ success: 1, file: { url: "https://example.com/cat.png" } });
});

it.each(["data:image/png;base64,AAAA", "blob:http://localhost/abc", "javascript:alert(1)"])(
"rejects non-http(s) source %s",
async url => {
// Act
const result = await uploader.uploadByUrl(url);

// Assert
expect(result).toEqual({ success: 0, file: { url: "" } });
},
);

it("does not handle pasted/dropped files", () => {
// Act
const pasteConfig = (imageTool.class as unknown as { pasteConfig: { files?: unknown } })
.pasteConfig;

// Assert
expect(pasteConfig.files).toBeUndefined();
});
});
79 changes: 78 additions & 1 deletion src/components/RichTextEditor/consts.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// @ts-strict-ignore
import { StrikethroughIcon } from "@dashboard/icons/StrikethroughIcon";
import { type ToolConstructable, type ToolSettings } from "@editorjs/editorjs";
import { type PasteConfig, type ToolConstructable, type ToolSettings } from "@editorjs/editorjs";
import Embed from "@editorjs/embed";
import Header from "@editorjs/header";
import Image from "@editorjs/image";
import List from "@editorjs/list";
import Paragraph from "@editorjs/paragraph";
import Quote from "@editorjs/quote";
Expand All @@ -11,6 +12,71 @@ import createGenericInlineTool from "editorjs-inline-tool";

const inlineToolbar = ["link", "bold", "italic", "strikethrough"];

// The plugin types mark `uploadUrl` private, but it is the tool's own entry point
// for filling a block from a URL (shows the preloader, then stores the result).
interface ImageToolInternals {
uploadUrl(url: string): void;
}

// Editor.js renders its own UI in English (toolbox names, tunes, error toasts), so
// these follow suit rather than being the only translated strings in the editor.
const ADD_BY_LINK_LABEL = "Add image by link";
const ADD_BY_LINK_PROMPT = "Paste a link to an image";

/**
* Uploading images to Saleor media storage is not wired up yet, so the image tool
* is restricted to externally hosted images: pasted as a link (or as HTML with an
* `<img>`), or entered through the toolbox. Existing image blocks still render.
*/
class ExternalImage extends Image {
// Drop the drag-n-drop / clipboard file handlers - without an uploader they can
// only fail. URL and <img> paste handling stays.
static get pasteConfig(): PasteConfig {
const inherited: Exclude<PasteConfig, false> = super.pasteConfig || {};

return { tags: inherited.tags, patterns: inherited.patterns };
}

// Both entry points into the empty block (the toolbox item and the button inside
// it) ask for a URL. The base implementation opens a file picker instead.
// ponytail: window.prompt keeps this to a few lines while uploads are disabled;
// replace it with a proper inline URL field if this outlives the missing API.
askForUrl(): void {
const url = window.prompt(ADD_BY_LINK_PROMPT);

if (url) {
(this as unknown as ImageToolInternals).uploadUrl(url.trim());
}
}

// Fires when the image tool is picked from the "+" toolbox.
appendCallback(): void {
this.askForUrl();
}

render(): HTMLDivElement {
const wrapper = super.render();
// Swap the plugin's "Select an Image" button - cloning it drops the built-in
// click listener that opens the file picker.
const fileButton = wrapper.querySelector(".cdx-button");
const linkButton = fileButton?.cloneNode(true);

if (fileButton && linkButton) {
linkButton.addEventListener("click", () => this.askForUrl());
fileButton.replaceWith(linkButton);
}

return wrapper;
}
}

const rejectUpload = async () => ({ success: 0, file: { url: "" } });

// Stores the pasted URL as-is. data:/blob: sources are rejected - they would inline
// the whole file into the saved rich text instead of referencing a hosted image.
const acceptExternalUrl = async (url: string) =>
/^https?:\/\//i.test(url) ? { success: 1, file: { url } } : rejectUpload();

export const tools: Record<string, ToolConstructable | ToolSettings> = {
embed: Embed,
header: {
Expand Down Expand Up @@ -43,6 +109,17 @@ export const tools: Record<string, ToolConstructable | ToolSettings> = {
class: Paragraph,
inlineToolbar,
},
image: {
class: ExternalImage,
config: {
buttonContent: ADD_BY_LINK_LABEL,
uploader: {
uploadByUrl: acceptExternalUrl,
// Without this the tool falls back to POSTing the file to an undefined endpoint.
uploadByFile: rejectUpload,
},
},
},
strikethrough: createGenericInlineTool({
sanitize: {
s: {},
Expand Down
10 changes: 10 additions & 0 deletions src/components/RichTextEditor/fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@
"text": "Dolor sit amet, consectetur adipiscing elit. <b>Sed iaculis urna et justo accumsan</b>, eget porta est egestas. Nunc odio libero, pharetra in tristique eget, pellentesque in lectus. Sed sed laoreet orci. Suspendisse dui nibh, iaculis ac dui posuere, placerat elementum dolor. In sit amet aliquet nibh. Maecenas sed felis sed lectus gravida vulputate et a mi. Sed a tristique neque, ut euismod arcu. <i>Donec quis aliquet massa.</i> Curabitur arcu purus, facilisis quis posuere sit amet, pharetra at erat."
}
},
{
"type": "image",
"data": {
"file": { "url": "https://placehold.co/600x400" },
"caption": "Placeholder image",
"withBorder": false,
"stretched": false,
"withBackground": false
}
},
{
"type": "list",
"data": {
Expand Down
40 changes: 40 additions & 0 deletions src/products/components/ProductMedia/ProductMedia.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,44 @@ describe("ProductMedia", () => {
expect(onImageUpload).toHaveBeenCalledWith(image, 0);
expect(screen.getByTestId("media-tile-loading")).toBeInTheDocument();
});
it("renders without an update loop while media is still loading", () => {
// Arrange
const consoleError = jest.spyOn(console, "error").mockImplementation(() => undefined);

// Act
const { rerender } = render(
<TestWrapper>
<ProductMedia
media={undefined}
getImageEditUrl={(id): string => `/media/${id}`}
onImageDelete={() => () => undefined}
onImagesDelete={() => undefined}
onImageUpload={jest.fn()}
openMediaUrlModal={() => undefined}
/>
</TestWrapper>,
);

// A re-render while media is still undefined is what kicks off the loop.
// A regression here does not fail an assertion - it hangs this test.
rerender(
<TestWrapper>
<ProductMedia
media={undefined}
getImageEditUrl={(id): string => `/media/${id}`}
onImageDelete={() => () => undefined}
onImagesDelete={() => undefined}
onImageUpload={jest.fn()}
openMediaUrlModal={() => undefined}
/>
</TestWrapper>,
);

// Assert
expect(
consoleError.mock.calls.some(call => String(call[0]).includes("Maximum update depth")),
).toBe(false);

consoleError.mockRestore();
});
});
10 changes: 8 additions & 2 deletions src/products/components/ProductMedia/ProductMedia.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ const MediaList = ({
);

interface ProductMediaProps {
media: ProductMediaFragment[];
// Undefined until the product query resolves - the component guards for it throughout.
media: ProductMediaFragment[] | undefined;
loading?: boolean;
getImageEditUrl: (id: string) => string;
onImageDelete: (id: string) => () => void;
Expand Down Expand Up @@ -135,6 +136,11 @@ const revokeObjectUrl = (url: string) => {
const getMediaIdsSignature = (media: ProductMediaFragment[] | undefined) =>
media === undefined ? null : media.map(item => item.id).join("\0");

// Stable identity for the "not loaded yet" case. An inline `media ?? []` would hand
// useProductMediaDrag a new array on every render, and its media-sync effect would
// setState on every one of them - an endless render loop while the query is in flight.
const NO_MEDIA: ProductMediaFragment[] = [];

const ProductMedia = (props: ProductMediaProps) => {
const {
media,
Expand Down Expand Up @@ -172,7 +178,7 @@ const ProductMedia = (props: ProductMediaProps) => {
handleDragEnd,
handleDragCancel,
} = useProductMediaDrag({
media: media ?? [],
media: media ?? NO_MEDIA,
onReorder: onImageReorder,
disabled: isUploading,
});
Expand Down
Loading