Skip to content

[WIKI-804] fix: refactor image uploader #8210

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.

Sign up for GitHub

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

Merged
sriramveeraghanta merged 3 commits into from
Dec 5, 2025
Merged

[WIKI-804] fix: refactor image uploader #8210

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b011100
fix: refactor uploader
Palanikannan1437 Dec 1, 2025
8733acb
fix: props
Palanikannan1437 Dec 1, 2025
0aafa94
fix: sites fix
Palanikannan1437 Dec 1, 2025
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
3 changes: 2 additions & 1 deletion packages/editor/src/core/components/menus/block-menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import type { LucideIcon } from "lucide-react";
import { Copy, Trash2 } from "lucide-react";
import { useCallback, useEffect, useRef, useState } from "react";
import type { ISvgIcons } from "@plane/propel/icons";
import { cn } from "@plane/utils";
// constants
import { CORE_EXTENSIONS } from "@/constants/extension";
Expand All @@ -27,7 +28,7 @@
workItemIdentifier?: IEditorProps["workItemIdentifier"];
};
export type BlockMenuOption = {
icon: LucideIcon;
icon: LucideIcon | React.FC<ISvgIcons>;
key: string;
label: string;
onClick: (e: React.MouseEvent) => void;
Expand All @@ -35,7 +36,7 @@
};

export function BlockMenu(props: Props) {
const { editor, workItemIdentifier } = props;

Check warning on line 39 in packages/editor/src/core/components/menus/block-menu.tsx

View workflow job for this annotation

GitHub Actions / Build and lint web apps 

'workItemIdentifier' is assigned a value but never used. Allowed unused vars must match /^_/u
const [isOpen, setIsOpen] = useState(false);
const [isAnimatedIn, setIsAnimatedIn] = useState(false);
const menuRef = useRef<HTMLDivElement | null>(null);
Expand Down
14 changes: 8 additions & 6 deletions packages/editor/src/core/extensions/callout/logo-selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,27 +53,29 @@ export function CalloutBlockLogoSelector(props: Props) {
};
if (val.type === "emoji") {
// val.value is now a string in decimal format (e.g. "128512")
const emojiValue = val.value as string;
newLogoValue = {
"data-emoji-unicode": val.value,
"data-emoji-unicode": emojiValue,
"data-emoji-url": undefined,
};
newLogoValueToStoreInLocalStorage = {
in_use: "emoji",
emoji: {
value: val.value,
value: emojiValue,
url: undefined,
},
};
} else if (val.type === "icon") {
const iconValue = val.value as { name: string; color: string };
newLogoValue = {
"data-icon-name": val.value.name,
"data-icon-color": val.value.color,
"data-icon-name": iconValue.name,
"data-icon-color": iconValue.color,
};
newLogoValueToStoreInLocalStorage = {
in_use: "icon",
icon: {
name: val.value.name,
color: val.value.color,
name: iconValue.name,
color: iconValue.color,
},
};
}
Expand Down
16 changes: 9 additions & 7 deletions packages/editor/src/core/extensions/custom-image/components/uploader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function CustomImageUploader(props: CustomImageUploaderProps) {
// refs
const fileInputRef = useRef<HTMLInputElement>(null);
const hasTriggeredFilePickerRef = useRef(false);
const hasTriedUploadingOnMountRef = useRef(false);
const { id: imageEntityId } = node.attrs;
// derived values
const imageComponentImageFileMap = useMemo(() => getImageComponentImageFileMap(editor), [editor]);
Expand Down Expand Up @@ -124,17 +125,16 @@ export function CustomImageUploader(props: CustomImageUploaderProps) {
uploader: uploadFile,
});

// the meta data of the image component
const meta = useMemo(
() => imageComponentImageFileMap?.get(imageEntityId ?? ""),
[imageComponentImageFileMap, imageEntityId]
);

// after the image component is mounted we start the upload process based on
// it's uploaded
useEffect(() => {
if (hasTriedUploadingOnMountRef.current) return;

// the meta data of the image component
const meta = imageComponentImageFileMap?.get(imageEntityId ?? "");
if (meta) {
if (meta.event === "drop" && "file" in meta) {
hasTriedUploadingOnMountRef.current = true;
uploadFile(meta.file);
} else if (meta.event === "insert" && fileInputRef.current && !hasTriggeredFilePickerRef.current) {
if (meta.hasOpenedFileInputOnce) return;
Expand All @@ -144,8 +144,10 @@ export function CustomImageUploader(props: CustomImageUploaderProps) {
hasTriggeredFilePickerRef.current = true;
imageComponentImageFileMap?.set(imageEntityId ?? "", { ...meta, hasOpenedFileInputOnce: true });
}
} else {
hasTriedUploadingOnMountRef.current = true;
}
}, [meta, uploadFile, imageComponentImageFileMap, imageEntityId, isTouchDevice]);
}, [imageEntityId, isTouchDevice, uploadFile, imageComponentImageFileMap]);

const onFileChange = useCallback(
async (e: ChangeEvent<HTMLInputElement>) => {
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/core/extensions/extensions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export const CoreEditorExtensions = (args: TArguments): Extensions => {
CustomCalloutExtension,
UtilityExtension({
disabledExtensions,
flaggedExtensions,
fileHandler,
getEditorMetaData,
isEditable: editable,
Expand Down
5 changes: 3 additions & 2 deletions packages/editor/src/core/extensions/utility.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ export type UtilityExtensionStorage = {
isTouchDevice: boolean;
};

type Props = Pick<IEditorProps, "disabledExtensions" | "getEditorMetaData"> & {
type Props = Pick<IEditorProps, "disabledExtensions" | "flaggedExtensions" | "getEditorMetaData"> & {
fileHandler: TFileHandler;
isEditable: boolean;
isTouchDevice: boolean;
};

export const UtilityExtension = (props: Props) => {
const { disabledExtensions, fileHandler, getEditorMetaData, isEditable, isTouchDevice } = props;
const { disabledExtensions, flaggedExtensions, fileHandler, getEditorMetaData, isEditable, isTouchDevice } = props;
const { restore } = fileHandler;

return Extension.create<Record<string, unknown>, UtilityExtensionStorage>({
Expand All @@ -79,6 +79,7 @@ export const UtilityExtension = (props: Props) => {
}),
DropHandlerPlugin({
disabledExtensions,
flaggedExtensions,
editor: this.editor,
}),
PasteAssetPlugin(),
Expand Down
5 changes: 4 additions & 1 deletion packages/editor/src/core/plugins/drop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import type { TEditorCommands, TExtensions } from "@/types";

type Props = {
disabledExtensions?: TExtensions[];
flaggedExtensions?: TExtensions[];
editor: Editor;
};

export const DropHandlerPlugin = (props: Props): Plugin => {
const { disabledExtensions, editor } = props;
const { disabledExtensions, flaggedExtensions, editor } = props;

Comment on lines 8 to 16
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/editor/src/core/plugins/drop.ts

Repository: makeplane/plane

Length of output: 4973

🏁 Script executed:

# Search for all usages of flaggedExtensions in the file
rg "flaggedExtensions" packages/editor/src/core/plugins/drop.ts -A 2 -B 2

Repository: makeplane/plane

Length of output: 636

flaggedExtensions is passed to handlePaste but missing from handleDrop, and unused in insertFilesSafely

flaggedExtensions is properly passed to insertFilesSafely in handlePaste (line 37) but missing from the handleDrop call (lines 70–76). Additionally, flaggedExtensions is defined in InsertFilesSafelyArgs but never destructured or used inside insertFilesSafely (line 98 only destructures disabledExtensions, not flaggedExtensions), making the new parameter dead code.

This creates an inconsistency: when behavior is added to insertFilesSafely that depends on flaggedExtensions, it will only apply to paste events, not drag-and-drop events. Add flaggedExtensions to the handleDrop call to match handlePaste, and ensure insertFilesSafely actually uses it once the logic is implemented.

🤖 Prompt for AI Agents 
In packages/editor/src/core/plugins/drop.ts around lines 8 to 16 and related
spots (handlePaste at ~line 37, handleDrop at ~lines 70–76, and
insertFilesSafely definition at ~line 98), flaggedExtensions is passed to
handlePaste but omitted from handleDrop and never destructured/used in
insertFilesSafely; update the handleDrop call to include flaggedExtensions
alongside disabledExtensions and editor so both paste and drop flows receive the
same args, and modify insertFilesSafely's parameter destructuring to include
flaggedExtensions (and wire it into the function body or placeholder where the
future logic will use it) so the parameter is not dead code and both paste and
drop behaviors remain consistent.

return new Plugin({
key: new PluginKey("drop-handler-plugin"),
Expand All @@ -33,6 +34,7 @@ export const DropHandlerPlugin = (props: Props): Plugin => {
const pos = view.state.selection.from;
insertFilesSafely({
disabledExtensions,
flaggedExtensions,
editor,
files: acceptedFiles,
initialPos: pos,
Expand Down Expand Up @@ -84,6 +86,7 @@ export const DropHandlerPlugin = (props: Props): Plugin => {

type InsertFilesSafelyArgs = {
disabledExtensions?: TExtensions[];
flaggedExtensions?: TExtensions[];
editor: Editor;
event: "insert" | "drop";
files: File[];
Expand Down
60 changes: 28 additions & 32 deletions packages/editor/src/core/plugins/file/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const TrackFileDeletionPlugin = (editor: Editor, deleteHandler: TFileHand
[nodeType: string]: Set<string> | undefined;
} = {};
if (!transactions.some((tr) => tr.docChanged)) return null;
if (transactions.some((tr) => tr.getMeta(CORE_EDITOR_META.SKIP_FILE_DELETION))) return null;

newState.doc.descendants((node) => {
const nodeType = node.type.name as keyof NodeFileMapType;
Expand All @@ -34,40 +35,35 @@ export const TrackFileDeletionPlugin = (editor: Editor, deleteHandler: TFileHand
}
});

transactions.forEach((transaction) => {
// if the transaction has meta of skipFileDeletion set to true, then return (like while clearing the editor content programmatically)
if (transaction.getMeta(CORE_EDITOR_META.SKIP_FILE_DELETION)) return;
const removedFiles: TFileNode[] = [];

const removedFiles: TFileNode[] = [];

// iterate through all the nodes in the old state
oldState.doc.descendants((node) => {
const nodeType = node.type.name as keyof NodeFileMapType;
const isAValidNode = NODE_FILE_MAP[nodeType];
// if the node doesn't match, then return as no point in checking
if (!isAValidNode) return;
// Check if the node has been deleted or replaced
if (!newFileSources[nodeType]?.has(node.attrs.src)) {
removedFiles.push(node as TFileNode);
}
});
// iterate through all the nodes in the old state
oldState.doc.descendants((node) => {
const nodeType = node.type.name as keyof NodeFileMapType;
const isAValidNode = NODE_FILE_MAP[nodeType];
// if the node doesn't match, then return as no point in checking
if (!isAValidNode) return;
// Check if the node has been deleted or replaced
if (!newFileSources[nodeType]?.has(node.attrs.src)) {
removedFiles.push(node as TFileNode);
}
});

removedFiles.forEach(async (node) => {
const nodeType = node.type.name as keyof NodeFileMapType;
const src = node.attrs.src;
const nodeFileSetDetails = NODE_FILE_MAP[nodeType];
if (!nodeFileSetDetails || !src) return;
try {
editor.storage[nodeType]?.[nodeFileSetDetails.fileSetName]?.set(src, true);
// update assets list storage value
editor.commands.updateAssetsList?.({
idToRemove: node.attrs.id,
});
await deleteHandler(src);
} catch (error) {
console.error("Error deleting file via delete utility plugin:", error);
}
});
removedFiles.forEach(async (node) => {
const nodeType = node.type.name as keyof NodeFileMapType;
const src = node.attrs.src;
const nodeFileSetDetails = NODE_FILE_MAP[nodeType];
if (!nodeFileSetDetails || !src) return;
try {
editor.storage[nodeType]?.[nodeFileSetDetails.fileSetName]?.set(src, true);
// update assets list storage value
editor.commands.updateAssetsList?.({
idToRemove: node.attrs.id,
});
await deleteHandler(src);
} catch (error) {
console.error("Error deleting file via delete utility plugin:", error);
}
});

return null;
Expand Down
Loading