Skip to content

[WIKI-449] feat: image block download and alignment options #7254

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

Merged
merged 14 commits into from
Jul 2, 2025

Conversation

aaryan610
Copy link
Member

@aaryan610 aaryan610 commented Jun 23, 2025

Description

This PR adds new features to the image block in the editor-

  1. Download option.
  2. Alignment option.
  3. Image specific options on context menu.

Type of Change

  • Feature (non-breaking change which adds functionality)

Media

Screen.Recording.2025-06-23.at.15.11.07.mov

Summary by CodeRabbit

  • New Features

    • Added support for image alignment (left, center, right) with alignment-aware resizing and positioning.
    • Introduced image download functionality directly from the editor toolbar.
    • Added full-screen image viewing mode with zoom and download options.
    • Enhanced image toolbar with alignment, download, and full-screen controls.
  • Improvements

    • Refined image insertion and source management for improved handling of file and download URLs.
    • Improved accessibility and user experience for image toolbar actions.
  • Chores

    • Added new types and updated dependencies to support enhanced image features.
    • Added an editor-portal container for modal rendering in the app layout.

Copy link
Contributor

coderabbitai bot commented Jun 23, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This update introduces comprehensive enhancements to the custom image extension in the editor, adding alignment and download support, refactoring toolbar and fullscreen components, and expanding type definitions. It also updates file handler interfaces and editor hooks to provide download URLs, reorganizes component structures, and adds a DOM portal for fullscreen modals.

Changes

File(s) Change Summary
.../custom-image/components/block.tsx, .../node-view.tsx Added support for image alignment and download source, refactored props/types, updated resizing and toolbar logic, reorganized component structure.
.../custom-image/components/toolbar/alignment.tsx, .../download.tsx Introduced new toolbar components for image alignment and download actions, including dropdown and button UI.
.../custom-image/components/toolbar/root.tsx Refactored toolbar root: flattened/expanded props, updated children, switched from fullscreen to toolbar visibility state.
.../custom-image/components/toolbar/full-screen/modal.tsx Renamed and refactored fullscreen modal, flattened props, improved zoom/download controls, separated internal modal logic.
.../custom-image/components/toolbar/full-screen/root.tsx, .../index.ts Added new root component for fullscreen action, managing modal state and toolbar interaction; added re-export index.
.../custom-image/extension.ts, .../types.ts, .../utils.ts Added/expanded types for image attributes, alignment, and extension options; updated extension logic for alignment, download, and validation.
.../image/extension-config.tsx Introduced image extension config with default attributes for width, height, aspect ratio, and alignment.
.../types/config.ts, .../hooks/editor/use-editor-config.ts Added getAssetDownloadSrc method to file handler types and hook, providing download URLs for assets.
.../package.json Added @plane/hooks as a new dependency.
.../app/layout.tsx (space/, web/) Added <div id="editor-portal" /> to layout for fullscreen modal portal support.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Editor
    participant CustomImageExtension
    participant FileHandler
    participant CustomImageBlock
    participant Toolbar
    participant FullScreenModal

    User->>Editor: Insert or interact with image
    Editor->>CustomImageExtension: Validate and insert image node
    CustomImageExtension->>FileHandler: getImageSource / getImageDownloadSource
    FileHandler-->>CustomImageExtension: Return image src and download src
    CustomImageExtension->>CustomImageBlock: Render with src, downloadSrc, alignment
    CustomImageBlock->>Toolbar: Show toolbar (alignment, download, fullscreen)
    User->>Toolbar: Change alignment / Download / Fullscreen
    Toolbar->>CustomImageBlock: Update alignment or trigger download/fullscreen
    Toolbar->>FullScreenModal: Open modal with image and download controls
Loading

Suggested labels

ready_to_merge

Suggested reviewers

  • anmolsinghbhatia
  • Palanikannan1437

Poem

In the meadow where images align,
Rabbits hop with code so fine.
With download links and toolbars bright,
Fullscreen dreams take off in flight.
Alignment left, center, or right—
Every pixel’s just delight!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5358617 and bf30383.

📒 Files selected for processing (1)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (4 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

makeplane bot commented Jun 23, 2025

Pull Request Linked with Plane Work Items

Comment Automatically Generated by Plane

@aaryan610 aaryan610 marked this pull request as ready for review June 23, 2025 09:42
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (7)
packages/utils/src/editor.ts (1)

25-38: Improve JSDoc documentation specificity.

The JSDoc comment is generic and doesn't clearly indicate this function generates download URLs. Consider making it more specific.

 /**
- * @description generate the file source using assetId
+ * @description generate the download URL for an asset using assetId  
  * @param {TEditorSrcArgs} args
  */
packages/editor/src/core/extensions/custom-image/components/toolbar/download.tsx (1)

5-7: Consider adding prop validation for better type safety.

The src prop should be validated to ensure it's a valid URL string to prevent runtime issues.

 type Props = {
-  src: string;
+  src: string;
 };
+
+// Add validation helper
+const isValidUrl = (url: string): boolean => {
+  try {
+    new URL(url);
+    return true;
+  } catch {
+    return false;
+  }
+};
web/core/hooks/editor/use-editor-config.ts (1)

36-49: Consider refactoring to eliminate code duplication and improve error handling.

The logic is nearly identical to the existing getAssetSrc method. Also, the function doesn't need to be async since it's not awaiting anything.

+        // Helper function to reduce duplication
+        const resolveAssetPath = (path: string, isDownload: boolean = false) => {
+          if (!path) return "";
+          if (path?.startsWith("http")) {
+            return path;
+          } else {
+            const resolver = isDownload ? getEditorAssetDownloadSrc : getEditorAssetSrc;
+            return resolver({
+              assetId: path,
+              projectId,
+              workspaceSlug,
+            }) ?? "";
+          }
+        },
+
-        getAssetDownloadSrc: async (path) => {
-          if (!path) return "";
-          if (path?.startsWith("http")) {
-            return path;
-          } else {
-            return (
-              getEditorAssetDownloadSrc({
-                assetId: path,
-                projectId,
-                workspaceSlug,
-              }) ?? ""
-            );
-          }
-        },
+        getAssetDownloadSrc: async (path) => resolveAssetPath(path, true),
         getAssetSrc: async (path) => {
-          if (!path) return "";
-          if (path?.startsWith("http")) {
-            return path;
-          } else {
-            return (
-              getEditorAssetSrc({
-                assetId: path,
-                projectId,
-                workspaceSlug,
-              }) ?? ""
-            );
-          }
+          return resolveAssetPath(path, false);
         },
packages/editor/src/core/extensions/custom-image/extension-config.ts (1)

36-42: Document the custom tag usage.

The extension uses a custom image-component tag instead of the standard img tag. Consider adding a comment to explain this design decision.

  parseHTML() {
    return [
      {
+       // Custom tag for image components with enhanced features (alignment, download, etc.)
        tag: "image-component",
      },
    ];
  },
packages/editor/src/core/extensions/image/extension.tsx (1)

40-40: Make the validation check more robust.

The current check only verifies if the validation property exists but not if it's defined.

-      const maxFileSize = "validation" in fileHandler ? fileHandler.validation?.maxFileSize : 0;
+      const maxFileSize = "validation" in fileHandler && fileHandler.validation?.maxFileSize 
+        ? fileHandler.validation.maxFileSize 
+        : 0;
packages/editor/src/core/extensions/custom-image/utils.ts (1)

22-35: Consider simplifying the ensurePixelString function logic.

The function handles multiple cases but the logic flow could be more explicit, especially for the null/undefined handling.

 export const ensurePixelString = <TDefault>(
   value: Pixel | TDefault | number | undefined | null,
   defaultValue?: TDefault
 ) => {
-  if (!value || value === defaultValue) {
+  // Return default for null, undefined, or when value equals default
+  if (value == null || value === defaultValue) {
     return defaultValue;
   }

   if (typeof value === "number") {
     return `${value}px` satisfies Pixel;
   }

   return value;
 };
packages/editor/src/core/extensions/custom-image/components/uploader.tsx (1)

76-78: Review dependency array update.

The dependency array for the onUpload callback was updated but still uses an eslint-disable comment. Consider if all dependencies are properly included.

-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [imageComponentImageFileMap, imageEntityId, updateAttributes, getPos]
+    [imageComponentImageFileMap, imageEntityId, updateAttributes, getPos, editor, setIsUploaded]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7045a1f and d98162b.

📒 Files selected for processing (30)
  • packages/editor/src/ce/types/storage.ts (1 hunks)
  • packages/editor/src/core/extensions/core-without-props.ts (2 hunks)
  • packages/editor/src/core/extensions/custom-image/components/block.tsx (6 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/alignment.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/download.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (6 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-config.tsx (1 hunks)
  • packages/editor/src/core/extensions/image/extension.tsx (2 hunks)
  • packages/editor/src/core/extensions/image/image-component-without-props.tsx (0 hunks)
  • packages/editor/src/core/extensions/image/index.ts (1 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)
  • packages/editor/src/core/helpers/editor-commands.ts (1 hunks)
  • packages/editor/src/core/hooks/use-outside-click-detector.ts (1 hunks)
  • packages/editor/src/core/types/config.ts (1 hunks)
  • packages/utils/src/editor.ts (1 hunks)
  • web/core/hooks/editor/use-editor-config.ts (2 hunks)
💤 Files with no reviewable changes (7)
  • packages/editor/src/core/extensions/custom-image/index.ts
  • packages/editor/src/core/extensions/index.ts
  • packages/editor/src/core/extensions/custom-image/components/index.ts
  • packages/editor/src/core/extensions/image/read-only-image.tsx
  • packages/editor/src/core/extensions/image/image-component-without-props.tsx
  • packages/editor/src/core/extensions/custom-image/read-only-custom-image.ts
  • packages/editor/src/core/extensions/custom-image/custom-image.ts
🔇 Additional comments (27)
packages/utils/src/editor.ts (1)

29-37: LGTM! Consistent implementation for download URLs.

The function correctly mirrors the existing getEditorAssetSrc pattern while adding the /download/ path segment for asset downloads. The logic properly handles both project-scoped and workspace-scoped assets.

packages/editor/src/core/hooks/use-outside-click-detector.ts (1)

8-21: LGTM! Solid outside click detection logic.

The click detection logic is well-implemented with proper DOM traversal and the data-prevent-outside-click attribute check provides good flexibility for preventing unwanted callback triggers.

packages/editor/src/ce/types/storage.ts (1)

5-5: LGTM! Import path updated for better modularity.

The import path change aligns with the new modular structure where types have been extracted into dedicated files, improving code organization.

packages/editor/src/core/helpers/editor-commands.ts (1)

6-6: LGTM! Good use of type-only import.

The change to use a type-only import from the new modular types file follows TypeScript best practices and improves build performance.

packages/editor/src/core/types/config.ts (1)

6-6: LGTM! Well-designed interface extension.

The new getAssetDownloadSrc method follows the established pattern of other methods in the interface and properly supports the new download functionality with appropriate async typing.

packages/editor/src/core/extensions/image/index.ts (1)

2-2: LGTM! Clean modularization of image extension exports.

The addition of the extension-config export aligns well with the broader refactoring to introduce stronger typing and modular architecture for image extensions.

packages/editor/src/core/extensions/image/extension-config.tsx (1)

6-24: Excellent refactoring to introduce stronger typing.

The generic typing with Pick for options and proper storage typing significantly improves type safety while preserving all existing functionality. The default attribute values are sensible defaults for image handling.

packages/editor/src/core/extensions/extensions.ts (2)

40-41: Good refactoring to use local import for better modularity.

Using a local import for CustomImageExtension improves code organization and enables better tree-shaking compared to importing from the general extensions barrel file.


195-202: LGTM! API refactoring improves extensibility.

The change from passing individual parameters to options objects makes the API more extensible and consistent. The isEditable flag for CustomImageExtension allows proper behavior differentiation between editable and read-only contexts.

packages/editor/src/core/extensions/core-without-props.ts (1)

75-76: Verify that HTML attributes are preserved in the new configuration.

The previous implementation configured HTML attributes for the image extension. Please ensure that the new ImageExtensionConfig and CustomImageExtensionConfig handle these attributes appropriately, or if they were intentionally removed, document the reasoning.

#!/bin/bash
# Description: Check if ImageExtensionConfig includes HTML attributes configuration

# Search for HTMLAttributes in the new image extension config files
ast-grep --pattern 'HTMLAttributes' packages/editor/src/core/extensions/image/extension-config.tsx packages/editor/src/core/extensions/image/index.ts

# Also check the base implementation
rg -A 5 "HTMLAttributes|addAttributes" packages/editor/src/core/extensions/image/extension-config.tsx
packages/editor/src/core/extensions/read-only-extensions.ts (1)

139-146: LGTM! The refactoring to use configurable extensions is implemented correctly.

The changes properly instantiate the image extensions with the required props, including the isEditable: false flag for read-only mode.

packages/editor/src/core/extensions/custom-image/components/toolbar/alignment.tsx (1)

1-65: LGTM! Well-structured alignment component with proper accessibility.

The ImageAlignmentAction component is well-implemented with:

  • Proper TypeScript typing and interface design
  • Effective use of React hooks for state management
  • Accessibility features with tooltips
  • Clean UI implementation with proper styling

The outside click detection and parent notification via toggleToolbarViewStatus are handled correctly.

packages/editor/src/core/extensions/custom-image/components/uploader.tsx (2)

17-22: ```shell
#!/bin/bash

Search for CustomImageUploaderProps across all .ts and .tsx files

rg "CustomImageUploaderProps" -g '.ts' -g '.tsx' -n -A3


---

`80-83`: I’ll re-run the search for `uploadImage` across all TypeScript/TSX files to locate where it’s defined in the extension’s options interface:


```shell
#!/bin/bash
# Search for 'uploadImage' definitions/usages in .ts/.tsx files
rg -A3 -B3 "uploadImage" --glob "*.ts*" .
packages/editor/src/core/extensions/custom-image/extension.ts (3)

55-109: Excellent command implementation with proper validation.

The insertImageComponent command is well-implemented with:

  • Early validation of dropped files using isFileValid
  • Proper error handling with user feedback
  • UUID generation for unique file tracking
  • Correct file map management for both drop and insert events
  • Flexible position handling for content insertion

The validation logic properly checks MIME types and file size limits before proceeding.


111-116: LGTM! Proper keyboard shortcuts for navigation.

The keyboard shortcuts for ArrowUp/ArrowDown properly handle paragraph insertion at node boundaries using the insertEmptyParagraphAtNodeBoundaries helper function.


29-39: I’ll re-run the search without the incorrect --type typescript flag and look for both optional chaining and direct calls to uploadImage across .ts/.tsx files:

#!/bin/bash
# Look for optional chaining on uploadImage
rg -A5 -B5 "uploadImage\\?\\." -g "*.ts" -g "*.tsx"

# Look for any direct calls to uploadImage
rg -A5 -B5 "uploadImage\\(" -g "*.ts" -g "*.tsx"
packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (3)

28-34: LGTM! Improved toolbar visibility management.

The conditional styling properly manages toolbar visibility using the shouldShowToolbar state along with hover states. The opacity and pointer events are correctly toggled.


10-18: ```shell
#!/bin/bash

Display extended context for ImageToolbarRoot usage

rg -C10 "<ImageToolbarRoot" packages/editor/src/core/extensions/custom-image/components/block.tsx


---

`35-42`: ```bash
#!/bin/bash
# Find full-screen.tsx and search for toggleToolbarViewStatus usage
files=$(fd -e tsx full-screen.tsx)
if [ -z "$files" ]; then
  echo "No full-screen.tsx file found"
  exit 0
fi

for f in $files; do
  echo "=== File: $f ==="
  rg -n "toggleToolbarViewStatus" "$f" || echo "No occurrences in $f"
done
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (4)

22-26: LGTM! Proper internal state management.

The component now manages its own fullscreen state internally with useState, which is more appropriate than relying on external state management. This makes the component more self-contained and reusable.


191-194: Excellent external notification pattern.

The useEffect hook properly notifies the parent component of fullscreen status changes via the toggleToolbarViewStatus callback. This maintains proper separation of concerns while enabling external coordination.


262-274: LGTM! Improved accessibility with tooltip.

The fullscreen toggle button is now properly wrapped with a Tooltip component, improving accessibility and user experience. The button styling and event handling are correct.


170-172: Good simplification of dependency array.

The handleWheel callback dependency array was simplified by removing unused dependencies, which is good for performance and clarity.

packages/editor/src/core/extensions/custom-image/types.ts (1)

1-57: Well-structured type definitions!

The type definitions are comprehensive and provide excellent type safety with good use of enums, template literal types, and clear interfaces.

packages/editor/src/core/extensions/custom-image/components/block.tsx (2)

100-101: Verify TypeScript version supports the satisfies operator.

The satisfies operator requires TypeScript 4.9 or higher. Please ensure the project's TypeScript version meets this requirement.

#!/bin/bash
# Description: Check TypeScript version in the project

# Check package.json files for TypeScript version
fd -e json -x grep -l "typescript" {} \; | xargs grep -A1 -B1 '"typescript"' || echo "TypeScript not found in package.json files"

# Check for TypeScript config files
fd "tsconfig" -e json | head -5

237-264: Refine error handling logic to track restoration attempts accurately.

The hasTriedRestoringImageOnce flag is set to true in the finally block even when restoration wasn't attempted (when extension.options.restoreImage doesn't exist). This could incorrectly prevent future restoration attempts.

Apply this diff to track restoration attempts more accurately:

 onError={async (e) => {
   // for old image extension this command doesn't exist or if the image failed to load for the first time
-  if (!extension.options.restoreImage || hasTriedRestoringImageOnce) {
+  if (!extension.options.restoreImage) {
+    setFailedToLoadImage(true);
+    return;
+  }
+  
+  if (hasTriedRestoringImageOnce) {
     setFailedToLoadImage(true);
     return;
   }

+  setHasTriedRestoringImageOnce(true);
   try {
     setHasErroredOnFirstLoad(true);
     // this is a type error from tiptap, don't remove await until it's fixed
     if (!imgNodeSrc) {
       throw new Error("No source image to restore from");
     }
     await extension.options.restoreImage?.(imgNodeSrc);
     if (!imageRef.current) {
       throw new Error("Image reference not found");
     }
     if (!resolvedImageSrc) {
       throw new Error("No resolved image source available");
     }
     imageRef.current.src = resolvedImageSrc;
   } catch {
     // if the image failed to even restore, then show the error state
     setFailedToLoadImage(true);
     console.error("Error while loading image", e);
   } finally {
     setHasErroredOnFirstLoad(false);
-    setHasTriedRestoringImageOnce(true);
   }
 }}

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx (1)

6-11: Remove unused height property from Props type and destructuring.

The height property is defined in the Props type and destructured from the image object but is never used in this component. The modal component also doesn't receive or use this property.

 type Props = {
   image: {
     src: string;
-    height: string;
     width: string;
     aspectRatio: number;
   };
   isOpen: boolean;
   toggleFullScreenMode: (val: boolean) => void;
 };

 export const ImageFullScreenActionRoot: React.FC<Props> = (props) => {
   const { image, isOpen: isFullScreenEnabled, toggleFullScreenMode } = props;
   // derived values
   const { src, width, aspectRatio } = image;

Also applies to: 17-19

packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1)

120-135: Optimize drag performance with requestAnimationFrame.

The current drag implementation updates on every mouse move event, which can cause performance issues. Consider using requestAnimationFrame for smoother performance.

+  const rafRef = useRef<number | null>(null);
+
   const handleMouseMove = useCallback(
     (e: MouseEvent) => {
       if (!isDragging || !imgRef.current) return;

+      if (rafRef.current) cancelAnimationFrame(rafRef.current);
+      
+      rafRef.current = requestAnimationFrame(() => {
         const dx = e.clientX - dragStart.current.x;
         const dy = e.clientY - dragStart.current.y;

         // Apply the scale factor to the drag movement
         const scaledDx = dx / magnification;
         const scaledDy = dy / magnification;

         imgRef.current!.style.left = `${dragOffset.current.x + scaledDx}px`;
         imgRef.current!.style.top = `${dragOffset.current.y + scaledDy}px`;
+      });
     },
     [isDragging, magnification]
   );

Also clean up in the mouseup handler:

   const handleMouseUp = useCallback(() => {
     if (!isDragging || !imgRef.current) return;
     setIsDragging(false);
+    if (rafRef.current) {
+      cancelAnimationFrame(rafRef.current);
+      rafRef.current = null;
+    }
   }, [isDragging]);
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (1)

8-14: Remove unused height property from image type.

The height property is defined in the image type but is not used in this component or passed to the ImageFullScreenModal.

 type Props = {
   image: {
     width: string;
-    height: string;
     aspectRatio: number;
     src: string;
   };
   toggleToolbarViewStatus: (val: boolean) => void;
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d98162b and ec8636a.

📒 Files selected for processing (7)
  • packages/editor/src/core/extensions/custom-image/components/block.tsx (6 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/index.ts (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/root.tsx (1 hunks)
  • space/app/layout.tsx (1 hunks)
  • web/app/layout.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • web/app/layout.tsx
  • space/app/layout.tsx
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/editor/src/core/extensions/custom-image/components/block.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1)

32-35: Width parsing still needs improvement for robustness.

The current implementation still assumes width always has "px" suffix, which contradicts the past review feedback. This could fail with other CSS units or unitless values.

Apply the suggested fix from the previous review:

-  const widthInNumber = useMemo(() => {
-    if (!width) return 0;
-    return Number(width.replace("px", ""));
-  }, [width]);
+  const widthInNumber = useMemo(() => {
+    if (!width) return 0;
+    const match = width.match(/^(\d+(?:\.\d+)?)/);
+    return match ? Number(match[1]) : 0;
+  }, [width]);
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1)

103-121: Improve drag detection logic for better UX.

The current drag detection only allows dragging when the image exceeds viewport dimensions, but this doesn't account for the current magnification level correctly.

  const handleMouseDown = (e: React.MouseEvent) => {
    if (!imgRef.current) return;

-   const imgWidth = imgRef.current.offsetWidth * magnification;
-   const imgHeight = imgRef.current.offsetHeight * magnification;
+   const imgWidth = imgRef.current.offsetWidth * initialMagnification * magnification;
+   const imgHeight = imgRef.current.offsetHeight * initialMagnification * magnification;
    const viewportWidth = window.innerWidth;
    const viewportHeight = window.innerHeight;

    if (imgWidth > viewportWidth || imgHeight > viewportHeight) {
      e.preventDefault();
      e.stopPropagation();
      setIsDragging(true);
      dragStart.current = { x: e.clientX, y: e.clientY };
      dragOffset.current = {
        x: parseInt(imgRef.current.style.left || "0"),
        y: parseInt(imgRef.current.style.top || "0"),
      };
    }
  };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efb511b and 0127b7d.

📒 Files selected for processing (3)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx (2 hunks)
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1 hunks)
  • packages/editor/src/core/hooks/use-outside-click-detector.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/editor/src/core/hooks/use-outside-click-detector.ts
  • packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/editor/src/core/extensions/custom-image/components/toolbar/full-screen/modal.tsx (1)

257-266: Portal handling has been improved as requested.

The portal implementation now includes proper error handling with a warning message when the portal element is not found, addressing the previous review feedback.

@sriramveeraghanta sriramveeraghanta merged commit f679628 into preview Jul 2, 2025
3 of 5 checks passed
@sriramveeraghanta sriramveeraghanta deleted the feat/image-options branch July 2, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants