-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#264 editor component #297
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
base: main
Are you sure you want to change the base?
Conversation
Dokploy Preview Deployment
|
WalkthroughThis set of changes introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RichTextEditor
participant EditorToolbar
participant TiptapEditor
participant Server (Image Upload)
User->>RichTextEditor: Type or interact with editor
RichTextEditor->>TiptapEditor: Update content
TiptapEditor-->>RichTextEditor: Trigger onChange (HTML, text)
RichTextEditor-->>User: Calls onChange handler
User->>EditorToolbar: Click toolbar button (e.g., Bold, Heading)
EditorToolbar->>TiptapEditor: Execute command (toggle bold, heading, etc.)
TiptapEditor-->>RichTextEditor: Update state
User->>EditorToolbar: Click "Insert Image"
EditorToolbar->>Server (Image Upload): Upload image file
Server (Image Upload)-->>EditorToolbar: Return image URL
EditorToolbar->>TiptapEditor: Insert image with URL
User->>EditorToolbar: Toggle HTML view
EditorToolbar->>RichTextEditor: Switch to HTML textarea
User->>RichTextEditor: Edit HTML source
RichTextEditor->>TiptapEditor: Update content from HTML
Possibly related PRs
Suggested reviewers
Poem
✨ 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:
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 (
|
Feat/#264 editor component
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🧹 Nitpick comments (17)
packages/ui/src/components/molecules/editor/rich-text-styles-provider.tsx (1)
4-6: Add support for custom class namesThe component currently doesn't allow for customization beyond the default "prose prose-sm" classes. Consider extending the component to accept custom classes:
-export const RichTextStylesProvider = ({ children }: PropsWithChildren) => { - return <div className={cn("prose prose-sm")}>{children}</div>; +export const RichTextStylesProvider = ({ + children, + className +}: PropsWithChildren<{ className?: string }>) => { + return <div className={cn("prose prose-sm", className)}>{children}</div>;apps/core/app/test/page.tsx (1)
9-9: Consider adding proper state management instead of console.logWhile
console.logis acceptable for testing, consider implementing proper state management for the component:- onChange={(html, text) => console.log(html, text)} + onChange={(html, text) => { + // Store in state or perform actions with the content + setEditorContent(html); + }}packages/ui/src/components/molecules/editor/editor-toolbar-button.tsx (1)
17-21: Use thecnutility for improved className handlingSimilar to other components in your codebase, consider using the
cnutility function to handle class name composition for better readability and consistency:- className={`flex items-center gap-1 rounded-md px-1 py-1 text-sm transition-colors ${ - isActive - ? "bg-c1-500 text-white" - : " text-foreground hover:bg-gray-800" - } ${disabled ? "pointer-events-none cursor-not-allowed" : ""}`} + className={cn( + "flex items-center gap-1 rounded-md px-1 py-1 text-sm transition-colors", + isActive ? "bg-c1-500 text-white" : "text-foreground hover:bg-gray-800", + disabled && "pointer-events-none cursor-not-allowed" + )}Don't forget to import the
cnutility:+import { cn } from "~/lib/utils";packages/ui/src/components/molecules/editor/editor-toolbar-view-set-link.tsx (3)
34-34: Fix typo in classNameThere's a typo in the className "poooopppper" which appears to be unintentional.
- <PopoverContent className="poooopppper w-64 p-2"> + <PopoverContent className="w-64 p-2">
35-41: Enhance form usability with keyboard supportConsider adding keyboard support (e.g., submitting the form when the Enter key is pressed in the input field) to improve accessibility and user experience.
<Input type="url" placeholder="Enter URL" value={url} onChange={(e) => setUrl(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + onSubmit(); + } + }} />
16-21: Consider adding URL validationThe current implementation doesn't validate if the provided URL is properly formatted before setting the link.
const onSubmit = () => { if (!editor || !url) return; + const urlPattern = /^(https?:\/\/)?([\da-z.-]+)\.([a-z.]{2,6})([/\w .-]*)*\/?$/; + if (!urlPattern.test(url)) { + // Consider using toast.error("Please enter a valid URL"); + return; + } editor.chain().focus().extendMarkRange("link").setLink({ href: url }).run(); setOpen(false); setUrl(""); };packages/ui/src/components/molecules/editor/editor-toolbar-image.tsx (2)
1-1: Justify or remove eslint disable commentPlease add a comment explaining why the
no-restricted-importseslint rule is disabled or consider addressing the underlying issue.-/* eslint-disable no-restricted-imports */ +/* eslint-disable no-restricted-imports */ // Needed because we require direct axios import for file upload
16-20: Make upload endpoint configurableThe API endpoint for image uploads is hardcoded, which limits flexibility and complicates testing.
- const response = await axios.post('/api/upload', formData, { + const uploadUrl = process.env.NEXT_PUBLIC_UPLOAD_API_URL || '/api/upload'; + const response = await axios.post(uploadUrl, formData, { headers: { 'Content-Type': 'multipart/form-data' } });apps/storybook/src/stories/editor.stories.tsx (1)
23-29: Add a controlled example storyThe current examples only showcase the uncontrolled mode using
defaultValue. Consider adding a controlled example usingvalueandonChangeto demonstrate both usage patterns.export const Default: Story = { args: { label: "Rich Text Editor", helperText: "You can enter rich text here.", defaultValue: "<p>Initial content</p>", }, }; +export const Controlled: Story = { + render: () => { + const [value, setValue] = React.useState("<p>Controlled content</p>"); + return ( + <RichTextEditor + label="Controlled Rich Text Editor" + helperText="This editor uses controlled value state." + value={value} + onChange={(html) => setValue(html)} + /> + ); + } +};packages/ui/src/components/molecules/editor/editor-toolbar.tsx (4)
1-22: Remove unused icon importsSeveral icon imports are not used in the component implementation:
Link2Off,List,Quote,Underline,Heading1,Heading5,Heading6This can reduce bundle size and improve code cleanliness.
import { Bold, Italic, - Link2Off, List, ListOrdered, - Quote, Redo, Strikethrough, - Underline, Undo, - Heading1, Heading2, Heading3, Heading4, - Heading6, - Heading5, ChevronsLeftRightEllipsis, Code, } from "lucide-react";
24-28: Remove unused Separator importThe Separator component is imported but not used in the EditorToolbar.
-import { Separator } from "./../../atoms/separator"; import { EditorToolbarButton } from "./editor-toolbar-button";
63-64: Remove or implement "to do normal text" commentThis TODO comment should either be implemented or removed.
- {/* to do normal text */} + {/* Typography controls */}
86-99: Remove or implement commented-out heading levelsThere are commented-out heading buttons (levels 5 and 6) that should either be implemented or removed to maintain clean code.
Either remove these commented blocks entirely or uncomment and implement them if these heading levels are needed.
packages/ui/src/components/molecules/editor/editor.tsx (4)
8-22: Inconsistent imports and commented codeThere are a few issues with the imports:
- There are commented-out imports (
BubbleMenu,FloatingMenu) that should be removed if not used- There's a commented-out import for
RichTextStylesProvideron line 22 while the same component is imported on line 8 with a different pathClean up the imports for better maintainability.
import { RichTextStylesProvider } from "@repo/ui/components/rich-text-style-provider"; import Image from "@tiptap/extension-image"; import Link from "@tiptap/extension-link"; import TextAlign from "@tiptap/extension-text-align"; import { - // BubbleMenu, EditorContent, - // FloatingMenu, useEditor, } from "@tiptap/react"; import StarterKit from "@tiptap/starter-kit"; import { cva } from "class-variance-authority"; import { LabelWraper } from "../label-wrapper"; import { EditorToolbar } from "./editor-toolbar"; -// import { RichTextStylesProvider } from "@/components/providers/rich-text-style-provider";
61-65: Review commented-out extensionThere's a commented-out
Highlightextension. Either implement it or remove the comment for cleaner code.extensions: [ StarterKit, TextAlign.configure({ types: ["heading", "paragraph"] }), - // Highlight, Image, Link, ],
58-78: Configure link extension for better UXThe Link extension is added but lacks configuration for important UX aspects like opening links in new tabs by default and allowing link editing.
Consider configuring the Link extension with better defaults:
extensions: [ StarterKit, TextAlign.configure({ types: ["heading", "paragraph"] }), Image, - Link, + Link.configure({ + openOnClick: false, + linkOnPaste: true, + HTMLAttributes: { + rel: 'noopener noreferrer', + target: '_blank', + }, + }), ],
80-84: Improve HTML content update methodThe
updateEditorFromHtmlmethod only runs ifshowHtmlis true, but should ideally trigger an update regardless of the current view mode to ensure content consistency.const updateEditorFromHtml = () => { - if (editor && showHtml) { + if (editor) { editor.commands.setContent(htmlContent, false); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (16)
apps/core/app/test/page.tsx(1 hunks)apps/storybook/src/stories/editor.stories.tsx(1 hunks)package.json(1 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/atoms/command.tsx(1 hunks)packages/ui/src/components/atoms/rich-text-style-provider.tsx(1 hunks)packages/ui/src/components/atoms/textarea.tsx(2 hunks)packages/ui/src/components/molecules/editor/editor-toolbar-button.tsx(1 hunks)packages/ui/src/components/molecules/editor/editor-toolbar-image.tsx(1 hunks)packages/ui/src/components/molecules/editor/editor-toolbar-view-html.tsx(1 hunks)packages/ui/src/components/molecules/editor/editor-toolbar-view-set-link.tsx(1 hunks)packages/ui/src/components/molecules/editor/editor-toolbar.tsx(1 hunks)packages/ui/src/components/molecules/editor/editor.tsx(1 hunks)packages/ui/src/components/molecules/editor/rich-text-styles-provider.tsx(1 hunks)packages/ui/src/styles/globals.scss(1 hunks)packages/ui/tailwind.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/ui/src/components/molecules/editor/rich-text-styles-provider.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
packages/ui/src/components/molecules/editor/editor-toolbar-image.tsx (2)
packages/ui/src/components/molecules/editor/editor-toolbar.tsx (1)
ToolbarProps(30-35)packages/ui/src/components/molecules/editor/editor-toolbar-button.tsx (1)
EditorToolbarButton(1-26)
apps/storybook/src/stories/editor.stories.tsx (1)
packages/ui/src/components/molecules/editor/editor.tsx (1)
RichTextEditor(45-147)
packages/ui/src/components/molecules/editor/editor-toolbar.tsx (1)
packages/ui/src/components/molecules/editor/editor-toolbar-button.tsx (1)
EditorToolbarButton(1-26)
🔇 Additional comments (8)
packages/ui/tailwind.config.ts (1)
189-189: Added Tailwind Typography plugin
The@tailwindcss/typographyplugin has been correctly introduced to enhance rich text styling.packages/ui/src/components/atoms/textarea.tsx (1)
1-1: Trailing semicolons and commas for consistency
The added semicolons after imports, return JSX, function definitions, and exports, as well as the trailing comma in thecncall, align with the project's style guide.Also applies to: 14-14, 19-19, 21-21, 24-24
packages/ui/src/styles/globals.scss (2)
234-237: Remove default ProseMirror focus outline
Applyingoutline: none !important;ensures that focus styles are controlled via Tailwind utilities or custom styles.
239-241: Verify CSS variable usage for selected node outline
The rule usesvar(--c1-400), but this variable isn't defined in this file. Please confirm that--c1-400is provided by imported design-system styles or update it to a valid CSS variable (e.g.,--primary-400).package.json (1)
4-10: Reorder configuration fields for clarity
MovingdevDependencies,engines,packageManager, andprivatecloser to the top improves readability without altering functionality.Also applies to: 11-13, 15-16
packages/ui/package.json (1)
52-68: Added Tiptap and Typography dependencies
The new dependencies (@tailwindcss/typographyand@tiptap/*) support the rich text editor features. Versions align with existing setup.packages/ui/src/components/molecules/editor/editor-toolbar-view-html.tsx (1)
5-27: Looks good!The component is well-structured and correctly implements the HTML view toggle functionality. It properly synchronizes the HTML content state when switching between views.
apps/storybook/src/stories/editor.stories.tsx (1)
4-47: Good job with story variations and controls!The different story variations (Default, WithError, CustomClass) provide good coverage of the component's features and demonstrate proper usage patterns with meaningful examples.
|
|
||
| import * as React from "react" | ||
| import { type DialogProps } from "@radix-ui/react-dialog" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Missing import for DialogProps.
The CommandDialogProps interface extends DialogProps, but the import for that type was removed in this hunk. This will cause unresolved type errors. Please re-add the correct import, for example:
+import type { DialogProps } from "@repo/ui/components/ui/dialog"Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| import { cn } from "@repo/ui/lib/utils" | ||
| import { Dialog, DialogContent } from "@repo/ui/components/ui/dialog" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Missing imports for Dialog and DialogContent.
The CommandDialog component still references <Dialog> and <DialogContent>, but those components were removed from the import list. This will break the build. Please restore their import from the appropriate module, for example:
+import { Dialog, DialogContent } from "@repo/ui/components/ui/dialog"Or, if the dialog wrapper is no longer needed, remove/replace the CommandDialog implementation accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Dialog, DialogContent } from "@repo/ui/components/ui/dialog" |
| export const RichTextStylesProvider = ({ children }: PropsWithChildren) => { | ||
| return <div className={cn("prose prose-sm")}>{children}</div>; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider potential duplication with the atomic component
I notice there's another RichTextStylesProvider component in packages/ui/src/components/atoms/rich-text-style-provider.tsx with different styling implementation. This could cause confusion for developers. Consider:
- Using a different name for one of these components
- Consolidating them into a single, more configurable component
- Clearly documenting the purpose of each if they're intended to serve different use cases
| <RichTextEditor | ||
| onChange={(html, text) => console.log(html, text)} | ||
| defaultValue="hello" | ||
| value="salam" | ||
| label="badry" | ||
| helperText="ali" | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixing controlled and uncontrolled props can cause unpredictable behavior
The RichTextEditor component is currently receiving both value (controlled) and defaultValue (uncontrolled) props simultaneously. React components should typically be either fully controlled or uncontrolled, not both.
Choose one approach:
- For controlled: Provide
valueandonChange - For uncontrolled: Provide only
defaultValue
<RichTextEditor
onChange={(html, text) => console.log(html, text)}
defaultValue="hello"
- value="salam"
label="badry"
helperText="ali"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <RichTextEditor | |
| onChange={(html, text) => console.log(html, text)} | |
| defaultValue="hello" | |
| value="salam" | |
| label="badry" | |
| helperText="ali" | |
| /> | |
| <RichTextEditor | |
| onChange={(html, text) => console.log(html, text)} | |
| defaultValue="hello" | |
| label="badry" | |
| helperText="ali" | |
| /> |
| export const RichTextStylesProvider = ({ children }: PropsWithChildren) => { | ||
| return ( | ||
| // INFO: "prose" class has max-width and we should remove it by max-w-none | ||
| <div className="prose prose-sm max-w-none prose-h2:mb-1 prose-h2:mt-1 prose-h3:mt-1 prose-h4:mt-1 prose-p:my-0 prose-p:mb-1 prose-ul:my-1 prose-ul:list-none prose-ul:pl-1 prose-hr:my-5 prose:text-froground dark:prose-invert"> | ||
| {children} | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Resolve duplication with the molecular component
There are two components named RichTextStylesProvider in different directories with different styling implementations:
- This one in
atomswith extensive styling - Another in
molecules/editorwith minimal styling
This duplication makes maintenance more difficult and could confuse developers. Consider:
- Merging them into one configurable component
- Renaming one of them to better reflect its purpose
- Adding JSDoc comments to clarify the intended use case for each
| import { EditorToolbarImage } from "./editor-toolbar-image"; | ||
| import { EditorToolbarViewHtml } from "./editor-toolbar-view-html"; | ||
| import { EditorToolbarSetLink } from "./editor-toolbar-view-set-link"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing toolbar components in implementation
Several toolbar components are imported but not used in the render method:
- EditorToolbarImage
- EditorToolbarViewHtml
- EditorToolbarSetLink
This suggests missing functionality for image uploading, HTML view toggling, and link insertion, which are typically essential for a complete rich text editor.
Consider adding these components to the toolbar implementation:
<div className="flex flex-wrap gap-2 rounded-md p-2 shadow">
{/* Existing toolbar buttons */}
{/* Add the missing toolbar components */}
+ <Separator orientation="vertical" className="mx-1 h-6" />
+ <EditorToolbarSetLink editor={editor} disabled={showHtml} />
+ <EditorToolbarImage editor={editor} disabled={showHtml} />
+ <EditorToolbarViewHtml showHtml={showHtml} setShowHtml={setShowHtml} setHtmlContent={setHtmlContent} editor={editor} />
</div>Committable suggestion skipped: line range outside the PR's diff.
| <div className="flex flex-wrap gap-2 rounded-md p-2 shadow"> | ||
| {/* Typography */} | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().undo().run()} | ||
| isActive={false} | ||
| icon={Undo} | ||
| label="Undo" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().redo().run()} | ||
| isActive={false} | ||
| icon={Redo} | ||
| label="Redo" | ||
| /> | ||
|
|
||
| {/* to do normal text */} | ||
|
|
||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleHeading({ level: 2 }).run()} | ||
| isActive={editor.isActive("heading2")} | ||
| icon={Heading2} | ||
| label="heading 2" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleHeading({ level: 3 }).run()} | ||
| isActive={editor.isActive("heading3")} | ||
| icon={Heading3} | ||
| label="heading 3" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleHeading({ level: 4 }).run()} | ||
| isActive={editor.isActive("heading4")} | ||
| icon={Heading4} | ||
| label="heading 4" | ||
| /> | ||
| {/* <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleHeading({ level: 5 }).run()} | ||
| isActive={editor.isActive("heading5")} | ||
| icon={Heading5} | ||
| label="heading 5" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleHeading({ level: 6 }).run()} | ||
| isActive={editor.isActive("heading6")} | ||
| icon={Heading6} | ||
| label="heading 5" | ||
| /> */} | ||
|
|
||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleBold().run()} | ||
| isActive={editor.isActive("bold")} | ||
| icon={Bold} | ||
| label="Bold" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleItalic().run()} | ||
| isActive={editor.isActive("italic")} | ||
| icon={Italic} | ||
| label="Italic" | ||
| /> | ||
|
|
||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleStrike().run()} | ||
| isActive={editor.isActive("strike")} | ||
| icon={Strikethrough} | ||
| label="Strike" | ||
| /> | ||
| {/* to do code text */} | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleCodeBlock().run()} | ||
| isActive={editor.isActive("codeBlock")} | ||
| icon={ChevronsLeftRightEllipsis} | ||
| label="Strike" | ||
| /> | ||
|
|
||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleOrderedList().run()} | ||
| isActive={editor.isActive("orderedList")} | ||
| icon={ListOrdered} | ||
| label="Ordered List" | ||
| /> | ||
| <EditorToolbarButton | ||
| disabled={showHtml} | ||
| onClick={() => editor.chain().focus().toggleCode().run()} | ||
| isActive={editor.isActive("code")} | ||
| icon={Code} | ||
| label="Code" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing essential formatting options
The toolbar lacks several essential rich text formatting options:
- Bullet lists (unordered lists)
- Blockquote
- Text alignment options
- Link insertion/removal
These are standard features in rich text editors and would enhance the usability of the component.
Add the missing formatting options:
+ <EditorToolbarButton
+ disabled={showHtml}
+ onClick={() => editor.chain().focus().toggleBulletList().run()}
+ isActive={editor.isActive("bulletList")}
+ icon={List}
+ label="Bullet List"
+ />
+ <EditorToolbarButton
+ disabled={showHtml}
+ onClick={() => editor.chain().focus().toggleBlockquote().run()}
+ isActive={editor.isActive("blockquote")}
+ icon={Quote}
+ label="Blockquote"
+ />| onChange={(e) => { | ||
| setHtmlContent(e.target.value); | ||
| onChange?.(e.target.value, ""); | ||
| }} // Update state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect text value in HTML mode onChange handler
When the HTML content changes in the source editor, the onChange function is called with an empty string as the text value. This should be extracted from the HTML content to provide consistent behavior.
onChange={(e) => {
setHtmlContent(e.target.value);
- onChange?.(e.target.value, "");
+ // Extract text from HTML to maintain consistent behavior
+ const tempDiv = document.createElement('div');
+ tempDiv.innerHTML = e.target.value;
+ const textContent = tempDiv.textContent || tempDiv.innerText || '';
+ onChange?.(e.target.value, textContent);
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onChange={(e) => { | |
| setHtmlContent(e.target.value); | |
| onChange?.(e.target.value, ""); | |
| }} // Update state | |
| onChange={(e) => { | |
| setHtmlContent(e.target.value); | |
| // Extract text from HTML to maintain consistent behavior | |
| const tempDiv = document.createElement('div'); | |
| tempDiv.innerHTML = e.target.value; | |
| const textContent = tempDiv.textContent || tempDiv.innerText || ''; | |
| onChange?.(e.target.value, textContent); | |
| }} // Update state |
| <EditorToolbar | ||
| editor={editor} | ||
| setShowHtml={setShowHtml} | ||
| showHtml={showHtml} | ||
| setHtmlContent={setHtmlContent} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Missing toolbar functionality implementation
The EditorToolbar is used but based on the editor-toolbar.tsx file, it doesn't include implementation for image uploading, link insertion, or HTML view toggling despite importing these components.
Ensure that all necessary toolbar features are properly exposed and implemented.
🏁 Script executed:
#!/bin/bash
# Check if the toolbar components are actually used somewhere else
echo "Checking usage of toolbar components..."
rg -A 5 "EditorToolbarImage|EditorToolbarViewHtml|EditorToolbarSetLink" --type tsxLength of output: 201
🏁 Script executed:
#!/bin/bash
# Locate and display the EditorToolbar implementation to verify the imported but unused components
file=$(find . -type f -iname "editor-toolbar.tsx" -print -quit)
echo "Found EditorToolbar file at: $file"
echo "---- File Content Start ----"
sed -n '1,200p' "$file"
echo "---- File Content End ----"Length of output: 4669
Implement missing image upload, link insertion, and HTML‐view toggles in the toolbar
The EditorToolbar component imports EditorToolbarImage, EditorToolbarSetLink, and EditorToolbarViewHtml but never renders them—so users can’t insert images, links, or toggle HTML view. Please update:
• File: packages/ui/src/components/molecules/editor/editor-toolbar.tsx
– Inside the <div className="flex …"> add the three imported components, for example:
{/* existing buttons… */}
+ <Separator />
+ <EditorToolbarSetLink
+ disabled={showHtml}
+ editor={editor}
+ />
+ <EditorToolbarImage
+ disabled={showHtml}
+ editor={editor}
+ />
+ <Separator />
+ <EditorToolbarViewHtml
+ showHtml={showHtml}
+ setShowHtml={setShowHtml}
+ setHtmlContent={setHtmlContent}
+ />Ensure you import Separator (if needed) and pass the correct props so each feature is activated when clicked.
Committable suggestion skipped: line range outside the PR's diff.
| <RichTextStylesProvider> | ||
| <div | ||
| className={cn( | ||
| " flex min-h-[200px] flex-col bg-background ", | ||
|
|
||
| className, | ||
| )} | ||
| > | ||
| {showHtml ? ( | ||
| <Textarea | ||
| className="flex-1 border-none" | ||
| value={htmlContent} | ||
| onChange={(e) => { | ||
| setHtmlContent(e.target.value); | ||
| onChange?.(e.target.value, ""); | ||
| }} // Update state | ||
| onBlur={updateEditorFromHtml} // Update editor on blur | ||
| /> | ||
| ) : ( | ||
| <> | ||
| <RichTextStylesProvider> | ||
| <EditorContent className="p-2" editor={editor} /> | ||
| </RichTextStylesProvider> | ||
| </> | ||
| )} | ||
| </div> | ||
| </RichTextStylesProvider> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Nested RichTextStylesProvider components
There are two nested instances of RichTextStylesProvider (lines 116 and 136), which is redundant and may cause styling issues.
Remove the nested provider:
<div>
<RichTextStylesProvider>
<div
className={cn(
" flex min-h-[200px] flex-col bg-background ",
className,
)}
>
{showHtml ? (
<Textarea
className="flex-1 border-none"
value={htmlContent}
onChange={(e) => {
setHtmlContent(e.target.value);
onChange?.(e.target.value, "");
}}
onBlur={updateEditorFromHtml}
/>
) : (
<>
- <RichTextStylesProvider>
<EditorContent className="p-2" editor={editor} />
- </RichTextStylesProvider>
</>
)}
</div>
</RichTextStylesProvider>
</div>Committable suggestion skipped: line range outside the PR's diff.
#264 closee
Summary by CodeRabbit
New Features
Documentation
Style
Chores