-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[WEB-2450] dev: custom image extension #5585
Conversation
WalkthroughThe changes primarily focus on refactoring file handling and image upload functionalities within the editor. The Changes
Possibly related PRs
Suggested labels
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 (
|
634f5c1
to
d8308cf
Compare
e0c8453
to
78b1c50
Compare
2942c35
to
8b9418d
Compare
74fa836
to
d7484df
Compare
d7484df
to
5da5770
Compare
74e72b3
to
889dbb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (25)
- packages/editor/src/ce/extensions/document-extensions.tsx (1 hunks)
- packages/editor/src/core/components/editors/editor-content.tsx (0 hunks)
- packages/editor/src/core/components/menus/block-menu.tsx (1 hunks)
- packages/editor/src/core/components/menus/menu-items.ts (3 hunks)
- packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1 hunks)
- packages/editor/src/core/extensions/custom-image/custom-image.ts (1 hunks)
- packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts (1 hunks)
- packages/editor/src/core/extensions/extensions.tsx (4 hunks)
- packages/editor/src/core/extensions/image/extension.tsx (2 hunks)
- packages/editor/src/core/extensions/image/image-component-without-props.tsx (1 hunks)
- packages/editor/src/core/extensions/image/image-extension-without-props.tsx (2 hunks)
- packages/editor/src/core/extensions/image/image-resize.tsx (0 hunks)
- packages/editor/src/core/extensions/image/index.ts (0 hunks)
- packages/editor/src/core/extensions/image/read-only-image.tsx (2 hunks)
- packages/editor/src/core/hooks/use-editor.ts (5 hunks)
- packages/editor/src/core/hooks/use-file-upload.ts (1 hunks)
- packages/editor/src/core/hooks/use-read-only-editor.ts (2 hunks)
- packages/editor/src/core/plugins/image/delete-image.ts (3 hunks)
- packages/editor/src/core/plugins/image/restore-image.ts (2 hunks)
- packages/editor/src/core/plugins/image/utils/validate-file.ts (1 hunks)
- packages/editor/src/index.ts (0 hunks)
- packages/editor/src/styles/drag-drop.css (3 hunks)
- packages/editor/src/styles/editor.css (1 hunks)
- packages/helpers/package.json (1 hunks)
- packages/helpers/tsconfig.json (1 hunks)
Files not reviewed due to no reviewable changes (4)
- packages/editor/src/core/components/editors/editor-content.tsx
- packages/editor/src/core/extensions/image/image-resize.tsx
- packages/editor/src/core/extensions/image/index.ts
- packages/editor/src/index.ts
Files skipped from review as they are similar to previous changes (4)
- packages/editor/src/ce/extensions/document-extensions.tsx
- packages/editor/src/core/components/menus/menu-items.ts
- packages/editor/src/core/hooks/use-editor.ts
- packages/editor/src/core/hooks/use-file-upload.ts
Additional context used
Biome
packages/editor/src/core/extensions/custom-image/components/image-node.tsx
[error] 79-79: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (37)
packages/helpers/tsconfig.json (1)
1-8
: LGTM!The TypeScript configuration file is well-structured and follows best practices:
- Extending from
react-library.json
reuses base settings tailored for React library development.- Setting
jsx
toreact
enables the use of React components and features within the TypeScript files.- Including all files in the current directory and its subdirectories ensures that all relevant source files are processed by the TypeScript compiler.
- Excluding
dist
,build
, andnode_modules
directories optimizes the build process and prevents unnecessary compilation of output or dependency files.packages/editor/src/core/extensions/image/read-only-image.tsx (1)
18-20
: Great work on enhancing the image node with a custom view!Using
ReactNodeViewRenderer
with a customCustomImageNode
component is an excellent way to tailor the rendering and behavior of the image node. This approach allows for more flexibility in implementing features like resize handles, selection overlays, and other UI enhancements mentioned in the PR summary.The custom node view can greatly improve the user experience by providing visual cues and intuitive interactions. Keep up the good work!
packages/editor/src/core/extensions/image/image-extension-without-props.tsx (1)
20-22
: LGTM! The custom node view enhances image rendering flexibility.The addition of the
addNodeView
method and the use ofReactNodeViewRenderer
to render a customCustomImageNode
component is a great way to extend the functionality of the image extension.Benefits of using a custom node view:
- Allows for more control over how images are rendered within the editor.
- Enables the integration of custom properties and behaviors specific to the project's requirements.
- Promotes modularity by separating the image rendering logic into a dedicated component.
packages/helpers/package.json (5)
6-6
: LGTM!Updating the
main
entry point to target the compiled JavaScript file in thedist
directory follows the best practice for distributing packages.
7-8
: Great additions!The new
module
andtypes
fields improve the package's compatibility and usability:
- The
module
entry enables support for ES module syntax, aligning with modern JavaScript standards.- The
types
field ensures that TypeScript projects can properly consume this package by providing type definitions.
9-11
: Good practice!Defining the
files
array to include only thedist
directory is a great way to keep the package lightweight and ensure only necessary files are distributed.
12-14
: Excellent build script!The new
build
script efficiently automates the compilation process withtsup
:
- Generating both ESM and CommonJS formats ensures broad compatibility.
- Emitting type definitions is essential for TypeScript consumers.
- Minifying the output and excluding React from the bundle optimizes the package size and prevents potential issues.
18-19
: Appropriate dev dependencies!The updated
devDependencies
section looks good:
- Including
tsup
is necessary for the new build process.- Updating
typescript
ensures compatibility with the latest features and improvements.- Using caret version specifiers allows for seamless updates to compatible versions.
packages/editor/src/core/plugins/image/utils/validate-file.ts (1)
Line range hint
1-26
: LGTM!The addition of the
showAlert
parameter is a good enhancement. It provides flexibility to the caller to suppress alerts if needed, without affecting the core validation logic. The implementation looks correct and the changes are non-breaking.packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts (1)
1-54
: LGTM! Just one minor type suggestion.The custom read-only image extension is well-structured and follows the Tiptap extension API. The read-only nature of the extension is appropriate for scenarios where user interaction is not desired. The default attributes,
parseHTML
,renderHTML
, and storage mechanism are all implemented correctly. The integration of a React component ensures compatibility with React applications.Overall, this extension provides a clean and structured approach to display read-only images in Tiptap.
Define the generic type parameter explicitly instead of using
{}
.The static analysis hint correctly flags the usage of
{}
as a type. Using{}
is equivalent toany
and loses type safety.Define the generic type parameter explicitly like this:
-Image.extend<{}, UploadImageExtensionStorage>({ +Image.extend<Record<string, unknown>, UploadImageExtensionStorage>({
Record<string, unknown>
is a more explicit way to define an object type with string keys and values of any type.packages/editor/src/core/plugins/image/delete-image.ts (4)
8-8
: LGTM! The additionalnodeType
parameter makes the plugin more flexible.The modification to the
TrackImageDeletionPlugin
function signature to accept anodeType
parameter is a great improvement. It allows the plugin to handle different types of image nodes dynamically, making it more generic and adaptable to various contexts.This change eliminates the hardcoded reference to a specific image node type, enhancing the plugin's usability and flexibility.
10-10
: LGTM! The dynamic plugin key ensures uniqueness for each image node type.Updating the plugin key to include the
nodeType
parameter is a smart move. It ensures that each instance of the plugin has a unique key associated with the specific image node type it handles.This dynamic plugin key allows multiple instances of the plugin to coexist, each tracking deletions for a different image node type without conflicts. It enhances the plugin's flexibility and ensures proper functionality in various scenarios.
14-14
: LGTM! UsingnodeType
for identifying image nodes makes the plugin more versatile.Updating the logic to use the provided
nodeType
parameter for identifying image nodes in both the new and old document states is a great improvement. It allows the plugin to accurately track deletions for different types of image nodes based on the specifiednodeType
.This change makes the plugin more versatile and adaptable to various use cases, as it can now work with different image node types instead of being limited to a hardcoded type. It enhances the plugin's flexibility and ensures accurate tracking of image deletions.
Also applies to: 28-28
38-38
: LGTM! UsingnodeType
for accessing the storage structure enhances organization.Updating the storage mechanism to use the
nodeType
for accessing the appropriate storage structure is a valuable improvement. It allows the plugin to maintain separate storage for different types of image nodes, ensuring proper organization and tracking of deleted images based on their respective types.By using the
nodeType
to access the storage, the plugin prevents conflicts and maintains a clear separation between different image node types. This enhances the plugin's ability to handle multiple image types efficiently and keeps the storage well-structured.packages/editor/src/core/plugins/image/restore-image.ts (5)
8-10
: LGTM!The changes to the
TrackImageRestorationPlugin
function signature andPluginKey
generation enhance the plugin's flexibility and reusability for different image node types. This improves the plugin's integration within the editor's ecosystem.
14-14
: LGTM!Utilizing the
nodeType
parameter to identify image nodes makes the plugin more adaptable to different image node types. This is a positive change that enhances the plugin's flexibility.
25-25
: LGTM!Using the
nodeType
parameter to process newly added images ensures that the plugin works correctly with the specified image node type. This change enhances the plugin's adaptability.
32-39
: LGTM!Adjusting the handling of the deleted image set to reference the
nodeType
ensures that the plugin accesses the correct storage for deleted images based on the specified image node type. This change improves the plugin's compatibility with different image node types.
Line range hint
1-60
: Overall, the changes in this file are well-designed and enhance the plugin's functionality.The modifications make the plugin more generic and reusable for different image node types, improving its integration within the editor's ecosystem. The changes are consistent and do not introduce any apparent issues or side effects. Great work!
packages/editor/src/core/extensions/image/extension.tsx (4)
16-17
: LGTM!The keyboard shortcuts have been updated to use
this.name
, which improves adaptability if the node type name changes in the future. The changes look good.
22-23
: LGTM!The changes to pass
this.name
to theTrackImageDeletionPlugin
andTrackImageRestorationPlugin
look good. This likely improves the context in which these plugins operate.
30-30
: LGTM!The change to use
this.name
for checking the image node type looks good. This ensures consistency with the new dynamic naming approach.
65-67
: Verify the custom image node renderer's integration and testing.The addition of the
addNodeView
method to integrate a custom image node renderer usingReactNodeViewRenderer
andCustomImageNode
looks good. This allows for a more tailored rendering of image nodes, potentially improving the user experience.Please ensure that the custom renderer is thoroughly tested and handles all relevant use cases. Also, verify that it integrates well with other parts of the codebase, such as the
CustomImageNode
component.packages/editor/src/styles/drag-drop.css (3)
Line range hint
42-66
: LGTM!The changes to the selector for
.ProseMirror-selectednode
improve the precision of styling for non-image nodes during drag-and-drop operations. The use of a pseudo-element (::after) to apply styles such as the horizontal offset and background color enhances the visual feedback for different node types.
67-74
: LGTM!The introduction of new styles for nodes with classes
.node-imageComponent
and.node-image
enhances the visual distinction for image components during drag-and-drop interactions. The custom horizontal offset and background color applied via a pseudo-element (::after) provide clear visual cues for image nodes, improving the user experience.
108-109
: LGTM!The modification of the selector for image transitions to target nodes with classes
.node-imageComponent
and.node-image
allows for more granular control over the styling of image nodes. The transition effect applied specifically to image nodes provides visual feedback during interactions, enhancing the user experience.packages/editor/src/core/hooks/use-read-only-editor.ts (2)
61-61
: Preserve whitespace when setting editor content.The changes to the
setContent
method ensure that whitespace is preserved in its entirety when setting the initial editor value. This enhancement aligns with the PR objective of improving content management functionality in the editor.Note that preserving whitespace may affect how content is rendered or displayed, especially if the content relies on specific whitespace formatting. Ensure that this behavior is desired and thoroughly test the editor with various content scenarios to confirm the expected results.
71-71
: Maintain consistency in whitespace preservation.The changes to the
setEditorValue
method mirror the updates made to thesetContent
method in theuseEffect
hook. This consistency ensures that whitespace is preserved uniformly across different content-setting scenarios within the editor.Maintaining a consistent approach to whitespace handling enhances the predictability and reliability of the editor's behavior. It also aligns with the PR objective of improving content management functionality.
packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)
1-122
: Excellent work on theCustomImageNode
component!The component is well-structured and efficiently handles image uploads and rendering within the Prosemirror editor. The use of React hooks, such as
useState
,useEffect
,useRef
,useCallback
, anduseMemo
, demonstrates a solid understanding of state management and performance optimization techniques.The component effectively manages different upload scenarios (drop and insert) and updates the node's attributes accordingly. The conditional rendering logic ensures a seamless user experience by displaying either the
CustomImageBlock
orCustomImageUploader
based on the upload state.Overall, the implementation enhances the image upload functionality within the editor and provides a user-friendly experience.
Tools
Biome
[error] 79-79: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/editor/src/core/extensions/extensions.tsx (3)
15-15
: LGTM!The import statement for
CustomImageExtension
is syntactically correct and aligns with the PR objective of introducing a new custom image extension to enhance image upload functionality.
108-113
: LGTM!The configuration of
CustomImageExtension
with parameters fordelete
,restore
,upload
, andcancel
aligns with the PR objective of enhancing image management capabilities within the editor. The defensive programming practice of setting thecancel
parameter to an empty function ifcancelUploadImage
is not provided is appreciated.
83-83
: Verify the impact of removing theuploadFile
argument fromDropHandlerExtension
.The PR summary does not provide any context about the impact of this change. Please ensure that removing the
uploadFile
argument does not negatively affect the file upload functionality when files are dropped into the editor, especially for images.Run the following script to verify the usage of
DropHandlerExtension
:Verification successful
Verification successful: Removal of
uploadFile
argument is consistent with implementationThe removal of the
uploadFile
argument fromDropHandlerExtension
is consistent with its implementation and usage across the codebase. The extension is defined and called without any parameters, indicating that the change is intentional and part of a larger refactoring effort.File upload functionality still exists in the codebase, but it's not directly linked to the
DropHandlerExtension
. This suggests that the responsibility for file uploads has been moved elsewhere, improving separation of concerns.Based on the available evidence, this change does not appear to negatively affect the file upload functionality when files are dropped into the editor.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `DropHandlerExtension` and its impact on file uploads. # Test 1: Search for the usage of `DropHandlerExtension`. Expect: No occurrences of `DropHandlerExtension` with the `uploadFile` argument. rg --type typescript -A 5 $'DropHandlerExtension' # Test 2: Search for the implementation of `DropHandlerExtension`. Expect: The implementation should handle file uploads without the `uploadFile` argument. ast-grep --lang typescript --pattern $'DropHandlerExtension($$$) { $$$ }'Length of output: 618
Script:
#!/bin/bash # Description: Verify the usage and implementation of `DropHandlerExtension` # Search for the usage of `DropHandlerExtension` echo "Searching for DropHandlerExtension usage:" rg -A 5 "DropHandlerExtension" # Search for the definition of DropHandlerExtension echo -e "\nSearching for DropHandlerExtension definition:" rg -A 10 "function DropHandlerExtension" # Use ast-grep to find function declarations named DropHandlerExtension echo -e "\nSearching for DropHandlerExtension function declaration:" ast-grep --lang javascript --pattern "function DropHandlerExtension" # Search for any mentions of file upload functionality echo -e "\nSearching for file upload related code:" rg -i "file.?upload"Length of output: 3397
packages/editor/src/core/components/menus/block-menu.tsx (1)
104-105
: LGTM!The additional condition to disable the "Duplicate" action when the editor is active on an "imageComponent" enhances the control flow and user experience. The logic is correct, and the implementation is accurate.
packages/editor/src/core/extensions/custom-image/custom-image.ts (2)
1-157
: Excellent implementation of the custom image extension!The custom image extension is well-structured and follows best practices for extending the Tiptap editor. It provides a user-friendly experience for uploading images via drag-and-drop or insertion methods.
Key highlights:
- The
setImageUpload
command effectively handles image uploads based on the event type and generates unique identifiers for each image.- The
uploadImage
command asynchronously uploads image files and returns their URLs.- The extension specifies appropriate attributes for the custom image component, such as width, height, and an ID for tracking purposes.
- The storage mechanism efficiently manages the mapping of uploaded files.
- The React node view renderer ensures seamless integration of the custom image component into the editor interface.
Overall, this extension significantly enhances the editor's image handling capabilities while maintaining a clean and maintainable codebase.
32-32
: Explicitly define the object shape.The past review comment suggests explicitly defining the object shape instead of using
{}
as a type.{}
means "any non-nullable value", which is not a good practice.Explicitly define the object shape to improve the type safety and readability of the code. For example:
-Image.extend<Record<string, unknown>, UploadImageExtensionStorage>({ +Image.extend<{ /* define the object shape here */ }, UploadImageExtensionStorage>({packages/editor/src/styles/editor.css (2)
125-125
: LGTM!The
margin-top: 0 !important
rule for images within the.ProseMirror
class is correct and ensures that images do not have any top margin in the editor.
128-139
: LGTM!The conditional hover effect for images is implemented correctly:
- Images without the
.read-only-image
class have a brightness filter applied on hover, with a smooth transition.- Selected images (with the
.ProseMirror-selectednode
class) have a blue outline and brightness filter.The CSS rules are well-structured and achieve the desired visual effects for image interaction within the editor.
What's new?
This PR adds a new custom image extension to handle image uploads.
Improvements:
Media:
Screen.Recording.2024-09-13.at.16.18.35.mov
Plane issue: WEB-2450
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
These updates aim to enhance the user experience and functionality of the document editor.