Skip to content

Comments

[WEB-1974] fix: images getting replaced on resize#5233

Merged
SatishGandham merged 4 commits intopreviewfrom
fix/image-resize
Jul 30, 2024
Merged

[WEB-1974] fix: images getting replaced on resize#5233
SatishGandham merged 4 commits intopreviewfrom
fix/image-resize

Conversation

@aaryan610
Copy link
Member

@aaryan610 aaryan610 commented Jul 25, 2024

Problem:

When the user tries to resize an image when multiple instances of an editor are rendered on the page, the image being resized gets replaced with the image present in the other editor instance.

Solution:

Provided each editor instance with an unique id of its own to render the image resize handle properly.

Media:

Before After
Screen.Recording.2024-07-25.at.18.23.59.mov
Screen.Recording.2024-07-25.at.18.22.10.mov

Plane issue: WEB-1974

Summary by CodeRabbit

  • New Features

    • Added a unique id property to various editor components, enhancing accessibility and identification for better DOM manipulation.
    • Improved the tracking and management of comments, activity items, and editor instances by incorporating id attributes across multiple components.
    • Introduced id support in the DocumentReadOnlyEditorWithRef component for enhanced context-awareness.
  • Bug Fixes

    • Enhanced the robustness of the ImageResizer component by refining the method for image element retrieval.
  • Documentation

    • Updated internal documentation to reflect changes in component interfaces and props.
  • Chores

    • General code cleanup and adjustments to component structures for improved readability and maintainability.

@aaryan610 aaryan610 added this to the v0.23-dev milestone Jul 25, 2024
@aaryan610 aaryan610 requested a review from SatishGandham July 25, 2024 12:55
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 25, 2024

Walkthrough

The recent updates enhance various editor components by introducing a new id property, promoting better identification and accessibility throughout the application. These changes affect components like DocumentEditor, EditorContainer, and several others, improving configurability and interaction capabilities. The overall functionality remains intact, ensuring a seamless user experience while augmenting the components with unique identifiers for effective tracking and manipulation.

Changes

Files Change Summary
packages/editor/src/core/components/editors/document/*.tsx Added an id prop to DocumentEditor, PageRenderer, and related components for better identification.
packages/editor/src/core/components/editors/editor-container.tsx Introduced id to EditorContainerProps, allowing dynamic identification of editor containers.
packages/editor/src/core/components/editors/editor-content.tsx Added id to EditorContentProps, enabling unique identification for editor content.
packages/editor/src/core/components/editors/editor-wrapper.tsx Made id a required prop for EditorWrapper, enforcing explicit identification of editor instances.
packages/editor/src/core/extensions/image/image-resize.tsx Enhanced ImageResizer with an id prop for targeting specific images in resizing functions.
space/core/components/issues/*.tsx Introduced id props in various comment and issue components to improve tracking and interaction.
web/core/components/* Added id attributes to multiple components (e.g., RichTextEditor), improving accessibility and DOM manipulation.
web/core/components/pages/editor/editor-body.tsx Added an id property to DocumentReadOnlyEditorWithRef for better contextual linking.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant DocumentEditor
    participant PageRenderer
    participant EditorContainer
    participant RichTextEditor
    
    User->>DocumentEditor: Open Document
    DocumentEditor->>PageRenderer: Render Page with id
    PageRenderer->>EditorContainer: Load Editor Content
    EditorContainer->>RichTextEditor: Initialize Editor with id
    RichTextEditor-->>User: Display Content
Loading

🐰 In the garden of code, a change took flight,
New ids brought clarity, oh what a sight!
Each editor now knows just where it belongs,
With identifiers in place, we sing joyful songs.
So hop along, friends, to a more organized way,
With every little change, brighter blooms sway! 🌼


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?

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>.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @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.

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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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
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: 0

Outside diff range, codebase verification and nitpick comments (1)
packages/editor/src/core/components/editors/document/read-only-editor.tsx (1)

34-34: Ensure id prop is provided.

The id prop is now required. The instance in web/core/components/pages/editor/editor-body.tsx does not provide the id prop. Please update this instance to include the id prop.

  • web/core/components/pages/editor/editor-body.tsx: Ensure id prop is provided to DocumentReadOnlyEditorWithRef.
Analysis chain

Ensure id prop is provided.

The id prop is now required. Verify that all instances of DocumentReadOnlyEditor provide this prop.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `DocumentReadOnlyEditor` provide the `id` prop.

# Test: Search for the usage of `DocumentReadOnlyEditor`. Expect: All instances should provide the `id` prop.
rg --type js --type ts -A 5 $'<DocumentReadOnlyEditor'

Length of output: 1323

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 43103a1 and ea4730d.

Files selected for processing (21)
  • packages/editor/src/core/components/editors/document/editor.tsx (1 hunks)
  • packages/editor/src/core/components/editors/document/page-renderer.tsx (2 hunks)
  • packages/editor/src/core/components/editors/document/read-only-editor.tsx (3 hunks)
  • packages/editor/src/core/components/editors/editor-container.tsx (2 hunks)
  • packages/editor/src/core/components/editors/editor-content.tsx (1 hunks)
  • packages/editor/src/core/components/editors/editor-wrapper.tsx (2 hunks)
  • packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (2 hunks)
  • packages/editor/src/core/extensions/image/image-resize.tsx (1 hunks)
  • packages/editor/src/core/extensions/mentions/extension.tsx (1 hunks)
  • packages/editor/src/core/extensions/slash-commands.tsx (1 hunks)
  • packages/editor/src/core/types/editor.ts (2 hunks)
  • space/core/components/issues/peek-overview/comment/add-comment.tsx (1 hunks)
  • space/core/components/issues/peek-overview/comment/comment-detail-card.tsx (2 hunks)
  • space/core/components/issues/peek-overview/issue-details.tsx (1 hunks)
  • web/core/components/core/modals/gpt-assistant-popover.tsx (1 hunks)
  • web/core/components/inbox/modals/create-edit-modal/issue-description.tsx (1 hunks)
  • web/core/components/issues/description-input.tsx (1 hunks)
  • web/core/components/issues/issue-detail/issue-activity/comments/comment-card.tsx (2 hunks)
  • web/core/components/issues/issue-modal/form.tsx (1 hunks)
  • web/core/components/profile/activity/activity-list.tsx (1 hunks)
  • web/core/components/profile/activity/profile-activity-list.tsx (1 hunks)
Files skipped from review due to trivial changes (4)
  • space/core/components/issues/peek-overview/comment/add-comment.tsx
  • web/core/components/core/modals/gpt-assistant-popover.tsx
  • web/core/components/inbox/modals/create-edit-modal/issue-description.tsx
  • web/core/components/issues/issue-modal/form.tsx
Additional comments not posted (31)
packages/editor/src/core/components/editors/editor-content.tsx (2)

8-9: Addition of id property to EditorContentProps is correct.

The id property enhances the component's flexibility by enabling unique identification of editor instances.


14-19: Usage of id property in EditorContentWrapper is correct.

The id property is correctly destructured from props and passed to the ImageResizer component, ensuring unique identification of editor instances.

packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (1)

Line range hint 11-29:
Addition and usage of id property in ReadOnlyEditorWrapper is correct.

The id property is correctly added to the props object and passed to the EditorContainer and EditorContentWrapper components, ensuring unique identification of editor instances.

space/core/components/issues/peek-overview/issue-details.tsx (1)

29-29: Addition of id property to RichTextReadOnlyEditor is correct.

The id property is correctly added to the RichTextReadOnlyEditor component, ensuring unique identification of issue details.

packages/editor/src/core/components/editors/editor-wrapper.tsx (3)

62-63: Ensure id prop is correctly used in EditorContainer.

The id prop is passed to EditorContainer. Verify that EditorContainer correctly uses this prop.

Verification successful

The id prop is correctly used in the EditorContainer component.

The id prop is destructured from props and assigned to the id attribute of an HTML element within the EditorContainer component, ensuring it is utilized appropriately.

  • packages/editor/src/core/components/editors/editor-container.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `id` prop in `EditorContainer`.

# Test: Search for the `EditorContainer` component and check the usage of `id` prop. Expect: `id` prop should be used correctly.
rg --type js --type ts -A 5 $'<EditorContainer'

Length of output: 2532


Script:

#!/bin/bash
# Description: Verify the usage of `id` prop within the EditorContainer component.

# Test: Search for the usage of `id` prop within the EditorContainer component.
rg --type js --type ts -A 5 'id' packages/editor/src/core/components/editors/editor-container.tsx

Length of output: 664


67-67: Ensure id prop is correctly used in EditorContentWrapper.

The id prop is passed to EditorContentWrapper. Verify that EditorContentWrapper correctly uses this prop.

Verification successful

The id prop is correctly used in the EditorContentWrapper component.

The id prop is destructured from props and used within the EditorContentWrapper component, specifically passed to the ImageResizer component.

  • packages/editor/src/core/components/editors/editor-content.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `id` prop in `EditorContentWrapper`.

# Test: Search for the `EditorContentWrapper` component and check the usage of `id` prop. Expect: `id` prop should be used correctly.
rg --type js --type ts -A 5 $'<EditorContentWrapper'

Length of output: 1643


Script:

#!/bin/bash
# Description: Locate the definition of `EditorContentWrapper` and verify the usage of the `id` prop.

# Locate the definition of `EditorContentWrapper`
rg --type js --type ts 'function EditorContentWrapper' -A 10

# Locate the definition of `EditorContentWrapper` if it is a class component
rg --type js --type ts 'class EditorContentWrapper' -A 10

Length of output: 122


Script:

#!/bin/bash
# Description: Locate the import statements for `EditorContentWrapper` to find its source file.

# Search for import statements of `EditorContentWrapper`
rg --type js --type ts 'import EditorContentWrapper' -A 1

Length of output: 59


Script:

#!/bin/bash
# Description: Locate any occurrences of `EditorContentWrapper` to find its definition or source file.

# Search for any occurrences of `EditorContentWrapper`
rg --type js --type ts 'EditorContentWrapper'

Length of output: 1021


Script:

#!/bin/bash
# Description: Verify the usage of `id` prop in `EditorContentWrapper` component.

# Search for the definition and usage of `id` prop in `EditorContentWrapper`
rg --type js --type ts 'EditorContentWrapper' -A 20 packages/editor/src/core/components/editors/editor-content.tsx

Length of output: 539


24-24: Ensure id prop is provided.

The id prop is now required. Verify that all instances of EditorWrapper provide this prop.

packages/editor/src/core/components/editors/document/read-only-editor.tsx (2)

16-16: Ensure id prop is provided in implementations of IDocumentReadOnlyEditor.

The id prop is now required in IDocumentReadOnlyEditor. Verify that all implementations of this interface provide the id prop.

Verification successful

Ensure id prop is provided in implementations of IDocumentReadOnlyEditor.

The id prop is now required in IDocumentReadOnlyEditor. The verification confirms that the id prop is correctly included in the primary places where this interface is utilized.

  • packages/editor/src/core/components/editors/document/read-only-editor.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all implementations of `IDocumentReadOnlyEditor` provide the `id` prop.

# Test: Search for the implementations of `IDocumentReadOnlyEditor`. Expect: All implementations should provide the `id` prop.
rg --type js --type ts -A 5 $'IDocumentReadOnlyEditor'

Length of output: 2013


Script:

#!/bin/bash
# Description: Verify all instances of `IDocumentReadOnlyEditor` provide the `id` prop.

# Test: Search for instances where `IDocumentReadOnlyEditor` is used. Expect: All instances should provide the `id` prop.
rg --type js --type ts -A 5 'IDocumentReadOnlyEditor' | grep -E 'IDocumentReadOnlyEditor|id'

Length of output: 703


64-64: Ensure id prop is correctly used in PageRenderer.

The id prop is passed to PageRenderer. Verify that PageRenderer correctly uses this prop.

Verification successful

The id prop is correctly used in the PageRenderer component.

The id prop is passed to the EditorContentWrapper and EditorContainer components within PageRenderer, confirming its appropriate usage.

  • packages/editor/src/core/components/editors/document/page-renderer.tsx
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `id` prop in `PageRenderer`.

# Test: Search for the `PageRenderer` component and check the usage of `id` prop. Expect: `id` prop should be used correctly.
rg --type js --type ts -A 5 $'<PageRenderer'

Length of output: 1413


Script:

#!/bin/bash
# Locate the definition of the `PageRenderer` component and inspect the usage of the `id` prop.

# Find the definition of the `PageRenderer` component.
rg --type js --type ts 'function PageRenderer' -A 20

# Additionally, search for the `id` prop within the `PageRenderer` component to confirm its usage.
rg --type js --type ts 'PageRenderer' -A 20 | rg 'id'

Length of output: 1494


Script:

#!/bin/bash
# Verify the usage of the `id` prop within the `PageRenderer` component in `page-renderer.tsx`.

# Search for the `id` prop within the `page-renderer.tsx` file to confirm its usage.
rg --type js --type ts 'id' packages/editor/src/core/components/editors/document/page-renderer.tsx -A 10

Length of output: 2345

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

32-32: Ensure id prop is provided in implementations of IEditorProps.

The id prop is now required in IEditorProps. Verify that all implementations of this interface provide the id prop.


55-55: Ensure id prop is provided in implementations of IReadOnlyEditorProps.

The id prop is now required in IReadOnlyEditorProps. Verify that all implementations of this interface provide the id prop.

Verification successful

Ensure id prop is provided in all implementations of IReadOnlyEditorProps.

The id prop is now required in IReadOnlyEditorProps. Verify that all instances where this interface is used or implemented provide the id prop.


Ensure id prop is provided in implementations of IReadOnlyEditorProps.

The id prop is now required in IReadOnlyEditorProps. The verification shows that all instances of this interface correctly provide the id prop.

  • ReadOnlyEditorWrapper component correctly destructures and uses the id prop from IReadOnlyEditorProps.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all implementations of `IReadOnlyEditorProps` provide the `id` prop.

# Test: Search for the implementations of `IReadOnlyEditorProps`. Expect: All implementations should provide the `id` prop.
rg --type js --type ts -A 5 $'IReadOnlyEditorProps'

Length of output: 1671


Script:

#!/bin/bash
# Description: Verify all instances where `IReadOnlyEditorProps` is used or implemented provide the `id` prop.

# Test: Search for all instances of `IReadOnlyEditorProps` usage. Expect: All instances should provide the `id` prop.
rg --type js --type ts 'IReadOnlyEditorProps' -A 10

Length of output: 2444

packages/editor/src/core/components/editors/editor-container.tsx (3)

11-11: Addition of id property looks good.

The id property ensures unique identification of each EditorContainer instance, which is essential for the fix.


15-15: Inclusion of id in destructured props looks good.

This is necessary for using the id property within the component.


58-58: Usage of id in the div element looks good.

This ensures unique identification of each EditorContainer instance in the DOM.

packages/editor/src/core/components/editors/document/editor.tsx (1)

84-84: Addition of id prop to PageRenderer looks good.

This ensures that the unique identifier is passed down and can be utilized within the PageRenderer.

packages/editor/src/core/extensions/image/image-resize.tsx (5)

5-8: Addition of Props type looks good.

This improves type safety and clarity by explicitly defining the expected properties.


10-11: Introduction of getImageElement function looks good.

This centralizes the logic for retrieving the active image element, improving maintainability and readability.


14-14: Inclusion of id in destructured props looks good.

This is necessary for using the id property within the component.


19-21: Usage of getImageElement in updateMediaSize looks good.

This ensures that the correct image element is targeted for size updates.


42-43: Usage of getImageElement in Moveable component looks good.

This ensures that the correct image element is targeted for resizing.

packages/editor/src/core/extensions/mentions/extension.tsx (1)

94-95: Verify the new selector logic.

The updated selector logic uses [id^="editor-container"] to match any element whose ID starts with "editor-container". Ensure this selector correctly targets the intended elements and does not unintentionally match other elements.

web/core/components/issues/description-input.tsx (1)

121-123: LGTM! Verify the correct usage of the id prop.

The id prop has been correctly added to the RichTextReadOnlyEditor component. Ensure that the id prop is used correctly and does not conflict with other elements.

packages/editor/src/core/components/editors/document/page-renderer.tsx (2)

24-24: Ensure the id property is correctly added to the IPageRenderer type.

The id property has been correctly added to the IPageRenderer type. Ensure that this change is consistently applied across all usages of the IPageRenderer type.


29-29: LGTM! Verify the correct usage of the id property in PageRenderer.

The id property has been correctly added to the PageRenderer function. Ensure that the id property is used correctly and does not conflict with other elements.

web/core/components/issues/issue-detail/issue-activity/comments/comment-card.tsx (2)

146-146: Ensure the id prop is unique and correctly assigned.

The addition of the id prop to LiteTextEditor is a good practice for unique identification. Ensure that comment.id is always unique and correctly assigned.


194-194: Ensure the id prop is unique and correctly assigned.

The addition of the id prop to LiteTextReadOnlyEditor is a good practice for unique identification. Ensure that comment.id is always unique and correctly assigned.

web/core/components/profile/activity/activity-list.tsx (1)

71-71: Ensure the id prop is unique and correctly assigned.

The addition of the id prop to RichTextReadOnlyEditor is a good practice for unique identification. Ensure that activityItem.id is always unique and correctly assigned.

space/core/components/issues/peek-overview/comment/comment-detail-card.tsx (2)

108-108: Ensure the id prop is unique and correctly assigned.

The addition of the id prop to LiteTextEditor is a good practice for unique identification. Ensure that comment.id is always unique and correctly assigned.


136-136: Ensure the id prop is unique and correctly assigned.

The addition of the id prop to LiteTextReadOnlyEditor is a good practice for unique identification. Ensure that comment.id is always unique and correctly assigned.

web/core/components/profile/activity/profile-activity-list.tsx (1)

100-102: LGTM! Verify the uniqueness of activityItem.id.

The addition of the id attribute to the RichTextReadOnlyEditor component is correct and follows the logic described in the PR summary. Ensure that activityItem.id is unique to prevent any conflicts or unexpected behavior.

packages/editor/src/core/extensions/slash-commands.tsx (1)

377-378: LGTM! Verify the impact of the new selector.

The update to the selector for tippyContainer to use an attribute selector [id^="editor-container"] is correct and enhances the flexibility of the code. Ensure that this change does not introduce any unintended side effects, such as selecting multiple elements or causing conflicts with other elements.

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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ea4730d and 3a47173.

Files selected for processing (1)
  • web/core/components/pages/editor/editor-body.tsx (1 hunks)
Additional comments not posted (1)
web/core/components/pages/editor/editor-body.tsx (1)

156-158: Ensure the id property is correctly assigned and utilized.

The addition of the id property to the DocumentReadOnlyEditorWithRef component looks good. Ensure that the pageId is always defined and correctly assigned to avoid potential issues.

Verification successful

The id property is correctly assigned and utilized.

The addition of the id property to the DocumentReadOnlyEditorWithRef component is correctly implemented. The pageId is derived from page?.id and there is a conditional check to ensure that pageId is defined before it is used.

  • pageId is derived from page?.id.
  • There is a conditional check to ensure pageId is defined.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `pageId` is always defined and correctly assigned.

# Test: Search for the definition and assignment of `pageId`. Expect: `pageId` is derived from `page?.id`.
rg --type tsx -A 5 $'const pageId = page?.id'

Length of output: 74


Script:

#!/bin/bash
# Description: Verify that `pageId` is always defined and correctly assigned.

# Test: Search for the definition and assignment of `pageId`. Expect: `pageId` is derived from `page?.id`.
rg -A 5 'const pageId = page?.id'

Length of output: 546


Script:

#!/bin/bash
# Description: Verify the usage of `pageId` within `web/core/components/pages/editor/editor-body.tsx`.

# Test: Search for the usage of `pageId` within the `PageEditorBody` component.
rg 'pageId' web/core/components/pages/editor/editor-body.tsx -A 5 -B 5

Length of output: 1885

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants