Skip to content
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-2723] fix: change html tag name for editor callout component #5924

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

aaryan610
Copy link
Collaborator

@aaryan610 aaryan610 commented Oct 29, 2024

Bug fix:

Editor not able to identify custom callout tag with block content.

Summary by CodeRabbit

  • New Features

    • Enhanced HTML parsing and rendering for callout components in the editor.
    • Introduced a new attribute for callout block types to improve component identification.
  • Bug Fixes

    • Improved error handling for logo retrieval from local storage.
  • Documentation

    • Updated type definitions related to callout components for clarity and accuracy.

@aaryan610 aaryan610 added this to the v0.24.0 milestone Oct 29, 2024
Copy link
Contributor

coderabbitai bot commented Oct 29, 2024

Walkthrough

The changes involve modifications to the callout component within the Tiptap editor extension. Specifically, the HTML parsing and rendering logic has been updated to replace the custom tag "callout-component" with a standard div element, enhancing HTML integration. Additionally, a new block type attribute, BLOCK_TYPE, has been added to the EAttributeNames enum and the TCalloutBlockAttributes type, which now includes this attribute. The DEFAULT_CALLOUT_BLOCK_ATTRIBUTES object has also been updated to reflect this change.

Changes

File Path Change Summary
packages/editor/src/core/extensions/callout/... - Updated CustomCalloutExtensionConfig to change HTML parsing and rendering from "callout-component" to div.
- Streamlined integration of callout component into HTML.
packages/editor/src/core/extensions/callout/... - Added BLOCK_TYPE to EAttributeNames enum with value "data-block-type".
- Updated TCalloutBlockAttributes to include the new block type.
packages/editor/src/core/extensions/callout/... - Updated DEFAULT_CALLOUT_BLOCK_ATTRIBUTES to include "data-block-type": "callout-component".
- Minor adjustments to getStoredLogo function for fallback values.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Editor
    participant CalloutComponent

    User->>Editor: Insert callout
    Editor->>CalloutComponent: Parse HTML (div with data-block-type)
    CalloutComponent-->>Editor: Render callout
    Editor-->>User: Display callout in editor
Loading

🐇 "In the editor's bright light,
Callouts take flight,
From tags to divs they leap,
With attributes to keep.
A block type anew,
For all to view!" 🐇


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>, please review it.
    • 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 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 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 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 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.

@aaryan610 aaryan610 changed the title fix: change html tag name for editor callout component [WEB-2723] fix: change html tag name for editor callout component Oct 29, 2024
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 and nitpick comments (1)
packages/editor/src/core/extensions/callout/extension-config.ts (1)

61-70: Consider adding migration notes to documentation.

Since this change modifies the HTML structure of callout components, it would be helpful to:

  1. Document this change in the changelog
  2. Add migration notes for users who might be targeting these elements in their CSS/JS
  3. Update any relevant documentation that references the callout component structure
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b67f352 and 91d85a8.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • packages/editor/src/core/extensions/callout/extension-config.ts (1 hunks)
  • packages/editor/src/core/extensions/callout/types.ts (2 hunks)
  • packages/editor/src/core/extensions/callout/utils.ts (1 hunks)
🔇 Additional comments (6)
packages/editor/src/core/extensions/callout/types.ts (2)

24-24: LGTM! Type definition ensures consistent block identification.

The literal type ensures type safety when working with callout blocks and maintains compatibility with the HTML structure.


8-8: LGTM! Verify existing callout elements migration.

The new data attribute follows the established pattern and supports the migration from custom elements to standard div tags.

Let's verify the migration impact:

✅ Verification successful

Migration to data attributes is properly implemented

The verification shows the migration is complete and consistent:

  • No custom <callout-component> tags remain in the codebase
  • The new data-block-type="callout-component" attribute is properly implemented in utils.ts
  • The component is now rendered as a div with appropriate className in block.tsx
  • All references to "callout-component" are using the new attribute-based approach
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of the custom callout-component tag
rg -i "callout-component" --type html --type tsx --type jsx

# Search for the new data attribute usage
rg "data-block-type.*callout-component"

Length of output: 224


Script:

#!/bin/bash
# Let's try a more comprehensive search without file type restrictions
# Search for callout-component usage
rg -i "callout-component"

# Search for callout element creation or references in TypeScript/JavaScript files
ast-grep --pattern 'createElement($_, "callout-component"'

# Look for any HTML-like tag usage
rg "<callout-component"

# Check the migration implementation
rg "createCalloutElement|renderCallout" -A 5

Length of output: 697

packages/editor/src/core/extensions/callout/extension-config.ts (2)

61-65: LGTM! Good improvement in HTML standard compliance.

Replacing the custom callout-component element with a div that uses a data attribute for identification is a better approach. This avoids potential issues with unregistered custom elements while maintaining component identification.


70-70: Verify impact on existing callout components.

The change to render as div looks good and is consistent with the parsing changes. However, we should verify that existing callout components are properly migrated.

✅ Verification successful

Change from callout-component to div is safe to proceed

The search results show that callout-component is not used as an HTML tag in the codebase, but rather as:

  • A data attribute value (data-block-type="callout-component")
  • A class name (editor-callout-component)
  • A type identifier

Since the actual HTML tag change from callout-component to div doesn't affect these usages, and the component still maintains its distinct styling and functionality through classes and data attributes, this change is safe and won't impact existing callout components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of callout-component tag in the codebase
# This helps ensure we haven't missed any instances that might expect the old tag

echo "Checking for remaining callout-component tags in HTML/TSX/JSX files..."
rg -t html -t tsx -t jsx "callout-component"

echo "Checking for tests that might expect the old tag..."
rg -t test -t spec "callout-component"

Length of output: 404


Script:

#!/bin/bash
# Let's try a different approach using more generic search patterns

# Search for callout-component string across all files
echo "Searching for 'callout-component' across all files..."
rg "callout-component"

# Search for test files related to callout
echo -e "\nSearching for callout-related test files..."
fd -e test.ts -e spec.ts -e test.tsx -e spec.tsx | grep -i callout

# Look for any callout-related component definitions
echo -e "\nSearching for callout component definitions..."
rg "class.*Callout|interface.*Callout|type.*Callout"

Length of output: 1445

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

20-20: LGTM! Good use of data attribute for component identification.

Moving from a custom HTML tag to a standard div with a data attribute follows HTML5 best practices and improves standards compliance.


20-20: Verify related changes in parsing and rendering logic.

Since this is part of a larger change to replace the custom tag with a div, let's verify the corresponding updates in the extension configuration.

✅ Verification successful

Let me gather more information about the HTML parsing and rendering implementation to verify the changes.


Let me check the extension configuration and schema to complete the verification.


Let me check the extension implementation files to complete the verification.


Let me try a different approach to find the extension implementation files.


HTML parsing and rendering logic has been properly updated for the div-based implementation

The verification confirms that the HTML parsing and rendering logic has been correctly updated:

  1. The extension configuration in extension-config.ts properly handles both parsing and rendering:

    • parseHTML() correctly matches the div with data-block-type="callout-component"
    • renderHTML() renders the node as a div with the appropriate attributes
  2. The implementation is consistent across the codebase:

    • The block component uses the correct div structure with className editor-callout-component
    • The attributes and types are properly defined in types.ts
    • No legacy custom tag implementations were found

The changes are complete and consistent with the div-based approach.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that HTML parsing and rendering logic has been updated consistently

# Test 1: Check for any remaining references to the custom tag
echo "Checking for remaining references to custom callout-component tag..."
rg -i "callout-component" --type ts

# Test 2: Verify the new div-based implementation in extension config
echo "Checking for updated HTML parsing/rendering logic..."
ast-grep --pattern 'parseHTML|renderHTML' packages/editor/src/core/extensions/callout/

Length of output: 924


Script:

#!/bin/bash
# Check the callout extension implementation for HTML parsing and rendering
echo "Checking callout extension implementation..."
cat packages/editor/src/core/extensions/callout/block.tsx

# Check for any test files related to callout parsing/rendering
echo -e "\nChecking related test files..."
fd "callout.*test" --type f --exec cat {}

Length of output: 2351


Script:

#!/bin/bash
# Check the extension configuration and schema
echo "Checking callout extension configuration..."
cat packages/editor/src/core/extensions/callout/index.ts

echo -e "\nChecking callout extension schema..."
cat packages/editor/src/core/extensions/callout/types.ts

Length of output: 1227


Script:

#!/bin/bash
# Check the extension implementations
echo "Checking callout extension implementation..."
cat packages/editor/src/core/extensions/callout/extension.ts

echo -e "\nChecking read-only extension implementation..."
cat packages/editor/src/core/extensions/callout/read-only-extension.ts

Length of output: 531


Script:

#!/bin/bash
# Find and check all TypeScript files in the callout directory
echo "Finding all TypeScript files in callout directory..."
fd -e ts -e tsx . packages/editor/src/core/extensions/callout/ --exec cat {}

# Search for any parseHTML or renderHTML implementations
echo -e "\nSearching for HTML parsing/rendering implementations..."
rg -A 5 "parseHTML|renderHTML" --type ts

Length of output: 37708

@pushya22 pushya22 merged commit b4bbe3a into preview Oct 29, 2024
14 of 15 checks passed
@pushya22 pushya22 deleted the fix/callout branch October 29, 2024 08:42
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.

3 participants