fix: image deletion on submit fixed in comments #5748
Conversation
WalkthroughThe changes in this pull request modify the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Palanikannan1437
force-pushed
the
fix/image-deletion-on-clear-content
branch
from
fb1fdc3
to
0f7efb3
Compare
Palanikannan1437
force-pushed
the
fix/image-deletion-on-clear-content
branch
from
0f7efb3
to
50f4169
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/editor/src/core/plugins/image/delete-image.ts (1)
20-21: Approve the change with a minor suggestion for improvement.The added conditional check effectively addresses the PR objective by preventing image deletion when the "skipImageDeletion" meta property is set to true. This solution is elegant and non-intrusive, maintaining backward compatibility while solving the specific issue.
Consider adding a brief comment explaining when and why the "skipImageDeletion" meta property should be set. This would improve code maintainability and help other developers understand the purpose of this check. For example:
// if the transaction has meta of skipImageDeletion get to true, then return (like while clearing the editor content programatically) +// This is used to prevent image deletion when submitting comments or in other scenarios where we want to preserve images if (transaction.getMeta("skipImageDeletion")) return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/editor/src/core/hooks/use-editor.ts (1 hunks)
- packages/editor/src/core/plugins/image/delete-image.ts (1 hunks)
🔇 Additional comments (1)
packages/editor/src/core/plugins/image/delete-image.ts (1)
Line range hint
1-58: Verify if additional changes are needed to fully resolve the issue.The implemented change effectively prevents image deletion when the "skipImageDeletion" meta property is set. However, to ensure that this fully resolves the issue described in the PR objectives, we should verify:
- That the "skipImageDeletion" meta property is correctly set when clearing the editor content during comment submission.
- That no other parts of the codebase need modification to fully implement this solution.
To help verify this, you can run the following script to check for usage of "skipImageDeletion" across the codebase:
This script will help identify if the "skipImageDeletion" property is being set correctly in the relevant parts of the codebase, particularly where the editor content is being cleared.
✅ Verification successful
Verification Successful: "skipImageDeletion" is correctly implemented.
All instances of "skipImageDeletion" are appropriately set during editor content clearing operations. No additional changes are required.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of "skipImageDeletion" in the codebase # Search for "skipImageDeletion" in all TypeScript and JavaScript files echo "Searching for 'skipImageDeletion' usage:" rg --type-add 'web:*.{ts,tsx,js,jsx}' -t web 'skipImageDeletion' # Search for editor content clearing operations that might need to set "skipImageDeletion" echo "\nSearching for potential places where 'skipImageDeletion' should be set:" rg --type-add 'web:*.{ts,tsx,js,jsx}' -t web 'editor\s*\.\s*(commands\s*\.\s*clearContent|clear)'Length of output: 885
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (2)
Line range hint
113-127: LGTM with a minor suggestion.The simplification of the
getDisplayMessagefunction looks good. It aligns with the removal of logic related to handling existing files, as mentioned in the AI-generated summary.Consider renaming the
isUploadingvariable toisImageBeingUploadedfor consistency:- const isUploading = isImageBeingUploaded; + const isImageBeingUploaded = isImageBeingUploaded; if (failedToLoadImage) { return "Error loading image"; } - if (isUploading) { + if (isImageBeingUploaded) { return "Uploading..."; }This change would make the code more self-explanatory and consistent with the variable name used in the component's state.
Line range hint
1-165: Overall, the component looks well-structured and functional.The
CustomImageUploadercomponent appears to be well-implemented, using modern React practices such as hooks and functional components. The changes made to thegetDisplayMessagefunction simplify the logic without introducing any obvious issues.However, there are a few points to consider for future improvements:
Error Handling: The component handles a
failedToLoadImagestate, but it might be beneficial to add more robust error handling for upload failures.Accessibility: Consider adding appropriate ARIA attributes to improve accessibility, especially for the file input and the clickable div.
Internationalization: The displayed messages are hardcoded. If this application supports multiple languages, consider using a translation system.
Testing: Ensure that unit tests are updated or added to cover the changes in the
getDisplayMessagefunction and the overall behavior of the component.These suggestions are not critical for the current PR but could be considered for future improvements to enhance the component's robustness and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (2 hunks)
- packages/editor/src/core/hooks/use-read-only-editor.ts (1 hunks)
🔇 Additional comments (1)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
Line range hint
1-165: Clarify the relationship between these changes and the PR objectives.The PR objectives mention fixing an issue with image deletion on comment submission. However, the changes in this file don't directly address that issue. Instead, they simplify the image upload component by removing logic related to handling existing files.
Could you please clarify:
- How do these changes in the
CustomImageUploadercomponent contribute to fixing the image deletion issue?- Are there other files not included in this review that more directly address the PR objectives?
- Is this simplification a side effect of changes made elsewhere, or is it intentional refactoring?
To help understand the context, let's check for related changes in other files:
This will help us identify other relevant files that might be part of this fix.
✅ Verification successful
Verified: The changes in
CustomImageUploaderdo not directly address the image deletion issue. The image deletion functionality is handled in other parts of the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that might be related to comment submission or image handling rg -i -t typescript -t javascript "(comment.*submit|image.*delete|skipImageDeletion)" --glob '!**/node_modules/**'Length of output: 151
Script:
#!/bin/bash # Search for files related to comment submission or image deletion using file extensions rg -i "(comment.*submit|image.*delete|skipImageDeletion)" --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' --glob '**/*.ts' --glob '**/*.tsx' --glob '**/*.js' --glob '**/*.jsx' --glob '!**/node_modules/**'Length of output: 2285
| clearEditor: (emitUpdate = false) => { | ||
| editorRef.current?.chain().setMeta("skipImageDeletion", true).clearContent(emitUpdate).run(); |
There was a problem hiding this comment.
💡 Codebase verification
Action Required: Update clearEditor Calls to Include emitUpdate Parameter
The following files contain calls to clearEditor without the emitUpdate parameter:
space/core/components/issues/peek-overview/comment/add-comment.tsxweb/core/components/pages/editor/page-root.tsxweb/core/components/issues/issue-detail/issue-activity/comments/comment-create.tsxweb/core/components/issues/issue-modal/form.tsxweb/core/components/inbox/modals/create-edit-modal/edit-root.tsxweb/core/components/inbox/modals/create-edit-modal/create-root.tsx
Please update these calls to explicitly include the emitUpdate parameter to ensure consistent and intended behavior.
🔗 Analysis chain
Approved: Good fix for the image deletion issue.
The changes effectively address the problem described in the PR objectives. By setting the "skipImageDeletion" meta property to true before clearing the content, you prevent images from being incorrectly marked for deletion during comment submission.
A minor suggestion for improved clarity:
Consider adding a brief comment explaining the purpose of setting the "skipImageDeletion" meta property. This will help future maintainers understand the reasoning behind this change. For example:
clearEditor: (emitUpdate = false) => {
+ // Prevent images from being marked for deletion when clearing editor content
editorRef.current?.chain().setMeta("skipImageDeletion", true).clearContent(emitUpdate).run();
},To ensure this change doesn't affect other parts of the codebase, let's verify the usage of clearEditor:
This will help identify if there are any existing calls to clearEditor that might need to be updated to include the new emitUpdate parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any calls to clearEditor that might need updating
# Search for clearEditor calls
rg --type typescript --type javascript 'clearEditor\s*\(' -A 2
Length of output: 99
Script:
#!/bin/bash
# Description: Check for any calls to clearEditor that might need updating
# Search for clearEditor calls with corrected file types
rg --type ts --type js 'clearEditor\s*\(' -A 2
Length of output: 1688
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
packages/editor/src/core/hooks/use-file-upload.ts (1)
Line range hint
28-61: Include all dependencies in the 'useCallback' dependency arrayThe
uploadFilefunction depends ononUpload,editor, andloadImageFromFileSystem, but onlyonUploadis included in the dependency array. This could lead to stale references ifeditororloadImageFromFileSystemchange.Update the dependency array to include all dependencies:
const uploadFile = useCallback( async (file: File) => { // ... existing code ... }, - [onUpload] + [onUpload, editor, loadImageFromFileSystem] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/editor/src/core/extensions/custom-image/components/image-block.tsx (1 hunks)
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (5 hunks)
- packages/editor/src/core/extensions/drop.tsx (3 hunks)
- packages/editor/src/core/hooks/use-file-upload.ts (4 hunks)
🔇 Additional comments (11)
packages/editor/src/core/extensions/drop.tsx (3)
44-44: LGTM: Consistent use ofinsertImagesSafelyinhandleDropThis change is consistent with the modification in
handlePaste, replacinginsertImageswithinsertImagesSafely. It aligns well with the PR objective and maintains consistency across the file.
Line range hint
57-85: LGTM with suggestion: NewinsertImagesSafelyfunctionThe new
insertImagesSafelyfunction is a good addition that aligns with the PR objective of improving image handling. It includes safety checks to prevent insertion beyond document size and implements error handling for each image insertion, which improves robustness.However, I have one suggestion:
Consider adding a comment or log message in the catch block to provide more context about the error, such as the file name or size. This could help with debugging in the future. For example:
} catch (error) { - console.error(`Error while ${event}ing image:`, error); + console.error(`Error while ${event}ing image (${file.name}):`, error); }Also, I noticed that the function doesn't explicitly set the "skipImageDeletion" property as mentioned in the PR objectives. Could you clarify if this is handled elsewhere or if it needs to be addressed here?
Let's verify if "skipImageDeletion" is set elsewhere:
#!/bin/bash # Description: Check for usage of 'skipImageDeletion' in the codebase # Expected: At least one occurrence of 'skipImageDeletion' being set rg --type typescript --type tsx 'skipImageDeletion'
24-24: LGTM: ReplacedinsertImageswithinsertImagesSafelyThis change aligns with the PR objective of fixing image deletion on submit. The function signature matches the previous
insertImagesfunction, ensuring a smooth transition.Let's verify if this change is consistent across the codebase:
packages/editor/src/core/extensions/custom-image/components/image-block.tsx (3)
111-117: LGTM: Improved handling of aspect ratio for imagesThis change addresses the case when
aspectRatiois not provided (falsy), ensuring that it's set in both the component state and the node attributes. This improvement aligns with the PR objective of fixing image handling issues and should prevent problems with image resizing and display.
120-120: LGTM: Correct dependency array updateAdding
aspectRatioto the dependency array of theuseCallbackhook is a good change. It ensures that thehandleImageLoadfunction is recreated whenaspectRatiochanges, guaranteeing that the function always has access to the latestaspectRatiovalue. This update follows React best practices and helps prevent potential bugs related to stale closure values.
111-120: Summary: Improved image handling with aspect ratioThese changes effectively address the PR objective of fixing image handling issues. The modifications improve the management of
aspectRatioin theCustomImageBlockcomponent by:
- Ensuring that
aspectRatiois set in both the component state and node attributes when it's initially undefined.- Updating the
useCallbackdependency array to properly react toaspectRatiochanges.These improvements should lead to more consistent and reliable image rendering and resizing in the editor. The changes are well-implemented, localized, and maintain the overall code quality and readability of the component.
packages/editor/src/core/hooks/use-file-upload.ts (1)
67-75:⚠️ Potential issueAdd missing dependencies to the 'useDropZone' hook's callbacks
Within the
useDropZonehook, functions likeonDragEnter,onDragLeave, and event handlers reference state variables but may not include all necessary dependencies, potentially causing inconsistencies.Ensure all dependencies are included:
const onDragEnter = useCallback(() => { setDraggedInside(true); - }, []); + }, [setDraggedInside]); const onDragLeave = useCallback(() => { setDraggedInside(false); - }, []); + }, [setDraggedInside]);Similarly, update the dependency array for
onDrop:const onDrop = useCallback( async (e: DragEvent<HTMLDivElement>) => { // ... existing code ... }, - [uploader] + [uploader, editor, pos, setDraggedInside] );Likely invalid or redundant comment.
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (4)
75-79: PasspostouseDropZonefor accurate image placementAdding
pos: getPos()to theuseDropZonehook ensures that dropped images are inserted at the correct position within the editor. This change enhances the user experience by maintaining content flow.
112-112: Dependencies ingetDisplayMessageare accurateThe dependencies
[draggedInside, failedToLoadImage, isImageBeingUploaded]in thegetDisplayMessageuseCallbackhook are correctly specified. This ensures the message updates appropriately when these variables change.
159-159: Enable multiple image selection by addingmultipleattributeAdding the
multipleattribute to the<input>element allows users to select and upload multiple images simultaneously, improving the image uploading experience.
104-106: Verify error handling for multiple file uploadsWith the support for multiple file uploads, ensure that
uploadFirstImageAndInsertRemaininghandles errors gracefully for individual file uploads. This includes providing feedback if some files fail to upload while others succeed.
| } else { | ||
| console.error("No file found"); | ||
| } | ||
| uploadFirstImageAndInsertRemaining(editor, fileList, pos, uploader); |
There was a problem hiding this comment.
Handle the Promise returned by 'uploadFirstImageAndInsertRemaining'
Since uploadFirstImageAndInsertRemaining is now an async function, you should await its execution within the onDrop handler to ensure proper sequencing of operations.
Modify the onDrop function as follows:
const onDrop = useCallback(
- (e: DragEvent<HTMLDivElement>) => {
+ async (e: DragEvent<HTMLDivElement>) => {
setDraggedInside(false);
if (e.dataTransfer.files.length === 0) {
return;
}
const fileList = e.dataTransfer.files;
- uploadFirstImageAndInsertRemaining(editor, fileList, pos, uploader);
+ await uploadFirstImageAndInsertRemaining(editor, fileList, pos, uploader);
},
- [uploader]
+ [uploader, editor, pos]
);Committable suggestion was skipped due to low confidence.
| if (files.some((file) => file.type.indexOf("image") === -1)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify file type checking by filtering non-image files
Currently, the code returns early if any non-image files are found, which prevents image files from being processed if mixed content is dropped. Consider filtering out non-image files and proceeding with valid images.
Modify the code as follows:
- if (files.some((file) => file.type.indexOf("image") === -1)) {
- return;
- }
const filteredFiles = files.filter((f) => f.type.indexOf("image") !== -1);
+ if (filteredFiles.length !== files.length) {
+ console.warn("Some files were not images and have been ignored.");
+ }
if (filteredFiles.length === 0) {
console.error("No image files found to upload");
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (files.some((file) => file.type.indexOf("image") === -1)) { | |
| return; | |
| } | |
| const filteredFiles = files.filter((f) => f.type.indexOf("image") !== -1); | |
| if (filteredFiles.length !== files.length) { | |
| console.warn("Some files were not images and have been ignored."); | |
| } | |
| if (filteredFiles.length === 0) { | |
| console.error("No image files found to upload"); | |
| return; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
75-79: LGTM: EnhanceduseDropZonehook usage.The addition of
editorandposparameters to theuseDropZonehook provides more context, which can improve its functionality. This change aligns with the PR objectives of enhancing image handling.Consider destructuring
getPosin the component's props to directly usegetPos()instead ofgetPos: getPos,. This would make the code slightly more concise:const { draggedInside, onDrop, onDragEnter, onDragLeave } = useDropZone({ uploader: uploadFile, editor, pos: getPos(), });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/editor/src/core/extensions/custom-image/components/image-block.tsx (3 hunks)
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/extensions/custom-image/components/image-block.tsx
🔇 Additional comments (5)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (5)
8-8: LGTM: New import for handling multiple image uploads.The addition of
uploadFirstImageAndInsertRemainingimport aligns with the PR objective of improving image handling in comments. This change supports the new functionality for multiple image uploads.
Line range hint
112-126: LGTM: SimplifiedgetDisplayMessagefunction.The removal of
existingFilechecks simplifies the logic, making the code more readable and maintainable. This change aligns with the PR objectives of improving image handling.
131-133: LGTM: Improved component styling and interactivity.The addition of
cursor-defaultto base styles and the modification of hover behavior based on the editor's editability improve the user experience by providing appropriate visual feedback. The conditional styling is a good practice that enhances the component's usability.
160-160: LGTM: Enabled multiple file selection.The addition of the
multipleattribute to the file input aligns perfectly with the PR objectives of supporting multiple image uploads. This simple change is crucial for enabling the new functionality.
Line range hint
1-163: Overall assessment: Approved with minor suggestions.The changes in this file significantly improve the image upload functionality, aligning well with the PR objectives. Key improvements include:
- Support for multiple image uploads
- Enhanced error handling and user feedback
- Improved component styling and interactivity
The code is well-structured and follows good practices. However, there's one minor issue to address:
- Add a null check for
fileListin theonFileChangefunction to prevent potential runtime errors.Once this small change is implemented, the code will be ready for merging.
| e.preventDefault(); | ||
| const fileList = e.target.files; | ||
| uploadFirstImageAndInsertRemaining(editor, fileList, getPos(), uploadFile); |
There was a problem hiding this comment.
Approve changes, but address potential null fileList.
The changes align with the PR objectives of handling multiple image uploads. The use of e.preventDefault() is a good practice to prevent default form submission behavior.
However, the previous review comment about handling potential null fileList hasn't been addressed. To prevent potential runtime errors, consider adding a null check before proceeding:
onFileChange = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
e.preventDefault();
const fileList = e.target.files;
if (!fileList) {
return;
}
uploadFirstImageAndInsertRemaining(editor, fileList, getPos(), uploadFile);
},
[uploadFile]
);
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
103-106: Unnecessarye.preventDefault()inonChangehandlerCalling
e.preventDefault()in theonChangeevent handler for the input element is unnecessary, as thechangeevent does not have a default action that needs to be prevented. Usinge.preventDefault()here may have unintended side effects.Consider removing
e.preventDefault()from theonFileChangehandler.Apply this diff to remove
e.preventDefault():async (e: ChangeEvent<HTMLInputElement>) => { - e.preventDefault(); const fileList = e.target.files; await uploadFirstImageAndInsertRemaining(editor, fileList, getPos(), uploadFile); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (6 hunks)
- packages/editor/src/core/hooks/use-file-upload.ts (4 hunks)
🔇 Additional comments (4)
packages/editor/src/core/hooks/use-file-upload.ts (3)
98-105: Properly awaits 'uploadFirstImageAndInsertRemaining' in 'onDrop' handlerThe
onDrophandler is now asynchronous and correctly awaitsuploadFirstImageAndInsertRemaining, ensuring the upload process completes before proceeding.
147-149: Previous issue regarding file type checking still appliesThe earlier feedback about simplifying the file type checking logic by filtering out non-image files instead of returning early is still applicable.
161-161: Previous issue regarding awaiting 'uploaderFn' call still appliesThe
uploaderFnreturns a Promise and should be awaited to ensure the upload completes before proceeding. This issue was highlighted in a previous review and still needs to be addressed.packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
103-108: Previous comment about nullfileListis still validThe previous review comment regarding adding a null check for
fileListto prevent potential runtime errors is still applicable. Please consider adding a null check before proceeding.
| const files: File[] = []; | ||
| for (let i = 0; i < fileList.length; i += 1) { | ||
| const item = fileList.item(i); | ||
| if (item) { | ||
| files.push(item); | ||
| } | ||
| } | ||
| if (files.some((file) => file.type.indexOf("image") === -1)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize file processing by filtering non-image files directly
Consider refactoring the code to filter non-image files during the initial iteration, removing redundant checks and improving efficiency.
Apply the following changes:
export async function uploadFirstImageAndInsertRemaining(
editor: Editor,
fileList: FileList,
pos: number,
uploaderFn: (file: File) => Promise<void>
) {
- const files: File[] = [];
+ const filteredFiles: File[] = [];
for (let i = 0; i < fileList.length; i += 1) {
const item = fileList.item(i);
- if (item) {
- files.push(item);
+ if (item && item.type.indexOf("image") !== -1) {
+ filteredFiles.push(item);
}
}
- if (files.some((file) => file.type.indexOf("image") === -1)) {
- return;
- }
- const filteredFiles = files.filter((f) => f.type.indexOf("image") !== -1);
if (filteredFiles.length !== fileList.length) {
console.warn("Some files were not images and have been ignored.");
}
if (filteredFiles.length === 0) {
console.error("No image files found to upload");
return;
}
// Rest of the function...
}This refactor streamlines the logic by filtering non-image files during the initial collection, eliminating the need for an additional check.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const files: File[] = []; | |
| for (let i = 0; i < fileList.length; i += 1) { | |
| const item = fileList.item(i); | |
| if (item) { | |
| files.push(item); | |
| } | |
| } | |
| if (files.some((file) => file.type.indexOf("image") === -1)) { | |
| return; | |
| } | |
| const filteredFiles: File[] = []; | |
| for (let i = 0; i < fileList.length; i += 1) { | |
| const item = fileList.item(i); | |
| if (item && item.type.indexOf("image") !== -1) { | |
| filteredFiles.push(item); | |
| } | |
| } | |
| if (filteredFiles.length !== fileList.length) { | |
| console.warn("Some files were not images and have been ignored."); | |
| } | |
| if (filteredFiles.length === 0) { | |
| console.error("No image files found to upload"); | |
| return; | |
| } |
| }, | ||
| [uploadFile] | ||
| [uploadFile, editor, getPos, uploadFile] |
There was a problem hiding this comment.
Duplicate dependency uploadFile in dependency array
In the dependency array of the useCallback hook for onFileChange, uploadFile is included twice.
Current dependency array:
[uploadFile, editor, getPos, uploadFile]This redundancy is unnecessary and may cause confusion. Please remove the duplicate.
Apply this diff to fix the issue:
- [uploadFile, editor, getPos, uploadFile]
+ [uploadFile, editor, getPos]| "image-upload-component flex items-center justify-start gap-2 py-3 px-2 rounded-lg text-custom-text-300 hover:text-custom-text-200 bg-custom-background-90 hover:bg-custom-background-80 border border-dashed border-custom-border-300 cursor-pointer transition-all duration-200 ease-in-out cursor-default", | ||
| { | ||
| "hover:text-custom-text-200 cursor-pointer": editor.isEditable, |
There was a problem hiding this comment.
Conflicting cursor style classes
In line 131, both cursor-pointer and cursor-default classes are applied unconditionally. These classes set opposing cursor styles, and applying them together can cause confusion and unexpected behavior.
Additionally, in line 133, cursor-pointer is conditionally applied when editor.isEditable is true. However, since cursor-pointer is already applied unconditionally in line 131, the conditional application has no effect.
Consider removing cursor-pointer from line 131 to ensure that the cursor style is only set to pointer when editor.isEditable is true.
Apply this diff to fix the issue:
"image-upload-component flex items-center justify-start gap-2 py-3 px-2 rounded-lg text-custom-text-300 hover:text-custom-text-200 bg-custom-background-90 hover:bg-custom-background-80 border border-dashed border-custom-border-300 cursor-pointer transition-all duration-200 ease-in-out cursor-default",
+ "image-upload-component flex items-center justify-start gap-2 py-3 px-2 rounded-lg text-custom-text-300 hover:text-custom-text-200 bg-custom-background-90 hover:bg-custom-background-80 border border-dashed border-custom-border-300 transition-all duration-200 ease-in-out cursor-default",Committable suggestion was skipped due to low confidence.
Description
truewhile clearing the content of our editor via the editor ref APIs.Summary by CodeRabbit
Summary by CodeRabbit