[WIKI-471] refactor: custom image extension#7247
Conversation
WalkthroughThe changes introduce a comprehensive refactor of the custom image extension for a rich text editor. They modularize types, utilities, and extension logic, replace previous read-only and editable image extensions with a unified approach, update component props and imports, and streamline extension configuration and registration throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor
participant CustomImageExtension
participant FileHandler
participant ReactComponent
Editor->>CustomImageExtension: Initialize with { fileHandler, isEditable }
CustomImageExtension->>FileHandler: Validate file, getImageSource, uploadImage (if supported)
CustomImageExtension->>CustomImageExtension: Manage storage (fileMap, deletedImageSet)
CustomImageExtension->>ReactComponent: Render node view with attributes, handlers
ReactComponent->>CustomImageExtension: Request image source, restore image, upload image
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/editor/src/core/extensions/custom-image/extension-config.ts (1)
22-34: Consider performance optimization for attribute merging.The
reduceoperation to merge attributes is correct but could be optimized for better performance, especially ifECustomImageAttributeNamescontains many values.Consider this more efficient approach:
addAttributes() { - const attributes = { - ...this.parent?.(), - ...Object.values(ECustomImageAttributeNames).reduce((acc, value) => { - acc[value] = { - default: DEFAULT_CUSTOM_IMAGE_ATTRIBUTES[value], - }; - return acc; - }, {}), - }; + const customAttributes = Object.fromEntries( + Object.values(ECustomImageAttributeNames).map(value => [ + value, + { default: DEFAULT_CUSTOM_IMAGE_ATTRIBUTES[value] } + ]) + ); + + const attributes = { + ...this.parent?.(), + ...customAttributes, + }; return attributes; },packages/editor/src/core/extensions/custom-image/utils.ts (1)
20-33: Improve type safety for pixel string normalization.The function handles various input types well, but the type assertion could be more robust.
Consider making the return type more explicit:
export const ensurePixelString = <TDefault>( value: Pixel | TDefault | number | undefined | null, defaultValue?: TDefault -) => { +): Pixel | TDefault => { if (!value || value === defaultValue) { return defaultValue; } if (typeof value === "number") { - return `${value}px` satisfies Pixel; + return `${value}px` as Pixel; } - return value; + return value as Pixel; };packages/editor/src/core/extensions/custom-image/types.ts (1)
31-31: Union type design could be more explicit.The
UploadEntitytype combines event-specific properties with optional properties. Consider using discriminated unions for better type safety.-export type UploadEntity = ({ event: "insert" } | { event: "drop"; file: File }) & { hasOpenedFileInputOnce?: boolean }; +export type UploadEntity = + | { event: "insert"; hasOpenedFileInputOnce?: boolean } + | { event: "drop"; file: File; hasOpenedFileInputOnce?: boolean };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/editor/src/ce/types/storage.ts(1 hunks)packages/editor/src/core/extensions/custom-image/components/block.tsx(7 hunks)packages/editor/src/core/extensions/custom-image/components/index.ts(0 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(2 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx(5 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx(1 hunks)packages/editor/src/core/extensions/custom-image/components/uploader.tsx(4 hunks)packages/editor/src/core/extensions/custom-image/custom-image.ts(0 hunks)packages/editor/src/core/extensions/custom-image/extension-config.ts(1 hunks)packages/editor/src/core/extensions/custom-image/extension.ts(1 hunks)packages/editor/src/core/extensions/custom-image/index.ts(0 hunks)packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts(0 hunks)packages/editor/src/core/extensions/custom-image/types.ts(1 hunks)packages/editor/src/core/extensions/custom-image/utils.ts(1 hunks)packages/editor/src/core/extensions/extensions.ts(2 hunks)packages/editor/src/core/extensions/image/extension.tsx(3 hunks)packages/editor/src/core/extensions/image/index.ts(0 hunks)packages/editor/src/core/extensions/image/read-only-image.tsx(0 hunks)packages/editor/src/core/extensions/index.ts(0 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(2 hunks)
💤 Files with no reviewable changes (7)
- packages/editor/src/core/extensions/image/index.ts
- packages/editor/src/core/extensions/custom-image/index.ts
- packages/editor/src/core/extensions/custom-image/components/index.ts
- packages/editor/src/core/extensions/index.ts
- packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts
- packages/editor/src/core/extensions/image/read-only-image.tsx
- packages/editor/src/core/extensions/custom-image/custom-image.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (39)
packages/editor/src/ce/types/storage.ts (1)
5-5: LGTM! Improved type organization.The import path change from a general extension import to a dedicated types module improves code organization and separation of concerns. This modularization makes the codebase more maintainable and the imports more explicit.
packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (2)
2-5: LGTM! Improved type safety and organization.The changes enhance the codebase by:
- Using a centralized
TCustomImageAttributestype instead of inline object types- Improving import categorization with clearer comments
- Centralizing type definitions for better maintainability
This refactoring provides better type safety and a single source of truth for image attributes.
10-10: Good type consistency improvement.Replacing the inline object type with
TCustomImageAttributesensures type consistency across components and makes the code more maintainable when image attribute structure changes.packages/editor/src/core/extensions/extensions.ts (2)
40-41: LGTM! Better import organization.The addition of local imports section and clearer categorization improves code readability and maintainability.
195-201: Verify the new extension instantiation pattern is consistent.The changes to use object parameters for both
ImageExtensionandCustomImageExtensionimprove API consistency and extensibility. TheisEditableparameter inCustomImageExtensionsuggests a unified approach for handling editable/read-only modes.Please verify that this new instantiation pattern is consistently implemented across all extension usage:
#!/bin/bash # Description: Verify consistent extension instantiation patterns # Search for ImageExtension and CustomImageExtension usage patterns echo "=== ImageExtension usage patterns ===" rg -A 3 "ImageExtension\(" echo -e "\n=== CustomImageExtension usage patterns ===" rg -A 3 "CustomImageExtension\("packages/editor/src/core/extensions/read-only-extensions.ts (2)
26-26: LGTM! Consistent import organization.The import additions align with the architectural refactoring to use unified extensions for both editable and read-only modes, which is a good consolidation approach.
Also applies to: 34-35
139-145: Excellent architectural improvement.Replacing separate read-only extensions with unified extensions configured via
isEditable: falseis a significant improvement that:
- Reduces code duplication
- Ensures consistency between editable and read-only behaviors
- Simplifies maintenance by having a single extension implementation
- Follows the DRY principle effectively
The instantiation pattern is consistent with the changes in
extensions.ts.packages/editor/src/core/extensions/custom-image/extension-config.ts (3)
1-7: LGTM! Well-organized imports.The import structure clearly separates TipTap dependencies, constants, and local imports, following good organizational practices.
9-15: Good TypeScript module augmentation.The command declaration properly extends the TipTap Commands interface, enabling type-safe usage of the
insertImageComponentcommand throughout the codebase.
36-46: Here’s the replacement logic so we can confirm how<image-component>is transformed and whetheralt(or other a11y attributes) are applied:#!/bin/bash echo "=== Snippet around image-component replacement in web/core/hooks/use-parse-editor-content.ts ===" rg -n "image-component" -A5 -B5 web/core/hooks/use-parse-editor-content.tspackages/editor/src/core/extensions/custom-image/extension.ts (3)
15-18: LGTM! Clean props interface design.The props interface clearly defines the required dependencies and supports both read-only and editable modes effectively.
110-115: LGTM! Clean keyboard shortcut implementation.The keyboard shortcuts for inserting empty paragraphs around images provide good UX for navigation.
29-38: ```shell
#!/bin/bashDisplay the custom-image types file
sed -n '1,200p' packages/editor/src/core/extensions/custom-image/types.ts
Search specifically for uploadImage in the types file
grep -n "uploadImage" -n packages/editor/src/core/extensions/custom-image/types.ts
Locate TFileHandler definition
grep -R "export type TFileHandler" -n packages/editor/src
```shell #!/bin/bash # As a fallback, search for TFileHandler in absolute imports grep -R "interface TFileHandler" -n .packages/editor/src/core/extensions/custom-image/utils.ts (1)
9-15: LGTM! Well-structured default attributes.The use of enums for attribute names provides good type safety, and the default values are reasonable for image components.
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (2)
6-12: LGTM! Improved type safety with centralized types.The switch to
TCustomImageAttributesimproves consistency across the codebase and provides better type safety.
31-31: Good defensive programming with nullish coalescing.The explicit null handling with fallback values (aspect ratio fallback to 1, src fallback to undefined) prevents runtime errors and improves robustness.
Also applies to: 42-42, 208-208, 214-214, 244-244
packages/editor/src/core/extensions/image/extension.tsx (3)
15-23: LGTM! Clean props interface and extraction.The simplified props interface and clear extraction of
getAssetSrcimproves readability and consistency with the custom image extension pattern.
41-47: Good defensive programming for optional properties.The safe access to
fileHandler.validation?.maxFileSizewith fallback to 0 prevents runtime errors when validation is not available.
66-66: ```shell
#!/bin/bashInspect ImageExtension definition and default options
sed -n '1,200p' packages/editor/src/core/extensions/image/extension.tsx
</details> <details> <summary>packages/editor/src/core/extensions/custom-image/components/uploader.tsx (4)</summary> `13-15`: **LGTM! Cleaner import organization.** The switch to local relative imports aligns with the removal of the centralized exports and improves module organization. --- `17-22`: **Good type interface evolution.** The addition of `failedToLoadImage` property and reordering improves the interface structure for better error handling. --- `80-83`: **LGTM! Proper extension method usage.** The switch from `editor.commands.uploadImage` to `extension.options.uploadImage` aligns with the new extension architecture where upload functionality is provided through options. --- `76-78`: **Good improvement to dependency arrays.** The addition of missing dependencies like `imageEntityId` and `editor.isEditable` improves hook correctness and prevents stale closure issues. Also applies to: 133-133, 168-168 </details> <details> <summary>packages/editor/src/core/extensions/custom-image/components/node-view.tsx (5)</summary> `7-10`: **LGTM! Well-organized imports with clear separation.** The local imports are properly separated and the new type imports align with the refactoring objectives. --- `12-21`: **Strong type safety improvements with explicit extension typing.** The new `CustomImageNodeViewProps` type provides better type safety by explicitly requiring the `CustomImageExtension` type for the extension prop, replacing the generic `NodeViewProps["extension"]`. --- `23-24`: **Component rename aligns with PR objectives.** The rename from `CustomImageNode` to `CustomImageNodeView` better reflects the component's purpose as a node view implementation. --- `67-82`: **Simplified prop passing reduces boilerplate but verify component contracts.** The spread operator approach (`{...props}`) simplifies prop passing, but ensure that `CustomImageBlock` and `CustomImageUploader` components can handle all props from `CustomImageNodeViewProps`. The change from individual prop passing to spreading all props could introduce unintended prop conflicts. Please verify that the child components properly type their expected props. ```shell #!/bin/bash # Description: Check CustomImageBlock and CustomImageUploader prop definitions # Expected: Components should properly type their expected props # Search for CustomImageBlock and CustomImageUploader prop type definitions ast-grep --pattern 'type $_ = $_ & { $$$ }' # Also check component function signatures ast-grep --pattern 'export const CustomImageBlock: React.FC<$_> = ($_) => {' ast-grep --pattern 'export const CustomImageUploader: React.FC<$_> = ($_) => {'
54-61: Let’s inspect theTFileHandlerand its baseTReadOnlyFileHandlerinconfig.tsto see whatgetAssetSrcactually returns:#!/bin/bash # Show TFileHandler and its parent type to verify getAssetSrc signature rg -n -A5 'export type TFileHandler' packages/editor/src/core/types/config.ts rg -n -A5 'TReadOnlyFileHandler' packages/editor/src/core/types/config.tspackages/editor/src/core/extensions/custom-image/types.ts (4)
5-11: Well-structured enum with descriptive naming.The
ECustomImageAttributeNamesenum provides type-safe attribute name constants, improving maintainability and reducing magic strings.
13-16: Clever pixel type definition with template literal types.The
Pixeltype using template literal types ensures type safety for CSS pixel values, andPixelAttributeallows for flexible default values.
39-43: Extension options provide good separation of concerns.The
CustomImageExtensionOptionstype clearly defines the required handlers with appropriate optional properties, following good architectural patterns.
45-49: Storage type structure supports the extension lifecycle well.The
CustomImageExtensionStoragetype appropriately usesMapfor tracking files and deleted images, with a clearmaxFileSizeproperty.packages/editor/src/core/extensions/custom-image/components/block.tsx (7)
6-9: Good modularization with centralized imports.The refactor to import types and utilities from centralized modules improves maintainability and reduces code duplication.
14-20: Props interface properly extends the node view props.The
CustomImageBlockPropsextension ofCustomImageNodeViewPropsmaintains type consistency across the component hierarchy.
27-35: Destructured props alignment with new interface.The destructuring properly extracts the new props structure, maintaining clean separation of concerns.
39-43: State initialization uses centralized utility function.The
ensurePixelStringutility usage provides consistent pixel value handling across the component.
145-145: Callback dependencies correctly updated.The
handleResizeEndcallback properly includes theupdateAttributesSafelydependency, maintaining correct closure behavior.
219-230: Proper refactoring to extension options pattern.The change from
editor.commands.restoreImagetoextension.options.restoreImagealigns with the new architecture, centralizing image handling through extension options.
265-265: Simplified toolbar prop passing with full attributes.Passing the entire
node.attrsobject toImageToolbarRootas theimageprop simplifies the interface and ensures all attributes are available.
packages/editor/src/core/extensions/custom-image/components/block.tsx
Outdated
Show resolved
Hide resolved
* refactor: custom image extension * refactor: extension config * revert: image full screen component * fix: undo operation
3 participants
Description
This PR refactors the existing image extensions, by-
Type of Change
Summary by CodeRabbit
New Features
Refactor
Removals