implement image pasting in md editor #7387
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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
Documentation and Community
|
![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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)src/core/ui/forms/MarkdownInput/MarkdownInput.tsx(4 hunks)src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/core/ui/forms/MarkdownInput/MarkdownInput.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
🔇 Additional comments (28)
src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx (1)
46-46: Good approach allowing protocol checks while disabling the ESLint rule.By using
// eslint-disable-next-line no-script-url, you can explicitly handle dangerous protocols likejavascript:for XSS prevention. The accompanying comment clarifies the intent, so this is a well-documented and secure workaround against malicious URLs.src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (22)
1-11: Imports look good.
No issues found with the newly introduced imports or usage.
22-22: Editor import is correct.
ImportingEditorto manage the Tiptap editor instance is appropriate.
33-35: New Apollo and notification imports are consistent.
These imports to handle file uploads and notifications align well with the existing architecture.
85-86: Tracking the editor in component state looks good.
Maintaining the editor instance in component state is a solid approach for external manipulations.
87-87: UsinguseNotificationis correct.
The notification hook usage is consistent with the project's pattern.
97-98: Retrieving storage configuration is valid.
This is a suitable way to determine which storage bucket to use for uploads.
101-108: Mutation for file uploads is well integrated.
Reliance on the global error handler is acceptable; no local error handling needed.
109-110: Translation hook usage is appropriate.
Provides localized feedback for users upon file upload success.
111-111: Validation message translation is consistent.
Ensures error messages are localized properly.
Line range hint
129-137: RenamedhandleOnChangelogic is clear.
Trimming extra<br>tags is a good approach for handling empty content.
203-209: Passing the editor instance upward is useful.
handlePassEditorandinputComponentkeep the component flexible.
216-216: CheckinglabelOffsetis a neat conditional.
This ensures the label is positioned correctly based on the editor's layout.
229-229:OutlinedInputusage is appropriate.
No issues with using the MUI component for a multiline Markdown editor.
231-231: Multiline prop is correctly set.
This ensures a suitable text area for Markdown entry.
233-233: Bindingfield.valueis correct.
Keeps the Formik state in sync with the component.
235-235:readOnlyprop is handled properly.
The component respects the read-only state as expected.
238-238: Additional props for controlling the component are well integrated.
Allows toggling various behavioral options seamlessly.
243-243:inputComponentusage is a creative approach.
PassingpassEditorto the underlyingMarkdownInputfosters reusability.
244-244: Applying custom padding matches design guidelines.
Minor styling changes look consistent with the overall UI.
245-245:onBlurhandling is straightforward.
Triggers Formik's touched state as expected.
246-247:onPasteandonChangeattachments are correct.
Both events are properly integrated to handle input changes and paste events.
249-249: Final closing tag is in place.
No structural issues here.src/core/ui/forms/MarkdownInput/MarkdownInput.tsx (5)
34-34:passEditorprop is a solid extension point.
Allows the parent component to capture the editor instance for advanced usage.
75-75: DestructuringpassEditoris correct.
Retrieves the optional callback properly from props.
100-105: InvokingpassEditorinside auseEffectis correct.
Ensures the parent gets the editor instance as soon as it's ready.
286-286: Bounding editor height improves usability.
A maximum height helps to contain large amounts of text in the UI.
289-289: Conditional rendering withCharacterCountContaineris consistent.
Prevents overflow highlighting unless needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (2)
101-107: Add error handling for file upload failures.While the success case is handled well, consider adding error handling to notify users when uploads fail.
const [uploadFile] = useUploadFileMutation({ onCompleted: data => { notify(t('components.file-upload.file-upload-success'), 'success'); editor?.commands.setImage({ src: data.uploadFileOnStorageBucket, alt: 'pasted-image' }); }, + onError: error => { + notify(t('components.file-upload.file-upload-error'), 'error'); + logError(error); + }, });
196-201: Add cleanup for editor instance.Consider cleaning up the editor reference when the component unmounts to prevent memory leaks.
+ useEffect(() => { + return () => { + setEditor(undefined); + }; + }, []);Also applies to: 236-240
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
🔇 Additional comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
1-11: LGTM! Clean implementation of new imports and state management.The new imports and state management are well-organized and properly typed for handling clipboard events and file uploads.
Also applies to: 33-35, 85-88
reactoholic
force-pushed
the
client-web/7385-implement-image-pasting-in-md-editor
branch
2 times, most recently
from
9f7a004 to
c91ddf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Line range hint
85-107: Add error handling for file upload failures.The upload mutation should handle error cases to provide feedback to users when uploads fail.
const [uploadFile] = useUploadFileMutation({ onCompleted: data => { notify(t('components.file-upload.file-upload-success'), 'success'); editor?.commands.setImage({ src: data.uploadFileOnStorageBucket, alt: 'pasted-image' }); }, + onError: error => { + notify(t('components.file-upload.file-upload-error'), 'error'); + console.error('File upload failed:', error); + }, });
♻️ Duplicate comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
138-180: 🛠️ Refactor suggestionAdd file validation checks in paste handler.
The paste handler should validate file types and sizes before uploading:
const handleOnPaste = useCallback( (event: ClipboardEvent) => { const clipboardData = event.clipboardData; const items = clipboardData.items; - if (!items) return; + if (!items || !storageConfig?.storageBucketId) return; - const storageBucketId = storageConfig?.storageBucketId; + const storageBucketId = storageConfig.storageBucketId; + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + const ALLOWED_TYPES = ['image/jpeg', 'image/png', 'image/gif']; if (storageBucketId) { let hasImage = false; for (const item of items) { - if (item.type.startsWith('image/')) { + if (ALLOWED_TYPES.includes(item.type)) { const file = item.getAsFile(); if (file) { + if (file.size > MAX_FILE_SIZE) { + notify(t('components.file-upload.file-too-large'), 'error'); + return; + } const reader = new FileReader();
🧹 Nitpick comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
227-244: Consider maintaining consistent props ordering.While the functionality is correct, maintaining a consistent props order across components helps with code readability and review efficiency.
Consider following this order:
- Core props (multiline, label, value)
- Refs and callbacks (inputRef)
- State props (readOnly, inputProps)
- Event handlers (onBlur, onPaste, onChange)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)src/core/ui/forms/MarkdownInput/MarkdownInput.tsx(4 hunks)src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/ui/forms/MarkdownInput/MarkdownInput.tsx
- src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx
🧰 Additional context used
📓 Path-based instructions (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
🔇 Additional comments (2)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (2)
Line range hint
1-35: Clean and well-organized imports!The new imports are properly organized and necessary for implementing the image paste functionality.
200-205: Well-implemented editor integration!The editor instance management is clean and properly memoized using useCallback.
reactoholic
force-pushed
the
client-web/7385-implement-image-pasting-in-md-editor
branch
from
dcbd959 to
ce38df9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
138-180: 🛠️ Refactor suggestionEnhance paste handler with file validations.
The paste handler should validate file types and sizes before upload.
const handleOnPaste = useCallback( (event: ClipboardEvent) => { const clipboardData = event.clipboardData; const items = clipboardData.items; - if (!items) return; + if (!items || !storageConfig?.storageBucketId) return; + + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + const ALLOWED_TYPES = ['image/jpeg', 'image/png', 'image/gif']; + let hasImage = false; - const storageBucketId = storageConfig?.storageBucketId; + const storageBucketId = storageConfig.storageBucketId; - if (storageBucketId) { - let hasImage = false; + for (const item of items) { + if (!ALLOWED_TYPES.includes(item.type)) continue; + + const file = item.getAsFile(); + if (!file) continue; + + if (file.size > MAX_FILE_SIZE) { + notify(t('components.file-upload.file-too-large'), 'error'); + return; + } - for (const item of items) { - if (item.type.startsWith('image/')) { - const file = item.getAsFile(); - if (file) { - const reader = new FileReader(); + const reader = new FileReader(); + reader.onload = () => { + uploadFile({ + variables: { + file, + uploadData: { + storageBucketId, + temporaryLocation: true, + }, + }, + }); + }; - reader.onload = () => { - uploadFile({ - variables: { - file, - uploadData: { - storageBucketId, - temporaryLocation: true, - }, - }, - }); - }; + reader.onerror = () => { + notify(t('components.file-upload.read-error'), 'error'); + }; reader.readAsDataURL(file); hasImage = true; - } - } - } if (hasImage) { event.preventDefault(); } } }, [storageConfig?.storageBucketId, uploadFile] );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)src/core/ui/forms/MarkdownInput/MarkdownInput.tsx(4 hunks)src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/ui/forms/MarkdownInput/MarkdownInput.tsx
- src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx
🧰 Additional context used
📓 Path-based instructions (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
🔇 Additional comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
202-205: LGTM! Good use of memoization and event handling.The component properly memoizes the input component and correctly wires up the event handlers.
Also applies to: 240-244
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)src/core/ui/forms/MarkdownInput/MarkdownInput.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/ui/forms/MarkdownInput/MarkdownInput.tsx
🧰 Additional context used
📓 Path-based instructions (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Travis CI - Pull Request
🔇 Additional comments (2)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (2)
164-166: Verify temporary storage usage policy.The
temporaryLocationflag is always set to the prop value, but there's uncertainty about when to use temporary storage. This needs clarification from the backend team.✅ Verification successful
Temporary storage usage policy is well-documented
The usage pattern is clearly defined in
useStorageConfig.tsx: temporaryLocation should be false by default and set to true only for specific components that need it (e.g., callout creation).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of temporaryLocation to understand the pattern rg -A 5 "temporaryLocation.*true"Length of output: 1236
141-183: 🛠️ Refactor suggestionEnhance image paste validation.
The paste handler should include file size and type validations to prevent issues with large or unsupported files.
const handleOnPaste = useCallback( (event: ClipboardEvent) => { const clipboardData = event.clipboardData; const items = clipboardData.items; - if (!items) return; + if (!items || !storageConfig?.storageBucketId) return; - const storageBucketId = storageConfig?.storageBucketId; + const storageBucketId = storageConfig.storageBucketId; + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + const ALLOWED_TYPES = ['image/jpeg', 'image/png', 'image/gif']; - if (storageBucketId) { - let hasImage = false; + let hasImage = false; - for (const item of items) { - if (item.type.startsWith('image/')) { - const file = item.getAsFile(); - if (file) { + for (const item of items) { + if (ALLOWED_TYPES.includes(item.type)) { + const file = item.getAsFile(); + if (file) { + if (file.size > MAX_FILE_SIZE) { + notify(t('components.file-upload.file-too-large'), 'error'); + return; + } const reader = new FileReader(); reader.onload = () => { uploadFile({ variables: { file, uploadData: { storageBucketId, temporaryLocation, }, }, }); }; + reader.onerror = () => { + notify(t('components.file-upload.read-error'), 'error'); + }; reader.readAsDataURL(file); hasImage = true; } } } if (hasImage) { event.preventDefault(); } - } }, [storageConfig?.storageBucketId, uploadFile, notify] );Likely invalid or redundant comment.
reactoholic
force-pushed
the
client-web/7385-implement-image-pasting-in-md-editor
branch
from
e9a9d11 to
ee9af27
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
217-222: Remove unused dependency from useCallback.The
handleOnPastedependency is not used in the callback and should be removed.const inputComponent = useCallback( (props: PropsWithChildren<InputBaseComponentProps>) => ( <MarkdownInput {...props} pasteImageHandler={handleImagePaste} /> ), - [handleImagePaste, handleOnPaste] + [handleImagePaste] );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx(8 hunks)src/core/ui/forms/MarkdownInput/MarkdownInput.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/ui/forms/MarkdownInput/MarkdownInput.tsx
🧰 Additional context used
📓 Path-based instructions (1)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (1)
Pattern src/**/*.{ts,tsx,js}: Review the React.js/TypeScript/JavaScript code for best practices and potential bugs.
Ensure that the code adheres to TypeScript's typing system and modern standards.
Use the following code guide documentation in the codebase: 'docs/code-guidelines.md'.
Ensure sufficient error handling and logging is present, but note:
- We have a global error handler in place. So local
try/catchblocks are not required unless they provide value beyond what the global error handler already covers. - Avoid flagging the absence of local error handling in cases where the global error handler suffices.
- Avoid flagging the absence of logging in cases where the global logger is already in place (graphql usage).
Inform about the recommendation of not using non-null assertion,
except on GraphQL queries where the variable is asserted non-null and followed by a
skip: with the same variable.
Examples:
- Not Required: there's a skip
{
variables: { templateId: templateId! },
skip: !templateId,
}
Check for common security vulnerabilities such as:
- SQL Injection
- XSS (Cross-Site Scripting)
- CSRF (Cross-Site Request Forgery)
- Insecure dependencies
- Sensitive data exposure
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Travis CI - Pull Request
🔇 Additional comments (3)
src/core/ui/forms/MarkdownInput/FormikMarkdownField.tsx (3)
Line range hint
1-277: Overall implementation looks good!The image paste functionality is well-implemented with proper separation of concerns. The suggested improvements will enhance the robustness and user experience of the feature.
141-195: 🛠️ Refactor suggestionEnhance paste handler with validation and loading state.
The paste handler should include file type/size validation and show loading state during upload.
+ const ALLOWED_IMAGE_TYPES = ['image/jpeg', 'image/png', 'image/gif']; + const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB const handleOnPaste = useCallback( (event: ClipboardEvent) => { const clipboardData = event.clipboardData; const items = clipboardData?.items; if (!items) return; const storageBucketId = storageConfig?.storageBucketId; if (storageBucketId) { let imageProcessed = false; for (const item of items) { if (item.kind === 'file' && item.type.startsWith('image/')) { const file = item.getAsFile(); if (file) { + if (!ALLOWED_IMAGE_TYPES.includes(file.type)) { + notify(t('components.file-upload.invalid-type'), 'error'); + return; + } + if (file.size > MAX_FILE_SIZE) { + notify(t('components.file-upload.file-too-large'), 'error'); + return; + } + const reader = new FileReader(); reader.onload = () => { + editor?.setOptions({ editable: false }); uploadFile({ variables: { file, uploadData: { storageBucketId, temporaryLocation, }, }, + }).finally(() => { + editor?.setOptions({ editable: true }); + }); }; reader.readAsDataURL(file); imageProcessed = true; break; } }Likely invalid or redundant comment.
101-110: 🛠️ Refactor suggestionImprove error handling in upload mutation.
The error handling should use the notification system for consistency and provide a better user experience.
const [uploadFile] = useUploadFileMutation({ onCompleted: data => { notify(t('components.file-upload.file-upload-success'), 'success'); editor?.commands.setImage({ src: data.uploadFileOnStorageBucket, alt: 'pasted-image' }); }, onError: error => { - console.error(error.message); + notify(t('components.file-upload.file-upload-error'), 'error'); }, });Likely invalid or redundant comment.
reactoholic
force-pushed
the
client-web/7385-implement-image-pasting-in-md-editor
branch
from
ee9af27 to
b6c9c52
Compare
reactoholic
force-pushed
the
client-web/7385-implement-image-pasting-in-md-editor
branch
from
b6c9c52 to
9a26fc7
Compare
src/core/ui/forms/MarkdownInputControls/MarkdownInputControls.tsx
Outdated
Show resolved
Hide resolved
bobbykolev
deleted the
client-web/7385-implement-image-pasting-in-md-editor
branch
implement image pasting in md editor
Summary by CodeRabbit
New Features
Documentation
The changes focus on improving the markdown input experience with advanced editing capabilities and better file upload support.