-
-
Notifications
You must be signed in to change notification settings - Fork 37
feat(cms): migrate editor to tiptap v3 #237
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
feat(cms): migrate editor to tiptap v3 #237
Conversation
- Remove Novel wrapper dependency and install Tiptap v3 packages - Add @tiptap/react, @tiptap/core, @tiptap/pm (v3.7.1) - Add extensions: starter-kit, text-align, image, youtube, horizontal-rule - Add list extensions (TaskList, TaskItem), code-block-lowlight - Add @tiptap/extensions (CharacterCount, Placeholder) - Replace Tippy.js with @floating-ui/dom for menu positioning - Update extensions.ts to use Tiptap v3 APIs - Replace Novel imports with direct Tiptap extension imports - Update Image extension (remove UploadImagesPlugin wrapper) - Remove redundant extensions (Link, Underline now in StarterKit v3) - Add CharacterCount extension to default extensions array - Migrate editor-page.tsx to Tiptap v3 patterns - Replace Novel's EditorRoot with React fragment - Replace EditorContent configuration with useEditor hook - Update editor initialization with v3 config (shouldRerenderOnTransaction) - Simplify EditorContent to single editor prop - Remove BubbleMenu and SlashCommandMenu temporarily (will reimplement) - Update TypeScript types across codebase - Replace EditorInstance type with Editor from @tiptap/core - Update editor-sidebar.tsx, analysis-tab.tsx, readability-suggestions.tsx - Update use-readability.ts and readability.ts hooks/utils - Update text-buttons.tsx to use useCurrentEditor from @tiptap/react BREAKING CHANGE: - Removed Novel dependency (was Tiptap v2 wrapper) - Temporarily disabled bubble menu and slash commands (pending reimplementation) - Image upload functionality needs migration to FileHandler API Next steps: - Implement slash commands, image upload, and bubble menu with Floating UI
Complete migration from Novel v1.0.2 (Tiptap v2 wrapper) to native Tiptap v3.7.2, providing full control over editor implementation and modernizing the architecture. BREAKING CHANGE: - Remove Novel dependency entirely - Replace EditorProvider with useEditor hook + EditorContext pattern - Update BubbleMenu to use Tiptap v3 API (Floating UI instead of Tippy.js) Editor Core: - Install @tiptap/react@^3.7.2, @tiptap/core@^3.7.2, and related v3 extensions - Migrate editor-page.tsx from EditorProvider to useEditor hook - Wrap with EditorContext.Provider for context sharing - Pass editor instance directly to EditorContent component - Update all editor props and event handlers for v3 API Extensions & Features: - Implement custom slash command system using @tiptap/suggestion + Floating UI - Configure slash menu with 11 commands (text, headings, lists, code, tasks, image, YouTube) - Add proper keyboard navigation (arrow keys, enter, escape) - Update BubbleMenu to import from @tiptap/react/menus (remove tippyOptions) - Configure Placeholder extension with "Press '/' for commands" text - Set placeholder to show only when editable and on current empty node Type System: - Replace all Novel EditorInstance types with @tiptap/core Editor type - Update useCurrentEditor hook across all child components: - text-buttons.tsx - link-selector.tsx - image-upload-modal.tsx - youtube-embed-modal.tsx - editor-sidebar.tsx - analysis-tab.tsx - readability-suggestions.tsx - Add proper ReactRenderer generic types for slash command ref Dependencies: - Add @tiptap/extension-underline@^3.7.1 - Add @tiptap/extension-code-block@^3.7.1 - Update @tiptap/pm@^3.7.2 - Remove Novel-specific imports and wrappers Known Issue: - Image and YouTube modals from slash command menu need editor prop passing (modals use useCurrentEditor but are rendered outside EditorContext)
- Added hooks for file upload, including useFileUpload, useUploader, and useDropZone. - Created ImageUploader component to handle image uploads via drag-and-drop or file input. - Developed ImageUploadView to replace image upload node with actual image upon successful upload. - Introduced YouTube upload functionality with YouTubeInput for embedding videos from URLs. - Implemented YouTubeUploadView to manage YouTube video embedding and cancellation.
- Updated YouTubeUploadView to enhance code formatting and maintainability. - Refactored ImageUploadModal for better structure and clarity. - Improved SlashCommand extension by organizing imports and enhancing readability. - Streamlined slash command items for better organization and consistency. - Enhanced SlashCommandMenu for improved key handling and rendering. - Refactored TextButtons to define formatting items outside the component for performance. - Updated YoutubeEmbedModal for cleaner code structure.
- Implemented ContentTypePickerComponent for selecting content types in the editor. - Created FloatingPortalContext for managing floating elements in the editor. - Developed DropdownButton component for slash command menu interactions. - Defined command groups and actions for formatting and inserting content. - Built MenuList component to display commands in a structured manner. - Added Surface component for consistent styling of dropdown surfaces. - Established types for command groups and commands to enhance type safety.
- add TableColumnMenu for managing columns (add before/after, delete) - add TableRowMenu for managing rows (add before/after, delete) - implement isColumnGripSelected and isRowGripSelected utilities - create index export for TableColumn and TableRow menus - add styles for table, column grips, and row grips to improve UI/UX
…tions - Added `useDragActions` hook to handle actions like resetting formatting, duplicating, copying, deleting nodes, and adding new nodes. - Introduced `useDragData` hook to manage the current node and its position during drag events. - Created `DragHandle` component to provide a UI for drag-and-drop interactions, including buttons for each action and a popover menu for additional options. - Integrated Tiptap's drag handle extension for better drag-and-drop support within the editor. - Fixed inconsistent node positioning during drag events in `useDragData`. - Resolved issues with context menu actions (reset formatting, duplicate, copy, delete) not triggering correctly. - Enhanced `DragHandle` responsiveness and ensured Tiptap drag handle extension integrates smoothly.
…prove drag handle memoization
Add multiple image insertion methods and improve bubble menu behavior: - Add URL embed functionality with validation to ImageUploader - Add media gallery drawer for selecting from uploaded images - Add three action buttons (Upload, Embed URL, Media) for clear UX - Hide bubble menu when images are selected (image and imageUpload nodes) - Remove default placeholder content from editor The ImageUploader now supports: 1. File upload via drag-and-drop or file picker 2. Embedding images from URL with validation 3. Selecting from media library gallery
…tiptap-v3-migration
- Change from vertical to horizontal single-row layout for all controls - Standardize buttons to ghost variant with color accents (green for confirm, red for delete) - Add visual separator between action and utility buttons - Add 'Open link' button and improve 'Toggle new tab' button - Add tooltips to all buttons for better discoverability
- Add YouTube and YouTubeUpload to bubble menu hide conditions - Prevent bubble menu from appearing over video elements - Apply code formatting fixes
…age upload handling
Improved type consistency and stability in the slash command extension and menu list by updating imperative handle and event types. Enhanced table selection utility to handle undefined cells, preventing potential runtime errors.
…tiptap-v3-migration
…tiptap-v3-migration
|
@YungKingJayy is attempting to deploy a commit to the Marble Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughMigrated CMS editor to TipTap v3, replaced EditorInstance types with @tiptap/core Editor, added many new extensions (figures, image/YouTube upload, tables, columns, markdown paste/drop, slash command), new upload UI/hooks, drag-handle, pickers, Floating portal support, Radix primitives refactor, styles, and removed legacy upload/slash modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Editor UI
participant Editor as TipTap Editor
participant Ext as Extensions / NodeViews
participant Store as pendingUploads / Form
User->>UI: type, select toolbar, or drop file
UI->>Editor: editor.chain().focus().<command>().run()
Editor->>Ext: extension/plugin handles command
Ext->>Editor: apply transaction (insert/replace node)
Editor->>Store: update content / pendingUploads
Editor->>UI: re-render view
alt Image upload flow
User->>UI: drop/choose file
UI->>Store: pendingUploads.set(fileId, File)
UI->>Editor: insert imageUpload placeholder(fileId)
Ext->>UI: ImageUploader uploads file -> returns URL
Ext->>Editor: replace placeholder with Figure (setFigure)
Editor->>UI: render figure with alt/caption
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
|
@coderabbitai review |
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cms/src/components/editor/link-selector.tsx (1)
116-120: Addrel="noopener noreferrer"to bothsetLink()calls to prevent reverse tabnabbing.The Link extension is not configured with a default
relattribute in your extension setup (onlylink: { openOnClick: false }via StarterKit). Whentarget="_blank"is set withoutrel="noopener noreferrer", the opened page can accesswindow.opener, creating a security vulnerability.Update both calls in link-selector.tsx:
.setLink({ href: url, target: openInNewTab ? "_blank" : "_self", + rel: openInNewTab ? "noopener noreferrer" : undefined, })Lines 116-120 and 167-171.
♻️ Duplicate comments (9)
apps/cms/src/components/editor/link-selector.tsx (1)
22-42: Critical security issues from previous review remain unaddressed.The previously flagged security vulnerabilities are still present:
- Lines 22-29:
isValidUrlaccepts any URL scheme, includingjavascript:,data:,vbscript:, etc.- Lines 31-42:
getUrlFromStringdoesn't validate the protocol of normalized URLs.- Line 219:
window.openis missing"noopener,noreferrer"in the features parameter, enabling reverse tabnabbing attacks.Please apply the fixes from the previous review:
+const ALLOWED_PROTOCOLS = new Set(["http:", "https:", "mailto:", "tel:"]); + export function isValidUrl(url: string) { try { - new URL(url); - return true; + const u = new URL(url); + return ALLOWED_PROTOCOLS.has(u.protocol); } catch (_e) { return false; } } export function getUrlFromString(str: string) { - if (isValidUrl(str)) { - return str; - } + if (isValidUrl(str)) return str; try { if (str.includes(".") && !str.includes(" ")) { - return new URL(`https://${str}`).toString(); + const normalized = new URL(`https://${str}`).toString(); + return isValidUrl(normalized) ? normalized : null; } } catch (_e) { - return null; + /* noop */ } + return null; }And:
if (url) { - window.open(url, "_blank"); + window.open(url, "_blank", "noopener,noreferrer"); }Also applies to: 217-221
apps/cms/src/components/editor/content-type-picker.tsx (1)
158-164: Critical: Unsafe cast before null check (already flagged).The cast
editor as Editoron line 159 occurs before the null check on line 166, risking a runtime crash. This issue was already identified in a previous review.Please apply the fix suggested in the previous review comment.
apps/cms/src/styles/editor.css (1)
7-9: Major: Horizontal overflow from excessive width (already flagged).The
width: 1000vwon line 8 causes page-wide horizontal scrollbars. This issue was identified in a previous review.Please apply the fix suggested in the previous review comment.
apps/cms/src/components/editor/editor-page.tsx (2)
44-52: Hooks misuse: remove useMemo from non-hook function.
useMemoinsidegetToggleSidebarShortcutviolates Rules of Hooks.Apply this diff:
const getToggleSidebarShortcut = () => { - const isMac = useMemo( - () => - typeof navigator !== "undefined" && - navigator.platform.toUpperCase().indexOf("MAC") >= 0, - [] - ); + const isMac = + typeof navigator !== "undefined" && /mac/i.test(navigator.platform ?? ""); return isMac ? "⌘K" : "Ctrl+K"; };
200-217: Add requestIdleCallback/cancelIdleCallback fallbacks (Safari/older browsers).Not all browsers implement these APIs. Provide setTimeout/clearTimeout fallbacks.
- // Cancel previous idle callback if exists + // Cancel previous idle callback if exists if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + if (typeof cancelIdleCallback === "function") { + cancelIdleCallback(idleCallbackId.current as unknown as number); + } else { + clearTimeout(idleCallbackId.current as unknown as number); + } } ... - // Schedule serialization when browser is idle - idleCallbackId.current = requestIdleCallback( - () => { + // Schedule serialization when browser is idle (fallback to setTimeout) + const schedule = + typeof requestIdleCallback === "function" + ? (cb: IdleRequestCallback) => requestIdleCallback(cb, { timeout: 100 }) + : (cb: () => void) => window.setTimeout(cb, 100) as unknown as number; + idleCallbackId.current = schedule(() => { if (contentChangedRef.current && editorRef.current) { const html = editorRef.current.getHTML(); const json = editorRef.current.getJSON(); onUpdateRef.current?.(html, json); contentChangedRef.current = false; } - }, - { timeout: 100 } - ); + });And in cleanup:
if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + if (typeof cancelIdleCallback === "function") { + cancelIdleCallback(idleCallbackId.current as unknown as number); + } else { + clearTimeout(idleCallbackId.current as unknown as number); + } }Also applies to: 222-230
apps/cms/src/components/editor/bubble-menu.tsx (1)
12-15: Fix formatting to satisfy Prettier.Run the formatter to resolve CI failures for this file.
Suggested: pnpm prettier --write apps/cms/src/components/editor/bubble-menu.tsx
apps/cms/src/components/editor/extensions.ts (3)
69-75: YouTube options misplaced under HTMLAttributesThis issue was previously flagged. The
controlsandnocookieoptions are extension-level configuration, not HTML attributes, so they won't take effect in their current position.Apply this diff:
const youtube = Youtube.configure({ + nocookie: true, + controls: false, HTMLAttributes: { class: cx("w-full aspect-video"), - controls: false, - nocookie: true, }, });
8-8: TaskList/TaskItem import path incorrect for TipTap v3This issue was previously flagged. In TipTap v3, TaskList and TaskItem are separate packages, not exported from
@tiptap/extension-list.Apply this diff:
-import { TaskItem, TaskList } from "@tiptap/extension-list"; +import TaskList from "@tiptap/extension-task-list"; +import TaskItem from "@tiptap/extension-task-item";Ensure
@tiptap/extension-task-listand@tiptap/extension-task-itemare in package.json dependencies.
146-146: Duplicate Dropcursor registrationThis issue was previously flagged. Dropcursor is configured in StarterKit (lines 110-113) but also added twice explicitly to the extensions array. This causes duplicate plugin registration and unpredictable behavior.
Remove both explicit Dropcursor entries:
CodeBlockLowlightEx as unknown as Extension, - Dropcursor as unknown as Extension, tiptapImage as unknown as Extension,SlashCommand as unknown as Extension, // DragHandle as unknown as Extension, NodeRange as unknown as Extension, - Dropcursor as unknown as Extension, ];The Dropcursor configuration in StarterKit (lines 110-113) is sufficient.
Also applies to: 166-166
🧹 Nitpick comments (15)
apps/cms/src/components/editor/image-upload-modal.tsx (2)
87-94: Remove redundant editor check.The
if (editor)check on line 87 is unnecessary since the function already returns early on line 79 ifeditoris falsy.Apply this diff to simplify the code:
const img = new window.Image(); img.onload = () => { - if (editor) { - editor - .chain() - .focus() - .setImage({ src: url }) - .createParagraphNear() - .run(); - } + editor + .chain() + .focus() + .setImage({ src: url }) + .createParagraphNear() + .run(); setIsOpen(false); setEmbedUrl(""); setIsValidatingUrl(false);
84-107: Add cleanup for image validation event listeners.The image validation creates event listeners (onload/onerror) that can fire after component unmount, potentially causing state updates on unmounted components. While React 18+ handles this better, it's still not ideal.
Consider using an abort signal or ref to track mount state:
const handleEmbed = async (url: string) => { if (!url || !editor) { return; } try { setIsValidatingUrl(true); const img = new window.Image(); let mounted = true; img.onload = () => { if (!mounted) return; editor .chain() .focus() .setImage({ src: url }) .createParagraphNear() .run(); setIsOpen(false); setEmbedUrl(""); setIsValidatingUrl(false); }; img.onerror = () => { if (!mounted) return; toast.error("Invalid image URL"); setIsValidatingUrl(false); }; img.src = url; return () => { mounted = false; }; } catch (_error) { toast.error("Failed to embed image"); setIsValidatingUrl(false); } };Alternatively, wrap the validation in a useEffect and return the cleanup function, or add a timeout for slow-loading images.
apps/cms/src/lib/utils/getRenderContainer.ts (3)
3-43: Add explicit return type annotation.The function lacks an explicit return type. Based on the implementation, it returns
Element | HTMLElement | null, but this should be made explicit for type safety and clarity.-export const getRenderContainer = (editor: Editor, nodeType: string) => { +export const getRenderContainer = (editor: Editor, nodeType: string): HTMLElement | null => {Note: You may need to adjust the return type if line 21 can return a non-HTMLElement. Consider casting or ensuring all return paths produce
HTMLElement | null.
27-29: Preferinstanceoffor node type checking.Using
!container.tagNameto detect text nodes is fragile. Consider using explicit type guards for clarity.- if (!container.tagName) { + if (!(container instanceof HTMLElement)) { container = node.parentElement; }
31-40: Eliminate duplicategetAttributecall.The
data-typeattribute is retrieved twice in the loop condition, which is slightly inefficient.+ let dataType: string | null; while ( container && !( - container.getAttribute("data-type") && - container.getAttribute("data-type") === nodeType + (dataType = container.getAttribute("data-type")) && + dataType === nodeType ) && !container.classList.contains(nodeType) ) { container = container.parentElement; }apps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx (1)
69-69: Optimize: Memoize the URL validation check.Line 69 calls
extractYouTubeVideoId(url)on every render to computeisValidUrl. Since this is used only for the button's disabled state, memoize it to avoid redundant computation.Apply this diff:
+ const isValidUrl = useMemo( + () => extractYouTubeVideoId(url) !== null, + [url] + ); - const isValidUrl = extractYouTubeVideoId(url) !== null;apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx (1)
57-70: Refactor: Remove non-null assertion with safer pattern.Line 58 uses
workspaceId!with a justification comment, but theenabled: !!workspaceIdguard on line 69 makes the assertion unnecessary. Consider making the queryKey conditional or using a type guard.Apply this diff:
const { data: media } = useQuery({ - // biome-ignore lint/style/noNonNullAssertion: workspaceId is required for media query - queryKey: QUERY_KEYS.MEDIA(workspaceId!), + queryKey: workspaceId ? QUERY_KEYS.MEDIA(workspaceId) : ["media", "null"], staleTime: 1000 * 60 * 60, queryFn: async () => { try { const res = await fetch("/api/media"); const data: MediaListResponse = await res.json(); return data.media; } catch (_error) { return []; } }, enabled: !!workspaceId, });apps/cms/src/components/editor/extensions/multi-column/Columns.ts (1)
41-54: Optional: Format the HTML string for readability.Lines 46-48 contain a long HTML string that's hard to read. Consider formatting it across multiple lines or using a template function.
Apply this diff:
addCommands() { return { setColumns: () => ({ commands }: { commands: Editor["commands"] }) => - commands.insertContent( - `<div data-type="columns"><div data-type="column" data-position="left"><p></p></div><div data-type="column" data-position="right"><p></p></div></div>` - ), + commands.insertContent(` + <div data-type="columns"> + <div data-type="column" data-position="left"><p></p></div> + <div data-type="column" data-position="right"><p></p></div> + </div> + `.trim()), setLayout: (layout: ColumnLayout) => ({ commands }: { commands: Editor["commands"] }) => commands.updateAttributes("columns", { layout }), }; },apps/cms/src/components/editor/slash-command/menu-list.tsx (4)
30-45: Stabilize selectItem deps to avoid unnecessary re-creations.
useCallback(..., [props])changes whenever any prop identity changes. Depend only on what’s used.Apply this diff:
- const selectItem = useCallback( + const selectItem = useCallback( (groupIndex: number, commandIndex: number) => { - const group = props.items[groupIndex]; + const group = props.items[groupIndex]; if (!group) return; - const command = group.commands[commandIndex]; + const command = group.commands[commandIndex]; if (!command) return; - props.command(command); + props.command(command); }, - [props] + [props.items, props.command] );
110-117: Prefer scrollIntoView for correctness across container sizes.Manually setting
scrollToptooffsetTop - offsetHeightcan misalign. UsescrollIntoView({ block: "nearest" }).- const offsetTop = activeItem.current.offsetTop; - const offsetHeight = activeItem.current.offsetHeight; - scrollContainer.current.scrollTop = offsetTop - offsetHeight; + activeItem.current.scrollIntoView({ block: "nearest" });
136-154: Ensure React keys are unique and stable across groups/commands.
key={group.title}andkey={command.label}can collide across groups. Use composite keys.- <div className="mb-4 last:mb-0" key={`${group.title}-wrapper`}> + <div className="mb-4 last:mb-0" key={`group-${group.title}-${groupIndex}`}> ... - className="mx-2 mt-4 select-none font-semibold text-[0.65rem] text-muted-foreground uppercase tracking-wider first:mt-0.5" - key={group.title} + className="mx-2 mt-4 select-none font-semibold text-[0.65rem] text-muted-foreground uppercase tracking-wider first:mt-0.5" + key={`group-title-${group.title}-${groupIndex}`} ... - key={command.label} + key={`cmd-${group.title}-${command.label}-${groupIndex}-${commandIndex}`}If the schema provides stable ids (e.g.,
group.id,command.id), prefer those over labels. As per coding guidelines.Also applies to: 150-164
14-17: Optional: Memoize MenuList to cut re-renders from parent prop identity changes.Wrapping with
memohelps whenpropsidentities change often (e.g., from TipTap Suggestion updates).-import { - forwardRef, +import { + forwardRef, + memo, ... -export const MenuList = forwardRef< +export const MenuList = forwardRef< { onKeyDown: (props: { event: KeyboardEvent }) => boolean }, MenuListProps >((props, ref) => { ... }); MenuList.displayName = "MenuList"; -export default MenuList; +export default memo(MenuList);Also applies to: 178-180
apps/cms/src/components/editor/bubble-menu.tsx (2)
70-79: Append to the local container instead of document.body.Use the existing
containerRefso the bubble menu portals alongside related UI and inherits z-index/stacking context.- <TiptapBubbleMenu - appendTo={() => document.body} + <TiptapBubbleMenu + appendTo={() => containerRef.current ?? document.body} className="z-50 flex h-fit w-fit gap-0.5 overflow-hidden rounded-lg border bg-background p-1 shadow-sm"
16-64: Use the passed state for selection checks (avoid editor.state coupling).
shouldShowreceivesstate; preferstate.selectionovereditor.state.selectionto match TipTap’s lifecycle and avoid stale reads. Type it or cast narrowly.- const shouldShow = useCallback( - ({ - view, - state, - from, - }: { - view: unknown; - state: unknown; - from: number; - }) => { + const shouldShow = useCallback(({ view, state, from }: { view: unknown; state: unknown; from: number }) => { if (!editor || !state) return false; ... - // Show for normal text selection - return !editor.state.selection.empty; + // Show for normal text selection + return !(state as { selection: { empty: boolean } }).selection.empty; }, [editor] );Also applies to: 60-63
apps/cms/src/components/editor/extensions/table/utils.ts (1)
82-117: DRY: unify row/column helpers; trim minor verbosity.
getCellsInColumnandgetCellsInRowshare structure. Consider a single parametric helper to reduce duplication; also avoidArray.from([x])where[x]suffices.Example:
function getCellsByRect( selection: Selection, indices: number | number[], kind: "row" | "column" ) { /* compute with a shared rect builder */ } export const getCellsInColumn = (i: number | number[]) => (s: Selection) => getCellsByRect(s, i, "column"); export const getCellsInRow = (i: number | number[]) => (s: Selection) => getCellsByRect(s, i, "row");And micro cleanups:
-const indexes = Array.isArray(columnIndex) ? columnIndex : Array.from([columnIndex]); +const indexes = Array.isArray(columnIndex) ? columnIndex : [columnIndex];Also applies to: 119-155
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/cms/src/components/editor/bubble-menu.tsx(1 hunks)apps/cms/src/components/editor/content-type-picker.tsx(1 hunks)apps/cms/src/components/editor/editor-page.tsx(6 hunks)apps/cms/src/components/editor/extensions.ts(3 hunks)apps/cms/src/components/editor/extensions/figure/figure-view.tsx(1 hunks)apps/cms/src/components/editor/extensions/figure/index.ts(1 hunks)apps/cms/src/components/editor/extensions/image-upload/image-upload-view.tsx(1 hunks)apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx(1 hunks)apps/cms/src/components/editor/extensions/image-upload/index.ts(1 hunks)apps/cms/src/components/editor/extensions/multi-column/Columns.ts(1 hunks)apps/cms/src/components/editor/extensions/multi-column/index.ts(1 hunks)apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx(1 hunks)apps/cms/src/components/editor/extensions/multi-column/menus/index.ts(1 hunks)apps/cms/src/components/editor/extensions/table/index.ts(1 hunks)apps/cms/src/components/editor/extensions/table/menus/index.tsx(1 hunks)apps/cms/src/components/editor/extensions/table/utils.ts(1 hunks)apps/cms/src/components/editor/extensions/youtube-upload/index.ts(1 hunks)apps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx(1 hunks)apps/cms/src/components/editor/image-toolbar.tsx(1 hunks)apps/cms/src/components/editor/image-upload-modal.tsx(3 hunks)apps/cms/src/components/editor/link-selector.tsx(3 hunks)apps/cms/src/components/editor/slash-command/dropdown-button.tsx(1 hunks)apps/cms/src/components/editor/slash-command/index.ts(1 hunks)apps/cms/src/components/editor/slash-command/menu-list.tsx(1 hunks)apps/cms/src/components/editor/slash-command/surface.tsx(1 hunks)apps/cms/src/lib/utils/getRenderContainer.ts(1 hunks)apps/cms/src/styles/editor.css(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/cms/src/components/editor/slash-command/index.ts
- apps/cms/src/components/editor/extensions/figure/figure-view.tsx
- apps/cms/src/components/editor/extensions/multi-column/menus/index.ts
- apps/cms/src/components/editor/extensions/figure/index.ts
- apps/cms/src/components/editor/extensions/image-upload/index.ts
- apps/cms/src/components/editor/image-toolbar.tsx
- apps/cms/src/components/editor/extensions/youtube-upload/index.ts
- apps/cms/src/components/editor/extensions/table/index.ts
- apps/cms/src/components/editor/extensions/image-upload/image-upload-view.tsx
- apps/cms/src/components/editor/extensions/table/menus/index.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...
Files:
apps/cms/src/components/editor/extensions/multi-column/index.tsapps/cms/src/components/editor/image-upload-modal.tsxapps/cms/src/lib/utils/getRenderContainer.tsapps/cms/src/components/editor/extensions/multi-column/Columns.tsapps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/content-type-picker.tsxapps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/extensions/image-upload/image-uploader.tsxapps/cms/src/components/editor/slash-command/dropdown-button.tsxapps/cms/src/components/editor/extensions.tsapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/slash-command/surface.tsxapps/cms/src/components/editor/bubble-menu.tsxapps/cms/src/components/editor/extensions/table/utils.tsapps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types
Files:
apps/cms/src/components/editor/extensions/multi-column/index.tsapps/cms/src/components/editor/image-upload-modal.tsxapps/cms/src/lib/utils/getRenderContainer.tsapps/cms/src/components/editor/extensions/multi-column/Columns.tsapps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/content-type-picker.tsxapps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/extensions/image-upload/image-uploader.tsxapps/cms/src/components/editor/slash-command/dropdown-button.tsxapps/cms/src/components/editor/extensions.tsapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/slash-command/surface.tsxapps/cms/src/components/editor/bubble-menu.tsxapps/cms/src/components/editor/extensions/table/utils.tsapps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...Files:
apps/cms/src/components/editor/image-upload-modal.tsxapps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/content-type-picker.tsxapps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/extensions/image-upload/image-uploader.tsxapps/cms/src/components/editor/slash-command/dropdown-button.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/slash-command/surface.tsxapps/cms/src/components/editor/bubble-menu.tsxapps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't destructure props inside JSX components in Solid projects
Files:
apps/cms/src/components/editor/image-upload-modal.tsxapps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/content-type-picker.tsxapps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/extensions/image-upload/image-uploader.tsxapps/cms/src/components/editor/slash-command/dropdown-button.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/slash-command/surface.tsxapps/cms/src/components/editor/bubble-menu.tsxapps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx🧬 Code graph analysis (11)
apps/cms/src/components/editor/image-upload-modal.tsx (3)
apps/cms/src/hooks/use-workspace-id.ts (1)
useWorkspaceId(7-10)apps/cms/src/lib/media/upload.ts (1)
uploadFile(66-85)apps/cms/src/types/media.ts (1)
Media(15-22)apps/cms/src/components/editor/extensions/multi-column/Columns.ts (1)
apps/cms/src/components/editor/extensions/multi-column/index.ts (2)
ColumnLayout(3-3)Columns(3-3)apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx (2)
apps/cms/src/components/editor/extensions/multi-column/Columns.ts (2)
ColumnLayout(4-8)ColumnLayout(10-10)apps/cms/src/components/editor/extensions/multi-column/menus/index.ts (1)
ColumnsMenu(2-2)apps/cms/src/components/editor/editor-page.tsx (7)
apps/cms/src/components/editor/extensions.ts (1)
defaultExtensions(135-167)apps/cms/src/components/editor/drag-handle/index.tsx (1)
DragHandle(155-155)apps/cms/src/components/editor/bubble-menu.tsx (1)
BubbleMenu(88-88)apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx (1)
TableRowMenu(100-100)apps/cms/src/components/editor/extensions/table/menus/TableColumn/index.tsx (1)
TableColumnMenu(118-118)apps/cms/src/components/editor/extensions/multi-column/menus/index.ts (1)
ColumnsMenu(2-2)apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx (1)
ColumnsMenu(139-139)apps/cms/src/components/editor/content-type-picker.tsx (3)
apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)packages/ui/src/components/dropdown-menu.tsx (6)
DropdownMenu(238-238)DropdownMenuTrigger(240-240)DropdownMenuContent(241-241)DropdownMenuSeparator(248-248)DropdownMenuLabel(243-243)DropdownMenuItem(244-244)packages/ui/src/components/button.tsx (1)
Button(58-58)apps/cms/src/components/editor/slash-command/menu-list.tsx (4)
apps/cms/src/components/editor/slash-command/types.ts (1)
MenuListProps(20-24)apps/cms/src/components/editor/slash-command/surface.tsx (1)
Surface(11-29)apps/cms/src/components/shared/icons.tsx (1)
Icon(3-3)apps/cms/src/components/editor/slash-command/dropdown-button.tsx (1)
DropdownButton(5-36)apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx (4)
apps/cms/src/hooks/use-workspace-id.ts (1)
useWorkspaceId(7-10)apps/cms/src/components/editor/extensions/image-upload/hooks.ts (3)
useUploader(16-44)useFileUpload(6-14)useDropZone(46-99)apps/cms/src/lib/queries/keys.ts (1)
QUERY_KEYS(1-54)apps/cms/src/types/media.ts (1)
MediaListResponse(31-35)apps/cms/src/components/editor/extensions.ts (11)
apps/cms/src/components/editor/extensions/document/Document.ts (1)
Document(3-5)apps/cms/src/components/editor/extensions/figure/index.ts (1)
Figure(20-108)apps/cms/src/components/editor/extensions/image-upload/index.ts (1)
ImageUpload(18-93)apps/cms/src/components/editor/extensions/youtube-upload/index.ts (1)
YouTubeUpload(15-48)apps/cms/src/components/editor/extensions/table/Table.ts (1)
Table(3-6)apps/cms/src/components/editor/extensions/table/TableRow.ts (1)
TableRow(3-6)apps/cms/src/components/editor/extensions/table/TableCell.ts (1)
TableCell(11-134)apps/cms/src/components/editor/extensions/table/TableHeader.ts (1)
TableHeader(7-97)apps/cms/src/components/editor/extensions/multi-column/Columns.ts (1)
Columns(22-71)apps/cms/src/components/editor/extensions/multi-column/Column.ts (1)
Column(3-35)apps/cms/src/components/editor/slash-command/index.ts (1)
SlashCommand(15-260)apps/cms/src/components/editor/link-selector.tsx (2)
apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/cms/src/components/editor/bubble-menu.tsx (3)
apps/cms/src/components/editor/extensions/table/menus/TableRow/utils.ts (1)
isRowGripSelected(7-39)apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts (1)
isColumnGripSelected(7-39)apps/cms/src/components/editor/floating-portal-context.tsx (1)
FloatingPortalProvider(13-25)apps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx (1)
apps/cms/src/components/editor/link-selector.tsx (1)
isValidUrl(22-29)🔇 Additional comments (16)
apps/cms/src/components/editor/image-upload-modal.tsx (2)
32-48: Well-designed editor resolution pattern.The dual-source editor resolution (prop vs context) provides flexibility while maintaining backwards compatibility. The fallback pattern
editorProp || editorFromContextensures the component works both standalone and within editor context.
51-76: LGTM: Upload flow is well-structured.The mutation properly handles success/error cases, uses optional chaining defensively, and correctly invalidates the media cache. The command chain (focus → setImage → createParagraphNear → run) follows TipTap patterns.
apps/cms/src/components/editor/extensions/youtube-upload/youtube-input.tsx (1)
9-27: LGTM: YouTube ID extraction logic is sound.The function correctly handles multiple YouTube URL formats and raw video IDs.
apps/cms/src/components/editor/content-type-picker.tsx (2)
28-44: LGTM: Type definitions are well-structured.The type hierarchy clearly distinguishes between options and categories, making the CONTENT_TYPES array type-safe.
46-152: LGTM: Content type definitions are comprehensive.The array properly categorizes hierarchy and list options with appropriate TipTap commands for each transformation.
apps/cms/src/styles/editor.css (3)
45-45: Verify: CSS relative color syntax browser support.Lines 45 and 53 use
hsl(from var(--muted-foreground) h s l / 0.5), which is CSS relative color syntax. This requires Chrome 119+, Safari 16.4+, or Firefox 128+. Verify that your target browsers support this feature, or use a fallback.If broader browser support is needed, consider using a preprocessor or CSS variable:
.ProseMirror .is-editor-empty:first-child::before { color: rgba(from var(--muted-foreground) 0.5); /* or use a dedicated CSS variable */ }Also applies to: 53-53
73-91: LGTM: Figure and caption styles are appropriate.The styling provides good visual hierarchy for figure elements with proper spacing and typography.
229-323: LGTM: Columns styling is comprehensive.The grid layouts, hover states, and dark mode support are well-implemented. The aggressive hiding of direct paragraph children (line 274) is noted as intentional per the comment.
apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx (1)
1-32: LGTM: Imports and validation schema are appropriate.The zod URL schema provides proper validation for embedded image URLs.
apps/cms/src/components/editor/extensions/multi-column/Columns.ts (2)
4-10: LGTM: ColumnLayout uses proper const assertion pattern.The constant object with
as constand derived type alias provides type-safe enum-like behavior.
12-20: LGTM: Command interface augmentation is correct.The module augmentation properly extends TipTap's Commands interface with the new columns commands.
apps/cms/src/components/editor/slash-command/surface.tsx (1)
1-31: LGTM: Surface component is well-implemented.The forwardRef pattern is appropriate, and the component provides clean defaults for shadow and border props with proper className composition.
apps/cms/src/components/editor/extensions/multi-column/index.ts (1)
1-3: LGTM: Barrel file provides clean API surface.The exports consolidate the multi-column extension's public API in a single location.
apps/cms/src/components/editor/slash-command/dropdown-button.tsx (1)
1-38: LGTM: DropdownButton component is well-structured.The forwardRef pattern is appropriate, and the component properly handles active, disabled, and hover states with good accessibility practices (type="button", disabled attribute).
apps/cms/src/components/editor/extensions.ts (2)
28-42: LGTM!The placeholder configuration correctly hides placeholders inside table cells and shows context-appropriate hints. The table detection using
isActiveis appropriate.
119-133: LGTM!The FileHandler configuration correctly processes image drops and pastes by delegating to the ImageUpload extension. The allowed MIME types are appropriate for web images.
apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx
Show resolved
Hide resolved
apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx
Show resolved
Hide resolved
apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx
Show resolved
Hide resolved
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: 12
♻️ Duplicate comments (9)
apps/cms/src/components/editor/color-picker.tsx (1)
27-29: Resolved: input state now syncs withcolorprop.The useEffect fixes stale input on external updates.
apps/cms/src/components/editor/editor-page.tsx (2)
44-52: Invalid hook usage: remove useMemo inside non-hook function.Hooks can’t be called in plain helpers. Compute synchronously.
-const getToggleSidebarShortcut = () => { - const isMac = useMemo( - () => - typeof navigator !== "undefined" && - navigator.platform.toUpperCase().indexOf("MAC") >= 0, - [], - ); - return isMac ? "⌘K" : "Ctrl+K"; -}; +const getToggleSidebarShortcut = () => { + const isMac = + typeof navigator !== "undefined" && /mac/i.test(navigator.platform ?? ""); + return isMac ? "⌘K" : "Ctrl+K"; +};
182-221: Idle scheduling lacks Safari/legacy fallback.requestIdleCallback/cancelIdleCallback aren’t universal. Add a safe fallback to setTimeout/clearTimeout and use it for schedule/cancel.
- const contentChangedRef = useRef(false); - const idleCallbackId = useRef<number | null>(null); + const contentChangedRef = useRef(false); + const idleCallbackId = useRef<number | null>(null); const editorRef = useRef<Editor | null>(null); @@ - // Cancel previous idle callback if exists + // Cancel previous idle callback if exists if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + // Fallback cancel + if ("cancelIdleCallback" in window) { + (window as any).cancelIdleCallback(idleCallbackId.current); + } else { + clearTimeout(idleCallbackId.current); + } } @@ - // Schedule serialization when browser is idle - idleCallbackId.current = requestIdleCallback( - () => { + // Schedule serialization when browser is idle (fallback to setTimeout) + const schedule = + "requestIdleCallback" in window + ? (cb: IdleRequestCallback) => + (window as any).requestIdleCallback(cb, { timeout: 100 }) + : (cb: () => void) => + window.setTimeout(cb, 100) as unknown as number; + idleCallbackId.current = schedule(() => { if (contentChangedRef.current && editorRef.current) { const html = editorRef.current.getHTML(); const json = editorRef.current.getJSON(); onUpdateRef.current?.(html, json); contentChangedRef.current = false; } - }, - { timeout: 100 } - ); + });apps/cms/src/components/editor/text-buttons.tsx (2)
303-307: Focus before applying text color to preserve selection.Mirror the highlight handler; call focus() before setColor.
- <ColorPicker - color={editor.getAttributes("textStyle").color} - onChange={(color) => editor.chain().setColor(color).run()} - onClear={() => editor.chain().focus().unsetColor().run()} - /> + <ColorPicker + color={editor.getAttributes("textStyle").color} + onChange={(color) => + editor.chain().focus().setColor(color).run() + } + onClear={() => editor.chain().focus().unsetColor().run()} + />
270-276: Prettier failure here; format long arrow call.Break across lines to satisfy formatter.
- <ColorPicker - color={editor.getAttributes("highlight").color} - onChange={(color) => editor.chain().focus().setHighlight({ color }).run()} - onClear={() => editor.chain().focus().unsetHighlight().run()} - /> + <ColorPicker + color={editor.getAttributes("highlight").color} + onChange={(color) => + editor.chain().focus().setHighlight({ color }).run() + } + onClear={() => editor.chain().focus().unsetHighlight().run()} + />apps/cms/src/components/editor/link-selector.tsx (4)
22-29: Block unsafe URL schemes + allow safe relatives.Restrict protocols to http/https (optionally mailto/tel) and treat root/hash/query-relative paths as safe. Current new URL(url) accepts javascript: and others.
+const ALLOWED_PROTOCOLS = new Set(["http:", "https:", "mailto:", "tel:"]); export function isValidUrl(url: string) { - try { - new URL(url); - return true; - } catch (_e) { - return false; - } + try { + // Support relative paths/hash/query as valid + if (url.startsWith("/") || url.startsWith("#") || url.startsWith("?")) return true; + // Parse with a base to avoid throwing on relatives + const u = new URL(url, typeof window !== "undefined" ? window.location.origin : "http://localhost"); + return ALLOWED_PROTOCOLS.has(u.protocol); + } catch { + return false; + } }
31-42: Harden URL normalization and re-validate; support relative paths.Trim input, support root/hash/query-relative, and re-check normalized hostnames.
export function getUrlFromString(str: string) { - if (isValidUrl(str)) { - return str; - } - try { - if (str.includes(".") && !str.includes(" ")) { - return new URL(`https://${str}`).toString(); - } - } catch (_e) { - return null; - } + const s = str.trim(); + if (!s) return null; + if (s.startsWith("/") || s.startsWith("#") || s.startsWith("?")) return s; + if (isValidUrl(s)) return s; + try { + if (s.includes(".") && !s.includes(" ")) { + const normalized = new URL(`https://${s}`).toString(); + return isValidUrl(normalized) ? normalized : null; + } + } catch { + /* noop */ + } + return null; }
217-221: Prevent reverse tabnabbing when opening links.Pass noopener,noreferrer when using window.open.
- window.open(url, "_blank"); + window.open(url, "_blank", "noopener,noreferrer");
61-66: Remove the unsafeas Editorcast and follow the TipTap pattern for nullable editors.The review comment is valid. The
useCurrentEditor()hook returnsEditor | null, and theuseEditorStateselector must guard for null before calling editor methods. The unsafe castas Editormasks this nullable type; the proposed change correctly removes it and uses the recommended pattern: destructure{ editor }in the selector and let optional chaining guard against nulls.Apply the suggested diff:
- const isLinkActive = useEditorState({ - editor: editor as Editor, - selector: (ctx) => ctx.editor?.isActive("link") ?? false, - }); + const isLinkActive = useEditorState({ + editor, + selector: ({ editor }) => editor?.isActive("link") ?? false, + });
🧹 Nitpick comments (14)
apps/cms/src/components/editor/slash-command/menu-list.tsx (5)
23-28: Consider destructuring props to eliminate biome-ignore.While the updated comment accurately reflects intent, you can avoid the lint suppression entirely by destructuring
itemsat the component's top:>((props, ref) => { + const { items, command, editor } = props; const scrollContainer = useRef<HTMLDivElement>(null); ... - // Reset selection whenever menu items change - // biome-ignore lint/correctness/useExhaustiveDependencies: intentionally reset only when items change useEffect(() => { setSelectedGroupIndex(0); setSelectedCommandIndex(0); - }, [props.items]); + }, [items]);This pattern is clearer and aligns with React best practices.
30-45: RefineselectItemdependency array to avoid unnecessary recreations.The dependency
[props]causes this callback to be recreated on every render becausepropsis not a stable reference (the parent passes a new object each time viaReactRenderer.updateProps()).Instead, destructure the specific values at the top of the component and depend on those:
>((props, ref) => { + const { items, command, editor } = props; const scrollContainer = useRef<HTMLDivElement>(null); ... const selectItem = useCallback( (groupIndex: number, commandIndex: number) => { - const group = props.items[groupIndex]; + const group = items[groupIndex]; if (!group) { return; } const command = group.commands[commandIndex]; if (!command) { return; } - props.command(command); + command(commandItem); }, - [props] + [items, command] );This reduces unnecessary callback recreation and improves performance.
110-117: Remove or correct misleading biome-ignore comment.The comment claims "selectItem is stable," but:
- The effect doesn't use
selectItemat allselectItemis not stable (it depends on[props]per line 44)The dependency array
[selectedCommandIndex, selectedGroupIndex]is correct for this effect's purpose (scroll active item into view when selection changes). The refs (scrollContainer,activeItem) are correctly excluded since refs don't trigger re-renders.If the linter is not complaining, remove the biome-ignore entirely:
- // biome-ignore lint/correctness/useExhaustiveDependencies: selectItem is stable useEffect(() => { if (activeItem.current && scrollContainer.current) { const offsetTop = activeItem.current.offsetTop; const offsetHeight = activeItem.current.offsetHeight; scrollContainer.current.scrollTop = offsetTop - offsetHeight; } }, [selectedCommandIndex, selectedGroupIndex]);If the linter does complain, update the comment to accurately explain why refs are excluded.
14-17: Consider wrapping withReact.memofor performance.Since the TipTap suggestion plugin calls
ReactRenderer.updateProps()frequently (on every keystroke),MenuListwill re-render often. Wrapping withReact.memocan prevent unnecessary re-renders when props haven't meaningfully changed:-export const MenuList = forwardRef< +const MenuListInner = forwardRef< { onKeyDown: (props: { event: KeyboardEvent }) => boolean }, MenuListProps >((props, ref) => { ... }); -MenuList.displayName = "MenuList"; +MenuListInner.displayName = "MenuList"; + +export const MenuList = React.memo(MenuListInner);This is especially beneficial if
itemsandcommandare memoized by the parent.
136-153: React keys should use stable indices rather than data values for robustness.The current code works because group titles ("Format", "Insert") and command labels are unique in the data. However, using indices with the values provides stronger guarantees:
- <div className="mb-4 last:mb-0" key={`${group.title}-wrapper`}> + <div className="mb-4 last:mb-0" key={`${group.title}-${groupIndex}`}> ... - key={group.title} + key={`${group.title}-${groupIndex}`} ... - key={command.label} + key={`${command.label}-${commandIndex}`}This ensures keys remain unique even if data structure changes, following React best practices.
apps/cms/src/components/editor/editor-page.tsx (3)
163-171: Browser Timeout types: avoid NodeJS.Timeout in client code.Use ReturnType to satisfy DOM types in Next.js.
- const debouncedUpdateFormValues = useMemo(() => { - let timeoutId: NodeJS.Timeout; + const debouncedUpdateFormValues = useMemo(() => { + let timeoutId: ReturnType<typeof setTimeout>;
223-231: Cleanup must mirror the idle fallback.Also clearTimeout when cancelIdleCallback is unavailable.
useEffect( () => () => { if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + if ("cancelIdleCallback" in window) { + (window as any).cancelIdleCallback(idleCallbackId.current); + } else { + clearTimeout(idleCallbackId.current); + } } }, [] );
235-265: Use browser-safe Timeout type.Same type fix for the unsaved-changes debounce.
- let timeoutId: NodeJS.Timeout; + let timeoutId: ReturnType<typeof setTimeout>;apps/cms/src/components/editor/text-buttons.tsx (2)
340-368: Propagate accessible names to mapped icon buttons.Set aria-label from item.tooltip.
- <Button + <Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={ activeStates[item.name as keyof typeof activeStates] } onClick={() => item.command(editor)} size="icon" type="button" variant="ghost" + aria-label={item.tooltip} >
376-404: Same a11y improvement for alignment buttons.Add aria-label derived from tooltip.
- <Button + <Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={ activeStates[item.name as keyof typeof activeStates] } onClick={() => item.command(editor)} size="icon" type="button" variant="ghost" + aria-label={item.tooltip} >apps/cms/src/components/editor/link-selector.tsx (4)
85-93: A11y polish for icon-only buttons; add type and labels; hide decorative SVGs.
- Add type="button" to the trigger button.
- Provide aria-labels for icon-only controls; add aria-pressed to the toggle.
- Mark SVG icons as aria-hidden.
Based on learnings<Button className="!rounded-sm gap-2 border-none data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={isLinkActive} size="icon" variant="ghost" + type="button" + aria-label="Set link" > - <Link className="size-4" /> + <Link className="size-4" aria-hidden="true" /> </Button> @@ <Button className="!rounded-sm text-destructive hover:bg-destructive/10 hover:text-destructive" onClick={() => { editor.chain().focus().unsetLink().run(); setInputValue(""); setIsOpen(false); }} size="icon" type="button" variant="ghost" + aria-label="Remove link" > - <Trash2 className="size-4" /> + <Trash2 className="size-4" aria-hidden="true" /> </Button> @@ <Button className="!rounded-sm text-green-600 hover:bg-green-500/10 hover:text-green-600" onClick={() => { const url = getUrlFromString(inputValue); if (url) { editor .chain() .focus() .setLink({ href: url, target: openInNewTab ? "_blank" : "_self", }) .run(); setInputValue(""); setIsOpen(false); } }} size="icon" type="button" variant="ghost" + aria-label="Apply link" > - <Check className="size-4" /> + <Check className="size-4" aria-hidden="true" /> </Button> @@ <Button className={cn( "!rounded-sm", openInNewTab && "bg-primary/20 text-primary" )} onClick={() => setOpenInNewTab(!openInNewTab)} size="icon" type="button" variant="ghost" + aria-label={openInNewTab ? "Open in new tab is on" : "Open in new tab is off"} + aria-pressed={openInNewTab} > - <Maximize2 className="size-4" /> + <Maximize2 className="size-4" aria-hidden="true" /> </Button> @@ <Button className="!rounded-sm" disabled={!inputValue || !getUrlFromString(inputValue)} onClick={() => { const url = getUrlFromString(inputValue); if (url) { - window.open(url, "_blank"); + window.open(url, "_blank"); } }} size="icon" type="button" variant="ghost" + aria-label="Open link in new tab" > - <ExternalLink className="size-4" /> + <ExternalLink className="size-4" aria-hidden="true" /> </Button>Also applies to: 140-152, 160-183, 196-206, 215-228
109-113: Improve input semantics and Enter handling.Use type="url", inputMode/autoComplete hints, and trim before parsing.
- if (e.key === "Enter") { + if (e.key === "Enter") { e.preventDefault(); - const url = getUrlFromString(inputValue); + const url = getUrlFromString(inputValue.trim()); if (url) { editor .chain() .focus() .setLink({ href: url, target: openInNewTab ? "_blank" : "_self", }) .run(); setInputValue(""); setIsOpen(false); } } @@ <input aria-label="Link URL" className="flex-1 bg-background px-2 py-1 text-sm outline-hidden" onChange={({ target }) => setInputValue(target.value)} placeholder="Paste or type link" ref={inputRef} - type="text" + type="url" + inputMode="url" + autoComplete="url" value={inputValue} />Also applies to: 127-135
141-146: Extend mark range for link set/unset and add rel attribute.Ensures operations affect the whole link when cursor is inside and sets rel to avoid tabnabbing on rendered anchors.
- editor.chain().focus().unsetLink().run(); + editor.chain().focus().extendMarkRange("link").unsetLink().run(); @@ - editor - .chain() - .focus() - .setLink({ - href: url, - target: openInNewTab ? "_blank" : "_self", - }) - .run(); + editor + .chain() + .focus() + .extendMarkRange("link") + .setLink({ + href: url, + target: openInNewTab ? "_blank" : "_self", + rel: openInNewTab ? "noopener noreferrer" : undefined, + }) + .run();Also applies to: 165-173
160-166: Disable “Set link” button when URL is invalid.Avoid no-op clicks; mirror the disabled logic used for “Open link.”
- <Button + <Button className="!rounded-sm text-green-600 hover:bg-green-500/10 hover:text-green-600" + disabled={!getUrlFromString(inputValue)} onClick={() => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/cms/src/components/editor/color-picker.tsx(1 hunks)apps/cms/src/components/editor/drag-handle/hooks/use-drag-data.tsx(1 hunks)apps/cms/src/components/editor/editor-page.tsx(6 hunks)apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx(1 hunks)apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts(1 hunks)apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx(1 hunks)apps/cms/src/components/editor/link-selector.tsx(3 hunks)apps/cms/src/components/editor/slash-command/menu-list.tsx(1 hunks)apps/cms/src/components/editor/text-buttons.tsx(1 hunks)apps/cms/src/lib/utils/getRenderContainer.ts(1 hunks)apps/cms/src/styles/editor.css(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/cms/src/lib/utils/getRenderContainer.ts
- apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts
- apps/cms/src/components/editor/drag-handle/hooks/use-drag-data.tsx
- apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx
- apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...Files:
apps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/text-buttons.tsxapps/cms/src/components/editor/color-picker.tsx**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...Files:
apps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/text-buttons.tsxapps/cms/src/components/editor/color-picker.tsx**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return typesFiles:
apps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/text-buttons.tsxapps/cms/src/components/editor/color-picker.tsx**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't destructure props inside JSX components in Solid projects
Files:
apps/cms/src/components/editor/slash-command/menu-list.tsxapps/cms/src/components/editor/link-selector.tsxapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/text-buttons.tsxapps/cms/src/components/editor/color-picker.tsx🧠 Learnings (2)
📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR PR: usemarble/marble#0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Ensure label elements have text content and are associated with an inputApplied to files:
apps/cms/src/components/editor/link-selector.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR PR: usemarble/marble#0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Ensure anchors have content accessible to screen readersApplied to files:
apps/cms/src/components/editor/link-selector.tsx🧬 Code graph analysis (4)
apps/cms/src/components/editor/slash-command/menu-list.tsx (4)
apps/cms/src/components/editor/slash-command/types.ts (1)
MenuListProps(20-24)apps/cms/src/components/editor/slash-command/surface.tsx (1)
Surface(11-29)apps/cms/src/components/shared/icons.tsx (1)
Icon(3-3)apps/cms/src/components/editor/slash-command/dropdown-button.tsx (1)
DropdownButton(5-36)apps/cms/src/components/editor/link-selector.tsx (2)
apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)packages/ui/src/lib/utils.ts (1)
cn(4-6)apps/cms/src/components/editor/editor-page.tsx (7)
apps/cms/src/components/editor/extensions.ts (1)
defaultExtensions(135-167)apps/cms/src/components/editor/drag-handle/index.tsx (1)
DragHandle(155-155)apps/cms/src/components/editor/bubble-menu.tsx (1)
BubbleMenu(88-88)apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx (1)
TableRowMenu(102-102)apps/cms/src/components/editor/extensions/table/menus/TableColumn/index.tsx (1)
TableColumnMenu(118-118)apps/cms/src/components/editor/extensions/multi-column/menus/index.ts (1)
ColumnsMenu(2-2)apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx (1)
ColumnsMenu(139-139)apps/cms/src/components/editor/text-buttons.tsx (5)
apps/cms/src/components/editor/utils/platform.ts (1)
getModifierKey(15-17)apps/cms/src/components/editor/keyboard-key.tsx (1)
KeyboardKey(10-29)apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)apps/cms/src/components/editor/content-type-picker.tsx (1)
ContentTypePicker(226-226)apps/cms/src/components/editor/color-picker.tsx (1)
ColorPicker(16-89)🪛 GitHub Actions: Code quality
apps/cms/src/components/editor/text-buttons.tsx
[error] 270-274: Prettier formatting check failed for text-buttons.tsx: arrow function formatting adjustments needed.
apps/cms/src/components/editor/color-picker.tsx
[error] 1-1: Prettier formatting check failed for color-picker.tsx. Formatter would print different content.
🔇 Additional comments (3)
apps/cms/src/styles/editor.css (2)
7-10: Slash menu width fix looks good.Responsive, intrinsic sizing prevents horizontal overflow and matches prior feedback.
74-92: Figure/caption styles: solid baseline.Block-level figure with full-width media and centered, muted figcaption reads well in both themes.
apps/cms/src/components/editor/editor-page.tsx (1)
187-221: Good: editor initialized from stable initial content (avoids feedback loops).Using a ref for initial content and one-way editor→form sync prevents re-inits and churn.
| <div className="flex items-center gap-2"> | ||
| <Input | ||
| className="h-8 font-mono text-xs" | ||
| onChange={handleHexInputChange} | ||
| placeholder="#000000" | ||
| value={hexInput} | ||
| /> | ||
| </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.
Add accessible name for the hex input.
Input lacks an associated label. Provide an aria-label to satisfy a11y guideline.
- <Input
+ <Input
className="h-8 font-mono text-xs"
onChange={handleHexInputChange}
placeholder="#000000"
value={hexInput}
+ aria-label="Hex color"
/>📝 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.
| <div className="flex items-center gap-2"> | |
| <Input | |
| className="h-8 font-mono text-xs" | |
| onChange={handleHexInputChange} | |
| placeholder="#000000" | |
| value={hexInput} | |
| /> | |
| </div> | |
| <div className="flex items-center gap-2"> | |
| <Input | |
| className="h-8 font-mono text-xs" | |
| onChange={handleHexInputChange} | |
| placeholder="#000000" | |
| value={hexInput} | |
| aria-label="Hex color" | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/color-picker.tsx around lines 56 to 63, the
hex input has no accessible name; add an accessible label by providing an
aria-label (or aria-labelledby) on the Input element (e.g., aria-label="Hex
color" or a more descriptive string) so screen readers can identify the control;
ensure the aria-label stays in sync with purpose and keep placeholder as
presentational only.
| {PRESET_COLORS.map((presetColor) => ( | ||
| <button | ||
| className="size-6 rounded border border-border transition-transform hover:scale-110" | ||
| key={presetColor} | ||
| onClick={() => handleColorChange(presetColor)} | ||
| style={{ backgroundColor: presetColor }} | ||
| title={presetColor} | ||
| type="button" | ||
| /> | ||
| ))} |
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.
Icon-only preset buttons need accessible labels.
Title isn’t a reliable accessible name. Add aria-label; keep type="button".
<button
className="size-6 rounded border border-border transition-transform hover:scale-110"
key={presetColor}
onClick={() => handleColorChange(presetColor)}
style={{ backgroundColor: presetColor }}
- title={presetColor}
+ title={presetColor}
+ aria-label={`Set color ${presetColor}`}
type="button"
/>📝 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.
| {PRESET_COLORS.map((presetColor) => ( | |
| <button | |
| className="size-6 rounded border border-border transition-transform hover:scale-110" | |
| key={presetColor} | |
| onClick={() => handleColorChange(presetColor)} | |
| style={{ backgroundColor: presetColor }} | |
| title={presetColor} | |
| type="button" | |
| /> | |
| ))} | |
| {PRESET_COLORS.map((presetColor) => ( | |
| <button | |
| className="size-6 rounded border border-border transition-transform hover:scale-110" | |
| key={presetColor} | |
| onClick={() => handleColorChange(presetColor)} | |
| style={{ backgroundColor: presetColor }} | |
| title={presetColor} | |
| aria-label={`Set color ${presetColor}`} | |
| type="button" | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/color-picker.tsx around lines 66 to 75, the
icon-only preset color buttons rely on title for labeling which is not a
reliable accessible name; add an explicit aria-label (for example "Select color
{presetColor}" or simply the color name) to each button, keep type="button" and
you may preserve the title if desired, so screen readers receive a proper
accessible name for each preset button.
| <Button | ||
| className="size-8 shrink-0" | ||
| onClick={onClear} | ||
| size="icon" | ||
| title="Reset color" | ||
| type="button" | ||
| variant="ghost" | ||
| > | ||
| <ArrowCounterClockwiseIcon className="size-4" /> | ||
| </Button> |
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.
Add aria-label to reset button and hide decorative SVG from AT.
Provide an explicit accessible name and mark the icon as decorative.
- <Button
+ <Button
className="size-8 shrink-0"
onClick={onClear}
size="icon"
- title="Reset color"
+ title="Reset color"
+ aria-label="Reset color"
type="button"
variant="ghost"
>
- <ArrowCounterClockwiseIcon className="size-4" />
+ <ArrowCounterClockwiseIcon className="size-4" aria-hidden="true" focusable="false" />
</Button>📝 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.
| <Button | |
| className="size-8 shrink-0" | |
| onClick={onClear} | |
| size="icon" | |
| title="Reset color" | |
| type="button" | |
| variant="ghost" | |
| > | |
| <ArrowCounterClockwiseIcon className="size-4" /> | |
| </Button> | |
| <Button | |
| className="size-8 shrink-0" | |
| onClick={onClear} | |
| size="icon" | |
| title="Reset color" | |
| aria-label="Reset color" | |
| type="button" | |
| variant="ghost" | |
| > | |
| <ArrowCounterClockwiseIcon className="size-4" aria-hidden="true" focusable="false" /> | |
| </Button> |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/color-picker.tsx around lines 76 to 85, the
reset Button lacks an accessible name and the SVG icon is exposed to assistive
tech; add an explicit aria-label (e.g., aria-label="Reset color") to the Button
and mark the ArrowCounterClockwiseIcon as decorative by adding
aria-hidden="true" (or role="presentation" and focusable="false") so screen
readers ignore the SVG.
| useImperativeHandle(ref, () => ({ | ||
| onKeyDown: ({ event }: { event: KeyboardEvent }) => { | ||
| if (event.key === "ArrowDown") { | ||
| if (!props.items.length) { | ||
| return false; | ||
| } | ||
|
|
||
| const currentGroup = props.items[selectedGroupIndex]; | ||
| if (!currentGroup) { | ||
| return false; | ||
| } | ||
|
|
||
| const commands = currentGroup.commands; | ||
| let newCommandIndex = selectedCommandIndex + 1; | ||
| let newGroupIndex = selectedGroupIndex; | ||
|
|
||
| if (newCommandIndex >= commands.length) { | ||
| newCommandIndex = 0; | ||
| newGroupIndex = (selectedGroupIndex + 1) % props.items.length; | ||
| } | ||
|
|
||
| setSelectedCommandIndex(newCommandIndex); | ||
| setSelectedGroupIndex(newGroupIndex); | ||
| return true; | ||
| } | ||
|
|
||
| if (event.key === "ArrowUp") { | ||
| if (!props.items.length) { | ||
| return false; | ||
| } | ||
|
|
||
| let newCommandIndex = selectedCommandIndex - 1; | ||
| let newGroupIndex = selectedGroupIndex; | ||
|
|
||
| if (newCommandIndex < 0) { | ||
| newGroupIndex = | ||
| selectedGroupIndex - 1 < 0 | ||
| ? props.items.length - 1 | ||
| : selectedGroupIndex - 1; | ||
| const newGroup = props.items[newGroupIndex]; | ||
| if (!newGroup) { | ||
| return false; | ||
| } | ||
| newCommandIndex = newGroup.commands.length - 1; | ||
| } | ||
|
|
||
| setSelectedCommandIndex(newCommandIndex); | ||
| setSelectedGroupIndex(newGroupIndex); | ||
| return true; | ||
| } | ||
|
|
||
| if (event.key === "Enter") { | ||
| if (!props.items.length) { | ||
| return false; | ||
| } | ||
| selectItem(selectedGroupIndex, selectedCommandIndex); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| }, | ||
| })); |
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 | 🟠 Major
Add explicit dependency array to useImperativeHandle.
useImperativeHandle is missing its dependency array, which means the exposed onKeyDown handler may close over stale values. Add all dependencies:
useImperativeHandle(ref, () => ({
onKeyDown: ({ event }: { event: KeyboardEvent }) => {
...
},
- }));
+ }), [selectedGroupIndex, selectedCommandIndex, props.items, selectItem]);Or, if you destructure items and command as suggested earlier:
+ }), [selectedGroupIndex, selectedCommandIndex, items, selectItem]);This ensures the handler always captures the latest state and props.
📝 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.
| useImperativeHandle(ref, () => ({ | |
| onKeyDown: ({ event }: { event: KeyboardEvent }) => { | |
| if (event.key === "ArrowDown") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| const currentGroup = props.items[selectedGroupIndex]; | |
| if (!currentGroup) { | |
| return false; | |
| } | |
| const commands = currentGroup.commands; | |
| let newCommandIndex = selectedCommandIndex + 1; | |
| let newGroupIndex = selectedGroupIndex; | |
| if (newCommandIndex >= commands.length) { | |
| newCommandIndex = 0; | |
| newGroupIndex = (selectedGroupIndex + 1) % props.items.length; | |
| } | |
| setSelectedCommandIndex(newCommandIndex); | |
| setSelectedGroupIndex(newGroupIndex); | |
| return true; | |
| } | |
| if (event.key === "ArrowUp") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| let newCommandIndex = selectedCommandIndex - 1; | |
| let newGroupIndex = selectedGroupIndex; | |
| if (newCommandIndex < 0) { | |
| newGroupIndex = | |
| selectedGroupIndex - 1 < 0 | |
| ? props.items.length - 1 | |
| : selectedGroupIndex - 1; | |
| const newGroup = props.items[newGroupIndex]; | |
| if (!newGroup) { | |
| return false; | |
| } | |
| newCommandIndex = newGroup.commands.length - 1; | |
| } | |
| setSelectedCommandIndex(newCommandIndex); | |
| setSelectedGroupIndex(newGroupIndex); | |
| return true; | |
| } | |
| if (event.key === "Enter") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| selectItem(selectedGroupIndex, selectedCommandIndex); | |
| return true; | |
| } | |
| return false; | |
| }, | |
| })); | |
| useImperativeHandle(ref, () => ({ | |
| onKeyDown: ({ event }: { event: KeyboardEvent }) => { | |
| if (event.key === "ArrowDown") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| const currentGroup = props.items[selectedGroupIndex]; | |
| if (!currentGroup) { | |
| return false; | |
| } | |
| const commands = currentGroup.commands; | |
| let newCommandIndex = selectedCommandIndex + 1; | |
| let newGroupIndex = selectedGroupIndex; | |
| if (newCommandIndex >= commands.length) { | |
| newCommandIndex = 0; | |
| newGroupIndex = (selectedGroupIndex + 1) % props.items.length; | |
| } | |
| setSelectedCommandIndex(newCommandIndex); | |
| setSelectedGroupIndex(newGroupIndex); | |
| return true; | |
| } | |
| if (event.key === "ArrowUp") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| let newCommandIndex = selectedCommandIndex - 1; | |
| let newGroupIndex = selectedGroupIndex; | |
| if (newCommandIndex < 0) { | |
| newGroupIndex = | |
| selectedGroupIndex - 1 < 0 | |
| ? props.items.length - 1 | |
| : selectedGroupIndex - 1; | |
| const newGroup = props.items[newGroupIndex]; | |
| if (!newGroup) { | |
| return false; | |
| } | |
| newCommandIndex = newGroup.commands.length - 1; | |
| } | |
| setSelectedCommandIndex(newCommandIndex); | |
| setSelectedGroupIndex(newGroupIndex); | |
| return true; | |
| } | |
| if (event.key === "Enter") { | |
| if (!props.items.length) { | |
| return false; | |
| } | |
| selectItem(selectedGroupIndex, selectedCommandIndex); | |
| return true; | |
| } | |
| return false; | |
| }, | |
| }), [selectedGroupIndex, selectedCommandIndex, props.items, selectItem]); |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/slash-command/menu-list.tsx around lines 47 to
108, the useImperativeHandle call is missing its dependency array so the
onKeyDown handler can close over stale state/props; add a third argument to
useImperativeHandle listing all reactive values used inside the handler
(props.items or items, selectedGroupIndex, selectedCommandIndex, selectItem,
setSelectedCommandIndex, setSelectedGroupIndex) so the imperative handle is
recreated when any of those change.
| {/* Basic formatting buttons */} | ||
| {BASIC_FORMATTING.map((item) => ( | ||
| <Tooltip delayDuration={400} key={item.name}> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" | ||
| data-active={activeStates[item.name as keyof typeof activeStates]} | ||
| onClick={() => item.command(editor)} | ||
| size="icon" | ||
| type="button" | ||
| variant="ghost" | ||
| > | ||
| <item.icon className="size-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span>{item.tooltip}</span> | ||
| {item.shortcut && ( | ||
| <span className="flex items-center gap-0.5"> | ||
| {renderShortcut(item.shortcut)} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| ))} |
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.
Icon-only buttons need accessible names.
Use aria-label for Buttons; Tooltip text isn’t an accessible name.
- <Button
+ <Button
className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary"
data-active={activeStates[item.name as keyof typeof activeStates]}
onClick={() => item.command(editor)}
size="icon"
type="button"
variant="ghost"
+ aria-label={item.tooltip}
>🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around lines 212 to 238, the
icon-only Buttons rely on visible Tooltips for meaning which aren't exposed as
accessible names; add an aria-label to each Button (preferably
aria-label={item.tooltip} or fallback to item.name) so screen readers receive a
proper accessible name, keep the Tooltip as visual helper, and ensure the
aria-label value is a concise descriptive string for each control.
| {/* Highlight Color */} | ||
| <Popover> | ||
| <Tooltip delayDuration={400}> | ||
| <PopoverTrigger asChild> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" | ||
| data-active={activeStates.highlight} | ||
| size="icon" | ||
| type="button" | ||
| variant="ghost" | ||
| > | ||
| <Highlighter className="size-4" /> | ||
| </Button> | ||
| </TooltipTrigger> |
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.
Add accessible names to highlight/text color triggers.
Provide aria-labels for icon-only buttons.
- <Button
+ <Button
className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary"
data-active={activeStates.highlight}
size="icon"
type="button"
variant="ghost"
+ aria-label="Highlight text"
>🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around lines 245 to 259, the
icon-only Popover trigger buttons lack accessible names; add descriptive
aria-label attributes to the icon-only Button elements (e.g.,
aria-label="Highlight color" for the highlighter trigger and aria-label="Text
color" for the text color trigger) so screen readers can identify the controls;
keep existing Tooltip/Popover behavior and ensure the aria-label strings are
concise and match visible tooltip text.
| <Button | ||
| className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" | ||
| data-active={activeStates.textColor} | ||
| size="icon" | ||
| type="button" | ||
| variant="ghost" | ||
| > | ||
| <Palette className="size-4" /> | ||
| </Button> | ||
| </TooltipTrigger> |
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.
Add accessible name to text color trigger.
Same as above.
- <Button
+ <Button
className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary"
data-active={activeStates.textColor}
size="icon"
type="button"
variant="ghost"
+ aria-label="Text color"
>📝 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.
| <Button | |
| className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" | |
| data-active={activeStates.textColor} | |
| size="icon" | |
| type="button" | |
| variant="ghost" | |
| > | |
| <Palette className="size-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <Button | |
| className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" | |
| data-active={activeStates.textColor} | |
| size="icon" | |
| type="button" | |
| variant="ghost" | |
| aria-label="Text color" | |
| > | |
| <Palette className="size-4" /> | |
| </Button> | |
| </TooltipTrigger> |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around lines 283 to 292, the
Palette button (text color trigger) lacks an accessible name; add an accessible
label and proper ARIA attributes by giving the Button an aria-label (e.g., "Text
color"), aria-haspopup="menu" if it opens a menu, and aria-expanded tied to
activeStates.textColor so screen readers know its purpose and state.
| <Button | ||
| className="!rounded-sm" | ||
| size="icon" | ||
| type="button" | ||
| variant="ghost" | ||
| > | ||
| <EllipsisVertical className="size-4" /> | ||
| </Button> |
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.
More options trigger needs an accessible name.
Add aria-label.
- <Button
+ <Button
className="!rounded-sm"
size="icon"
type="button"
variant="ghost"
+ aria-label="More options"
>🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around lines 321 to 328, the
"more options" Button lacks an accessible name; add an aria-label (e.g.
aria-label="More options") to the Button so assistive technologies can identify
it, and optionally include aria-haspopup="menu" if it opens a menu and manage
aria-expanded state when the menu is toggled.
| .ProseMirror .is-editor-empty:first-child::before { | ||
| content: attr(data-placeholder); | ||
| float: left; | ||
| color: hsl(var(--muted-foreground)); | ||
| color: hsl(from var(--muted-foreground) h s l / 0.5); | ||
| pointer-events: none; | ||
| height: 0; | ||
| font-size: 16px; | ||
| } | ||
| .ProseMirror .is-empty::before { | ||
| content: attr(data-placeholder); | ||
| float: left; | ||
| color: hsl(var(--muted-foreground)); | ||
| color: hsl(from var(--muted-foreground) h s l / 0.5); | ||
| pointer-events: none; | ||
| height: 0; | ||
| } |
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.
Invalid/fragile color syntax: replace relative HSL with standard var-based HSL.
"hsl(from var(--muted-foreground) h s l / 0.5)" will fail if --muted-foreground isn’t a valid color token and is less supported than standard var-based HSL. Use consistent syntax like elsewhere in this file.
.ProseMirror .is-editor-empty:first-child::before {
- color: hsl(from var(--muted-foreground) h s l / 0.5);
+ color: hsl(var(--muted-foreground) / 0.5);
}
.ProseMirror .is-empty::before {
- color: hsl(from var(--muted-foreground) h s l / 0.5);
+ color: hsl(var(--muted-foreground) / 0.5);
}📝 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.
| .ProseMirror .is-editor-empty:first-child::before { | |
| content: attr(data-placeholder); | |
| float: left; | |
| color: hsl(var(--muted-foreground)); | |
| color: hsl(from var(--muted-foreground) h s l / 0.5); | |
| pointer-events: none; | |
| height: 0; | |
| font-size: 16px; | |
| } | |
| .ProseMirror .is-empty::before { | |
| content: attr(data-placeholder); | |
| float: left; | |
| color: hsl(var(--muted-foreground)); | |
| color: hsl(from var(--muted-foreground) h s l / 0.5); | |
| pointer-events: none; | |
| height: 0; | |
| } | |
| .ProseMirror .is-editor-empty:first-child::before { | |
| content: attr(data-placeholder); | |
| float: left; | |
| color: hsl(var(--muted-foreground) / 0.5); | |
| pointer-events: none; | |
| height: 0; | |
| font-size: 16px; | |
| } | |
| .ProseMirror .is-empty::before { | |
| content: attr(data-placeholder); | |
| float: left; | |
| color: hsl(var(--muted-foreground) / 0.5); | |
| pointer-events: none; | |
| height: 0; | |
| } |
🤖 Prompt for AI Agents
In apps/cms/src/styles/editor.css around lines 43 to 57 the color uses the
fragile/unsupported syntax "hsl(from var(--muted-foreground) h s l / 0.5)";
replace it with the project's standard var-based HSL usage (e.g. if
--muted-foreground holds "h s l" use "hsl(var(--muted-foreground) / 0.5)" or if
it holds a full color token use "color: var(--muted-foreground)" or the exact
pattern used elsewhere in this file), and apply the same replacement for both
.is-editor-empty:first-child::before and .is-empty::before selectors so the
syntax is consistent and supported.
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: 2
♻️ Duplicate comments (5)
apps/cms/src/components/editor/text-buttons.tsx (5)
212-238: Add accessible names and pressed state to icon-only formatting buttons.Buttons need aria-label, and toggles should expose aria-pressed. As per coding guidelines.
<Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={activeStates[item.name as keyof typeof activeStates]} onClick={() => item.command(editor)} size="icon" type="button" variant="ghost" + aria-label={item.tooltip} + aria-pressed={Boolean( + activeStates[item.name as keyof typeof activeStates] + )} >#!/bin/bash # Check for icon-size Buttons missing aria-label in this file rg -nP -C1 'size="icon"' apps/cms/src/components/editor/text-buttons.tsx | rg -nP -C1 'Button(?![^>]*aria-label)'
245-259: Add accessible name to Highlight trigger.Provide aria-label for the icon-only button. As per coding guidelines.
<Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={activeStates.highlight} size="icon" type="button" variant="ghost" + aria-label="Highlight text" >
285-294: Add accessible name to Text Color trigger.Provide aria-label for the icon-only Palette button. As per coding guidelines.
<Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={activeStates.textColor} size="icon" type="button" variant="ghost" + aria-label="Text color" >
305-309: Focus before setColor to preserve selection.Call focus() before setColor; otherwise selection may be lost when the popover opens.
- onChange={(color) => editor.chain().setColor(color).run()} + onChange={(color) => + editor.chain().focus().setColor(color).run() + }
323-330: Add accessible name to “More options” trigger.Provide aria-label for the icon-only button. As per coding guidelines.
<Button className="!rounded-sm" size="icon" type="button" variant="ghost" + aria-label="More options" >
🧹 Nitpick comments (3)
apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts (1)
18-20: Consider removing premature HTMLElement casts.Lines 18-19 cast to
HTMLElementbefore verifying the node type, which is misleading sinceview.domAtPos(from).nodeandview.nodeDOM(from)returnNode(which could be Text, Comment, etc.). The code correctly checksnode.nodeType === Node.ELEMENT_NODEon line 32, but the earlier casts create confusing type expectations.Apply this diff to improve type accuracy:
- const domAtPos = view.domAtPos(from).node as HTMLElement; - const nodeDOM = view.nodeDOM(from) as HTMLElement; + const domAtPos = view.domAtPos(from).node; + const nodeDOM = view.nodeDOM(from); const node = nodeDOM || domAtPos;apps/cms/src/components/editor/text-buttons.tsx (2)
346-357: Expose toggle state via aria-pressed in popover buttons.Mirror data-active with aria-pressed for sub/superscript and alignment buttons to improve AT feedback.
<Button className="!rounded-sm data-[active=true]:bg-primary/20 data-[active=true]:text-primary" data-active={ activeStates[item.name as keyof typeof activeStates] } onClick={() => item.command(editor)} size="icon" type="button" variant="ghost" + aria-pressed={Boolean( + activeStates[item.name as keyof typeof activeStates] + )} >Also applies to: 381-392
20-35: Use Phosphor Icons consistently and decouple icon prop type.Guidelines: “Always use the Phosphor Icons package for icons” and “Suffix icon components with Icon.” Replace lucide-react icons with Phosphor equivalents and type icons generically to avoid coupling to a specific pack. As per coding guidelines.
-} from "lucide-react"; +} from "@phosphor-icons/react";-import { memo } from "react"; +import { memo } from "react"; +import type { ComponentType } from "react";export type SelectorItem = { name: string; - icon: typeof Bold; + icon: ComponentType<{ className?: string }>; command: (editor: Editor) => void; isActive: (editor: Editor) => boolean; tooltip: string; shortcut?: string[]; };Note: Update the imported icon names to the project’s Phosphor “...Icon” variants to maintain naming consistency.
Also applies to: 43-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/cms/src/components/editor/color-picker.tsx(1 hunks)apps/cms/src/components/editor/drag-handle/hooks/use-drag-data.tsx(1 hunks)apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts(1 hunks)apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx(1 hunks)apps/cms/src/components/editor/text-buttons.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/cms/src/components/editor/color-picker.tsx
- apps/cms/src/components/editor/drag-handle/hooks/use-drag-data.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...
Files:
apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.tsapps/cms/src/components/editor/text-buttons.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types
Files:
apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.tsapps/cms/src/components/editor/text-buttons.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...Files:
apps/cms/src/components/editor/text-buttons.tsx**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't destructure props inside JSX components in Solid projects
Files:
apps/cms/src/components/editor/text-buttons.tsx🧬 Code graph analysis (2)
apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts (2)
apps/cms/src/components/editor/extensions/table/Table.ts (1)
Table(3-6)apps/cms/src/components/editor/extensions/table/utils.ts (1)
isTableSelected(67-80)apps/cms/src/components/editor/text-buttons.tsx (5)
apps/cms/src/components/editor/utils/platform.ts (1)
getModifierKey(15-17)apps/cms/src/components/editor/keyboard-key.tsx (1)
KeyboardKey(10-29)apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)apps/cms/src/components/editor/content-type-picker.tsx (1)
ContentTypePicker(226-226)apps/cms/src/components/editor/color-picker.tsx (1)
ColorPicker(16-89)🔇 Additional comments (3)
apps/cms/src/components/editor/extensions/table/menus/TableColumn/utils.ts (2)
30-35: LGTM! Safe DOM traversal usingclosest.This implementation correctly addresses the infinite loop risk flagged in the previous review. Using
closest("td, th")is the safe approach for finding the owning table cell without manual iteration. The optional chaining properly handles cases where the method might not exist or the element is not found.
37-39: LGTM! Appropriate selector for grip column detection.The specific selector
"a.grip-column.selected"correctly targets the table column grip UI element. The boolean coercion is clean and readable.apps/cms/src/components/editor/text-buttons.tsx (1)
270-276: Good fix: focus before applying highlight color.The command now focuses the editor before setHighlight/unsetHighlight. LGTM.
| TooltipContent, | ||
| TooltipTrigger, | ||
| } from "@marble/ui/components/tooltip"; | ||
| import { cn } from "@marble/ui/lib/utils"; |
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.
Remove unused import.
cn is not used in this file.
-import { cn } from "@marble/ui/lib/utils";📝 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 { cn } from "@marble/ui/lib/utils"; |
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around line 15, the import
"cn" from "@marble/ui/lib/utils" is unused; remove that named import (delete the
import or remove "cn" from the import list), verify there are no remaining
references to cn in the file, and run the linter/formatter to ensure no
unused-import warnings remain.
| function TextButtonsComponent() { | ||
| const { editor } = useCurrentEditor(); | ||
| const portalContainer = useFloatingPortalContainer(); | ||
|
|
||
| // Track all active states reactively for proper re-rendering | ||
| const activeStates = useEditorState({ | ||
| editor: editor as Editor, | ||
| selector: (ctx) => ({ | ||
| bold: ctx.editor.isActive("bold"), | ||
| italic: ctx.editor.isActive("italic"), | ||
| underline: ctx.editor.isActive("underline"), | ||
| strike: ctx.editor.isActive("strike"), | ||
| code: ctx.editor.isActive("code"), | ||
| codeBlock: ctx.editor.isActive("codeBlock"), | ||
| highlight: ctx.editor.isActive("highlight"), | ||
| textColor: !!ctx.editor.getAttributes("textStyle").color, | ||
| subscript: ctx.editor.isActive("subscript"), | ||
| superscript: ctx.editor.isActive("superscript"), | ||
| alignLeft: ctx.editor.isActive({ textAlign: "left" }), | ||
| alignCenter: ctx.editor.isActive({ textAlign: "center" }), | ||
| alignRight: ctx.editor.isActive({ textAlign: "right" }), | ||
| justify: ctx.editor.isActive({ textAlign: "justify" }), | ||
| }), | ||
| }); | ||
|
|
||
| if (!editor) { | ||
| return null; | ||
| } | ||
|
|
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.
Fix Hooks ordering: don’t run useEditorState when editor is null; split into an inner component.
Call useEditorState only when editor exists. Move the hook into a child component rendered after the null-guard to avoid runtime errors and Hooks rule violations. As per coding guidelines.
function TextButtonsComponent() {
const { editor } = useCurrentEditor();
- const portalContainer = useFloatingPortalContainer();
-
- // Track all active states reactively for proper re-rendering
- const activeStates = useEditorState({
- editor: editor as Editor,
- selector: (ctx) => ({
- bold: ctx.editor.isActive("bold"),
- italic: ctx.editor.isActive("italic"),
- underline: ctx.editor.isActive("underline"),
- strike: ctx.editor.isActive("strike"),
- code: ctx.editor.isActive("code"),
- codeBlock: ctx.editor.isActive("codeBlock"),
- highlight: ctx.editor.isActive("highlight"),
- textColor: !!ctx.editor.getAttributes("textStyle").color,
- subscript: ctx.editor.isActive("subscript"),
- superscript: ctx.editor.isActive("superscript"),
- alignLeft: ctx.editor.isActive({ textAlign: "left" }),
- alignCenter: ctx.editor.isActive({ textAlign: "center" }),
- alignRight: ctx.editor.isActive({ textAlign: "right" }),
- justify: ctx.editor.isActive({ textAlign: "justify" }),
- }),
- });
-
- if (!editor) {
- return null;
- }
+ const portalContainer = useFloatingPortalContainer();
+ if (!editor) return null;
+ return (
+ <ToolbarInner editor={editor} portalContainer={portalContainer} />
+ );
- return (
- <div className="flex items-center gap-0.5">
- {/* existing toolbar markup */}
- </div>
- );
}
+
+function ToolbarInner({
+ editor,
+ portalContainer,
+}: {
+ editor: Editor;
+ portalContainer: HTMLElement | null;
+}) {
+ const activeStates = useEditorState({
+ editor,
+ selector: (ctx) => ({
+ bold: ctx.editor.isActive("bold"),
+ italic: ctx.editor.isActive("italic"),
+ underline: ctx.editor.isActive("underline"),
+ strike: ctx.editor.isActive("strike"),
+ code: ctx.editor.isActive("code"),
+ codeBlock: ctx.editor.isActive("codeBlock"),
+ highlight: ctx.editor.isActive("highlight"),
+ textColor: !!ctx.editor.getAttributes("textStyle").color,
+ subscript: ctx.editor.isActive("subscript"),
+ superscript: ctx.editor.isActive("superscript"),
+ alignLeft: ctx.editor.isActive({ textAlign: "left" }),
+ alignCenter: ctx.editor.isActive({ textAlign: "center" }),
+ alignRight: ctx.editor.isActive({ textAlign: "right" }),
+ justify: ctx.editor.isActive({ textAlign: "justify" }),
+ }),
+ });
+ return (
+ <div className="flex items-center gap-0.5">
+ {/* move the existing toolbar JSX here unchanged */}
+ </div>
+ );
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/text-buttons.tsx around lines 173-201, the
useEditorState hook is being called even when editor may be null which violates
Hooks ordering and causes runtime errors; fix by extracting the code that uses
useEditorState into a child component that is only rendered after the
null-guard. Concretely: keep lightweight hooks that are safe at the top (e.g.,
useFloatingPortalContainer), add an inner component (e.g., TextButtonsInner)
that accepts the editor (and portalContainer if needed) as props, move the
useEditorState call and all editor-dependent logic into that inner component,
and change the outer TextButtonsComponent to return null if !editor and
otherwise render the inner component with the editor prop; ensure proper typing
for Editor prop and no hooks run before the null check.
…tiptap-v3-migration
…tiptap-v3-migration
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: 1
♻️ Duplicate comments (9)
apps/cms/package.json (1)
45-72: Version mismatches and redundant umbrella package persist from previous review—please address before merging.The previous CodeRabbit review comprehensively flagged unresolved issues in this section:
Peer version mismatch (lines 51, 54):
@tiptap/extension-document@^3.8.0and@tiptap/extension-file-handler@^3.8.0declare peer dependencies on@tiptap/core@^3.8.0and@tiptap/pm@^3.8.0, but line 45 pins@tiptap/core@^3.7.2. This causes resolution conflicts in the lockfile.Redundant umbrella package (line 67):
@tiptap/extensions@^3.7.1duplicates your individual extension declarations (lines 46–66) and complicates dependency resolution. Remove it.Version outlier (line 68):
@tiptap/markdown@^3.9.0is ahead of the 3.7.x baseline; align to^3.7.2for consistency.Recommended fix:
- Upgrade
@tiptap/core,@tiptap/pm, and all@tiptap/extension-*packages to a uniform^3.8.0(or downgrade document/file-handler to^3.7.2), ensuring peer ranges align.- Remove
@tiptap/extensionsentirely.- Align
@tiptap/markdownto^3.7.2.- Regenerate lockfile to verify no peer warnings or mismatches remain.
apps/cms/src/components/editor/extensions.ts (4)
72-78: YouTube extension options misplaced under HTMLAttributes.
controlsandnocookieare top-level extension options, not HTML attributes. Placing them underHTMLAttributesmeans they won't take effect.const youtube = Youtube.configure({ + nocookie: true, + controls: false, HTMLAttributes: { class: cx("w-full aspect-video"), - controls: false, - nocookie: true, }, });
120-123: Dropcursor registered three times.Dropcursor is configured in StarterKit (lines 120-123) and also added explicitly to
defaultExtensionstwice (lines 158 and 179), causing duplicate plugin registration.Keep the StarterKit configuration and remove both explicit entries:
const starterKit = StarterKit.configure({ // ... dropcursor: { color: "#DBEAFE", width: 4, }, gapcursor: false, codeBlock: false, document: false, }); export const defaultExtensions: Extension[] = [ // ... - Dropcursor as unknown as Extension, // ... (line 158) // ... - Dropcursor as unknown as Extension, // ... (line 179) ];Remove the unused import if no longer needed.
Also applies to: 158-158, 179-179
8-8: Incorrect import path for TaskList/TaskItem in TipTap v3.In TipTap v3,
TaskListandTaskItemare separate packages, not part of@tiptap/extension-list.-import { TaskItem, TaskList } from "@tiptap/extension-list"; +import TaskList from "@tiptap/extension-task-list"; +import TaskItem from "@tiptap/extension-task-item";Verify that
@tiptap/extension-task-listand@tiptap/extension-task-itemare installed in package.json.
96-98: Invalid StarterKit option: link.StarterKit doesn't include the Link extension, so this configuration is ignored.
Remove the invalid configuration:
const starterKit = StarterKit.configure({ - link: { - openOnClick: false, - }, bulletList: {If you need custom Link behavior, import and configure it separately:
import Link from "@tiptap/extension-link"; const link = Link.configure({ openOnClick: false, }); // Add to defaultExtensions: // link as unknown as Extension,apps/cms/src/components/editor/extensions/figure/figure-view.tsx (1)
268-281: Add width/height and async loading attributes to img elements.Both img elements (lines 268-272 and 276-280) lack
width,height,loading="lazy", anddecoding="async"attributes. The component already loads natural dimensions (lines 68-79) but doesn't apply them.Apply the fix from the previous review:
import { useCallback, useEffect, useRef, useState } from "react"; + import { useId } from "react";Add state and handler:
const [imageNaturalHeight, setImageNaturalHeight] = useState<number | null>( null ); + const [imgSize, setImgSize] = useState<{ width: number; height: number } | null>(null); + const uid = useId();+ const handleImageLoad = useCallback( + (e: React.SyntheticEvent<HTMLImageElement>) => { + const img = e.currentTarget; + setImgSize({ width: img.naturalWidth, height: img.naturalHeight }); + }, + [] + );Update both img elements:
<img alt={altValue} - className="h-auto w-full rounded-md border border-muted" + className="h-auto w-full rounded-md border border-muted" src={src} + width={imgSize?.width ?? 1} + height={imgSize?.height ?? 1} + loading="lazy" + decoding="async" + onLoad={handleImageLoad} />apps/cms/src/components/editor/extensions/figure/index.ts (1)
130-145: renderHTML applies attributes to wrong elements.
HTMLAttributes(containingsrc,alt,caption, etc.) are merged onto the<figure>element (line 137), but the nested<img>(line 138) receives no attributes and<figcaption>(line 139) renders empty. This breaks HTML serialization and SSR.Apply the fix from the previous review:
renderHTML({ HTMLAttributes }) { - const { href, ...attrs } = HTMLAttributes; + const { src, alt, caption, href, ...figureAttrs } = HTMLAttributes ?? {}; + const imgAttrs: Record<string, string> = {}; + if (src) imgAttrs.src = src; + if (typeof alt === "string") imgAttrs.alt = alt; // If href exists, wrap img in anchor tag if (href) { return [ "figure", - mergeAttributes(attrs), + mergeAttributes(figureAttrs), - ["a", { href }, ["img"]], + ["a", { href }, ["img", imgAttrs]], - ["figcaption"], + ["figcaption", {}, caption ?? ""], ]; } // Otherwise, render img directly - return ["figure", mergeAttributes(attrs), ["img"], ["figcaption"]]; + return [ + "figure", + mergeAttributes(figureAttrs), + ["img", imgAttrs], + ["figcaption", {}, caption ?? ""], + ]; }apps/cms/src/components/editor/editor-page.tsx (2)
44-52: Hooks misuse: useMemo inside a non-hook function.
useMemomust be called at component top-level or inside a custom hook. Here it's insidegetToggleSidebarShortcut().Apply this diff to remove the hook and compute synchronously:
const getToggleSidebarShortcut = () => { - const isMac = useMemo( - () => - typeof navigator !== "undefined" && - navigator.platform.toUpperCase().indexOf("MAC") >= 0, - [] - ); + const isMac = + typeof navigator !== "undefined" && + /mac/i.test(navigator.platform ?? ""); return isMac ? "⌘K" : "Ctrl+K"; };
208-218: requestIdleCallback fallback for Safari/older browsers.
requestIdleCallbackandcancelIdleCallbackare not universally available (missing in Safari). Add a fallback tosetTimeout.+ // Polyfill for requestIdleCallback + const scheduleIdleCallback = + typeof requestIdleCallback === "function" + ? (cb: IdleRequestCallback, opts?: IdleRequestOptions) => + requestIdleCallback(cb, opts) + : (cb: () => void, opts?: { timeout?: number }) => + window.setTimeout(cb, opts?.timeout ?? 0) as unknown as number; + + const cancelScheduledCallback = + typeof cancelIdleCallback === "function" + ? cancelIdleCallback + : clearTimeout; + const editor = useEditor({ // ... onUpdate: ({ editor }) => { contentChangedRef.current = true; editorRef.current = editor; if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + cancelScheduledCallback(idleCallbackId.current); } - idleCallbackId.current = requestIdleCallback( + idleCallbackId.current = scheduleIdleCallback( () => { if (contentChangedRef.current && editorRef.current) { const html = editorRef.current.getHTML(); const json = editorRef.current.getJSON(); onUpdateRef.current?.(html, json); contentChangedRef.current = false; } }, { timeout: 100 } ); }, }); useEffect( () => () => { if (idleCallbackId.current !== null) { - cancelIdleCallback(idleCallbackId.current); + cancelScheduledCallback(idleCallbackId.current); } }, [] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/cms/package.json(5 hunks)apps/cms/src/components/editor/editor-page.tsx(7 hunks)apps/cms/src/components/editor/editor-sidebar.tsx(4 hunks)apps/cms/src/components/editor/extensions.ts(3 hunks)apps/cms/src/components/editor/extensions/figure/figure-view.tsx(1 hunks)apps/cms/src/components/editor/extensions/figure/index.ts(1 hunks)apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx(1 hunks)apps/cms/src/components/editor/extensions/markdown-file-drop/index.ts(1 hunks)apps/cms/src/components/editor/extensions/markdown-paste/index.ts(1 hunks)apps/cms/src/components/editor/extensions/markdown-paste/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cms/src/components/editor/extensions/image-upload/image-uploader.tsx
- apps/cms/src/components/editor/editor-sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...
Files:
apps/cms/src/components/editor/extensions/markdown-paste/index.tsapps/cms/src/components/editor/extensions/markdown-file-drop/index.tsapps/cms/src/components/editor/extensions/markdown-paste/utils.tsapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/extensions.tsapps/cms/src/components/editor/extensions/figure/index.tsapps/cms/src/components/editor/extensions/figure/figure-view.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types
Files:
apps/cms/src/components/editor/extensions/markdown-paste/index.tsapps/cms/src/components/editor/extensions/markdown-file-drop/index.tsapps/cms/src/components/editor/extensions/markdown-paste/utils.tsapps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/extensions.tsapps/cms/src/components/editor/extensions/figure/index.tsapps/cms/src/components/editor/extensions/figure/figure-view.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...Files:
apps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/extensions/figure/figure-view.tsx**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't destructure props inside JSX components in Solid projects
Files:
apps/cms/src/components/editor/editor-page.tsxapps/cms/src/components/editor/extensions/figure/figure-view.tsx🧠 Learnings (23)
📓 Common learnings
Learnt from: brandonmcconnell Repo: usemarble/marble PR: 113 File: apps/web/src/components/BlogHeader.astro:69-0 Timestamp: 2025-08-20T06:19:19.253Z Learning: Focus review comments on the specific scope of Tailwind CSS upgrade PRs. Pre-existing accessibility, HTML validity, or design issues should be noted for separate PRs rather than being flagged in upgrade-focused PRs, even if they're encountered during the review.Learnt from: brandonmcconnell Repo: usemarble/marble PR: 113 File: apps/cms/src/components/editor/editor-page.tsx:0-0 Timestamp: 2025-08-20T06:13:45.705Z Learning: When reviewing Tailwind CSS upgrade PRs, focus on upgrade-related issues rather than flagging pre-existing accessibility or design concerns that should be addressed in separate PRs focused on those specific improvements.📚 Learning: 2025-10-28T00:35:50.304Z
Learnt from: mezotv Repo: usemarble/marble PR: 235 File: apps/cms/src/components/editor/editor-sidebar.tsx:76-77 Timestamp: 2025-10-28T00:35:50.304Z Learning: "use no memo" is a valid React Compiler directive that opts out a function or component from React Compiler optimizations. It should be placed as the first statement in a function body or at the top of a file. It's intended as a temporary escape hatch for debugging or incompatible code.Applied to files:
apps/cms/src/components/editor/editor-page.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Call React hooks only at the top level of component functionsApplied to files:
apps/cms/src/components/editor/editor-page.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't use dangerous JSX propsApplied to files:
apps/cms/src/components/editor/editor-page.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx} : Don't use the TypeScript directive ts-ignoreApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{ts,tsx} : Don't merge interfaces and classes unsafelyApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{ts,tsx} : No implicit any in variable declarationsApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx} : Use array type consistently (either T[] or Array<T>)Applied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{ts,tsx} : Use consistent accessibility modifiers on class membersApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{ts,tsx} : Don't misuse non-null assertion operatorApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx,js,jsx} : Don't use undeclared variables in the documentApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx} : Don't use user-defined types (type predicates abuse)Applied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx,js,jsx} : Avoid functions exceeding allowed Cognitive ComplexityApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{ts,tsx} : Don't use any or unknown as type constraintsApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Provide meaningful alt text for elements requiring itApplied to files:
apps/cms/src/components/editor/extensions/figure/index.tsapps/cms/src/components/editor/extensions/figure/figure-view.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't include "image", "picture", or "photo" in img alt textApplied to files:
apps/cms/src/components/editor/extensions/figure/index.tsapps/cms/src/components/editor/extensions/figure/figure-view.tsx📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Always include title attribute for iframe elementsApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Include caption tracks for audio and video elementsApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Prefer semantic elements over role attributes in JSXApplied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Use <>...</> instead of <Fragment>...</Fragment>Applied to files:
apps/cms/src/components/editor/extensions/figure/index.ts📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{jsx,tsx} : In Next.js, don't use <img> elementsApplied to files:
apps/cms/src/components/editor/extensions/figure/figure-view.tsx📚 Learning: 2025-08-20T06:18:34.131Z
Learnt from: brandonmcconnell Repo: usemarble/marble PR: 113 File: apps/cms/src/components/nav/page-header.tsx:0-0 Timestamp: 2025-08-20T06:18:34.131Z Learning: When verifying Tailwind CSS syntax changes during upgrades, especially in large PRs, search more comprehensively across the entire codebase including packages/ directories, not just immediate files, as shared UI components may define group contexts and data attributes used across multiple applications.Applied to files:
apps/cms/package.json📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Ensure all dependencies are listed in React hooksApplied to files:
apps/cms/package.json🧬 Code graph analysis (6)
apps/cms/src/components/editor/extensions/markdown-paste/index.ts (1)
apps/cms/src/components/editor/extensions/markdown-paste/utils.ts (2)
looksLikeMarkdown(6-33)transformContent(148-161)apps/cms/src/components/editor/extensions/markdown-file-drop/index.ts (1)
apps/cms/src/components/editor/extensions/markdown-paste/utils.ts (1)
transformContent(148-161)apps/cms/src/components/editor/editor-page.tsx (7)
apps/cms/src/components/editor/extensions.ts (1)
defaultExtensions(145-180)apps/cms/src/components/editor/drag-handle/index.tsx (1)
DragHandle(155-155)apps/cms/src/components/editor/bubble-menu.tsx (1)
BubbleMenu(88-88)apps/cms/src/components/editor/extensions/table/menus/TableRow/index.tsx (1)
TableRowMenu(102-102)apps/cms/src/components/editor/extensions/table/menus/TableColumn/index.tsx (1)
TableColumnMenu(118-118)apps/cms/src/components/editor/extensions/multi-column/menus/index.ts (1)
ColumnsMenu(2-2)apps/cms/src/components/editor/extensions/multi-column/menus/ColumnsMenu.tsx (1)
ColumnsMenu(139-139)apps/cms/src/components/editor/extensions.ts (13)
apps/cms/src/components/editor/extensions/document/Document.ts (1)
Document(3-5)apps/cms/src/components/editor/extensions/markdown-paste/index.ts (1)
MarkdownPaste(5-55)apps/cms/src/components/editor/extensions/figure/index.ts (1)
Figure(33-174)apps/cms/src/components/editor/extensions/image-upload/index.ts (1)
ImageUpload(18-93)apps/cms/src/components/editor/extensions/markdown-file-drop/index.ts (1)
MarkdownFileDrop(6-116)apps/cms/src/components/editor/extensions/youtube-upload/index.ts (1)
YouTubeUpload(15-48)apps/cms/src/components/editor/extensions/table/Table.ts (1)
Table(3-6)apps/cms/src/components/editor/extensions/table/TableRow.ts (1)
TableRow(3-6)apps/cms/src/components/editor/extensions/table/TableCell.ts (1)
TableCell(11-134)apps/cms/src/components/editor/extensions/table/TableHeader.ts (1)
TableHeader(7-97)apps/cms/src/components/editor/extensions/multi-column/Columns.ts (1)
Columns(22-71)apps/cms/src/components/editor/extensions/multi-column/Column.ts (1)
Column(3-35)apps/cms/src/components/editor/slash-command/index.ts (1)
SlashCommand(15-260)apps/cms/src/components/editor/extensions/figure/index.ts (1)
apps/cms/src/components/editor/extensions/figure/figure-view.tsx (1)
FigureView(16-440)apps/cms/src/components/editor/extensions/figure/figure-view.tsx (1)
apps/cms/src/components/editor/floating-portal-context.tsx (1)
useFloatingPortalContainer(27-29)🔇 Additional comments (10)
apps/cms/package.json (1)
19-19: New dependencies for Tiptap v3 editor features are appropriate.The additions align with the migration scope:
@floating-ui/dom@^1.6.11(line 19): Floating UI for bubble menu positioning.@radix-ui/react-popover@^1.1.15(line 32): Radix popover primitive for menu overlays.react-colorful@^5.6.1(line 95): Color picker for rich-text formatting.yjs@^13.6.27&y-protocols@^1.0.6(lines 109–110): Collaboration backend.These packages are scoped to editor features and support the Tiptap v3 architecture. Versions are stable and mutually compatible.
Also applies to: 32-32, 95-95, 109-110
apps/cms/src/components/editor/extensions/markdown-paste/index.ts (1)
5-55: LGTM! Clean markdown paste handler.The extension correctly detects markdown-like content, parses it via the Markdown extension, transforms images to figures, and handles errors gracefully by falling back to default paste behavior.
apps/cms/src/components/editor/extensions/markdown-file-drop/index.ts (1)
40-63: Handler returns before async file processing completes.The function returns
trueat line 63 immediately after starting async file reads, but the actual parsing and insertion happens later in thereader.onloadcallback (lines 42-57). If file reading or parsing fails, the handler has already claimed success.Consider whether this early return is intentional (to prevent default browser behavior) or if you should await the file processing and only return
trueif insertion succeeds.The same pattern occurs in
handlePaste(lines 87-110).apps/cms/src/components/editor/extensions/markdown-paste/utils.ts (3)
6-33: LGTM! Robust markdown detection.The heuristic checks multiple markdown patterns and requires at least 2 matches (or a single heading) to avoid false positives on plain text.
48-111: LGTM! Well-structured transformation logic.The function correctly handles linked images, respects inline contexts where figures aren't valid, and recursively processes nested content while preserving parent context.
148-161: LGTM! Clean transformation pipeline.The two-phase approach (transform images → lift figures) ensures correct document structure for both single nodes and arrays.
apps/cms/src/components/editor/editor-page.tsx (4)
174-181: LGTM: Stable callback pattern.Using a ref to maintain a stable
onUpdatecallback is a good pattern to prevent unnecessary editor re-initialization.
234-266: LGTM: Efficient unsaved changes tracking.The debounced change detection with simple field comparison first is a good performance optimization. Cleanup is properly handled.
294-301: LGTM: Proper memoization.Memoizing the editor context value and drag handle with correct dependencies prevents unnecessary re-renders and plugin lifecycle churn.
100-101: Verify: Hard navigation instead of client-side routing.Using
window.location.hrefforces a full page reload instead of client-side navigation. The commentedrouter.pushsuggests this was intentional, but this breaks Next.js's SPA behavior and loses client-side state.If you're experiencing issues with
router.push, consider:
- Using
router.replace()instead ofrouter.push()- Awaiting the invalidation before navigating
- Using
router.refresh()after navigation if neededonSuccess: (data) => { toast.success("Post created"); - window.location.href = `/${params.workspace}/editor/p/${data.id}`; - // router.push(`/${params.workspace}/editor/p/${data.id}`); if (workspaceId) { queryClient.invalidateQueries({ queryKey: QUERY_KEYS.POSTS(workspaceId), }); } + router.push(`/${params.workspace}/editor/p/${data.id}`); setHasUnsavedChanges(false); },If there's a specific reason for the hard navigation (e.g., editor state reset), please document it with a comment.
| const debouncedUpdateFormValues = useMemo(() => { | ||
| let timeoutId: NodeJS.Timeout; | ||
| return (html: string, json: JSONContent) => { | ||
| clearTimeout(timeoutId); | ||
| timeoutId = setTimeout(() => { | ||
| updateFormValues(html, json); | ||
| }, 150); | ||
| }; | ||
| }, [updateFormValues]); |
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.
Manual debounce lacks cleanup mechanism.
The manual debounce implementation captures timeoutId in a closure, but when updateFormValues changes and a new debounced function is created, any pending timeout from the old closure isn't cleared. This could cause stale updates.
Consider using a proper debounce utility:
- const debouncedUpdateFormValues = useMemo(() => {
- let timeoutId: NodeJS.Timeout;
- return (html: string, json: JSONContent) => {
- clearTimeout(timeoutId);
- timeoutId = setTimeout(() => {
- updateFormValues(html, json);
- }, 150);
- };
- }, [updateFormValues]);
+ const debouncedUpdateFormValues = useMemo(() => {
+ const debounce = <T extends (...args: any[]) => void>(
+ fn: T,
+ delay: number
+ ) => {
+ let timeoutId: NodeJS.Timeout | undefined;
+ const debounced = (...args: Parameters<T>) => {
+ clearTimeout(timeoutId);
+ timeoutId = setTimeout(() => fn(...args), delay);
+ };
+ debounced.cancel = () => clearTimeout(timeoutId);
+ return debounced;
+ };
+ return debounce(updateFormValues, 150);
+ }, [updateFormValues]);
+
+ // Cleanup on unmount or when debounced function changes
+ useEffect(() => {
+ return () => debouncedUpdateFormValues.cancel?.();
+ }, [debouncedUpdateFormValues]);Alternatively, use a dedicated debounce library for production-grade handling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/cms/src/components/editor/editor-page.tsx around lines 164 to 172, the
manual debounce captures timeoutId in the closure and does not clear pending
timeouts when updateFormValues changes, which can cause stale updates; fix by
moving the timeoutId to a ref (e.g., useRef<number | NodeJS.Timeout | null>) or
use a proper debounce utility (lodash.debounce) and ensure you cancel/clear any
pending timeout/debounced calls on unmount or when dependencies change (call
clearTimeout or debounced.cancel in a useEffect cleanup) so no stale timers
remain.
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: 0
♻️ Duplicate comments (1)
apps/cms/src/components/editor/extensions/table/TableCell.ts (1)
40-69: Col/row span & width attrs still aren’t round-trippedThis is the same issue we flagged earlier:
colspan,rowspan,colwidth, andstylenever render back to the DOM, andcolwidthis still parsed from a nonstandard attribute. ProseMirror/Tiptap persist column widths ontd[data-colwidth](one entry per spanned column) and expect those attributes to survive serialization; skipping them drops every resize after a load/save cycle.(discuss.prosemirror.net) The missingrenderHTMLfor the span/style attrs means any non-default spans or inline styling also vanish on export, causing real data loss.Apply a render path and switch to
data-colwidthso tables remain intact:colspan: { default: 1, parseHTML: (element) => { const colspan = element.getAttribute("colspan"); const value = colspan ? Number.parseInt(colspan, 10) : 1; return value; }, + renderHTML: (attributes) => { + const colspan = attributes.colspan ?? 1; + return colspan !== 1 ? { colspan: String(colspan) } : {}; + }, }, rowspan: { default: 1, parseHTML: (element) => { const rowspan = element.getAttribute("rowspan"); const value = rowspan ? Number.parseInt(rowspan, 10) : 1; return value; }, + renderHTML: (attributes) => { + const rowspan = attributes.rowspan ?? 1; + return rowspan !== 1 ? { rowspan: String(rowspan) } : {}; + }, }, colwidth: { default: null, parseHTML: (element) => { - const colwidth = element.getAttribute("colwidth"); - const value = colwidth ? [Number.parseInt(colwidth, 10)] : null; - - return value; + const raw = + element.getAttribute("data-colwidth") ?? + element.getAttribute("colwidth"); + + if (!raw) { + return null; + } + + const widths = String(raw) + .split(",") + .map((entry) => Number.parseInt(entry, 10)) + .filter((entry) => Number.isFinite(entry)); + + return widths.length ? widths : null; }, + renderHTML: (attributes) => { + if ( + Array.isArray(attributes.colwidth) && + attributes.colwidth.length > 0 + ) { + return { "data-colwidth": attributes.colwidth.join(",") }; + } + + return {}; + }, }, style: { default: null, + parseHTML: (element) => element.getAttribute("style"), + renderHTML: (attributes) => + attributes.style ? { style: String(attributes.style) } : {}, },
🧹 Nitpick comments (5)
apps/web/src/components/icons/Logo.astro (1)
2-5: Verify if this file should be included in the PR.This file contains only whitespace changes (removed indentation) and appears unrelated to the Tiptap v3 editor migration scope. The Logo component is in
apps/web/src/components/icons/and has no connection to the CMS editor changes described in the PR objectives.Consider excluding this file from the PR if these formatting changes were unintentional (e.g., from auto-formatting tools running across the codebase) to keep the PR focused on its stated migration objectives.
apps/web/src/layouts/BlogLayout.astro (1)
2-5: Inconsistent component naming pattern.Two imports are aliased with a "Component" suffix (
FooterComponent,HeadComponent) while the others (BlogHeader,ScrollToTop) retain their original names. This creates an inconsistent naming convention within the same file.Consider one of the following:
- Remove the aliases and use the original component names directly
- Apply the "Component" suffix consistently to all imported components
- Use a different naming convention that provides clearer value
Additionally, these changes appear unrelated to the TipTap v3 migration scope. If this is part of a broader naming convention update, consider addressing it in a separate, focused PR for clarity.
apps/cms/src/components/editor/ai/readability-suggestions.tsx (2)
68-106: Consider rendering non-interactive elements for items without textReference.When
suggestion.textReferenceis absent, the button element is non-functional but remains focusable and in the tab order. Consider conditionally rendering a<div>or<span>instead of<button>for these non-interactive suggestions to improve keyboard navigation efficiency.Example refactor:
{suggestions.map((suggestion, index) => ( - <button - className={cn( - "text-left text-muted-foreground text-sm leading-relaxed transition-colors", - suggestion.textReference - ? "cursor-pointer hover:text-foreground" - : "" - )} - key={`${suggestion.text}-${index}`} - onClick={() => { - if (suggestion.textReference && editor) { - highlightTextInEditor(editor, suggestion.textReference); - } - }} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - if (suggestion.textReference && editor) { - highlightTextInEditor(editor, suggestion.textReference); - } - } - }} - type="button" - > + suggestion.textReference ? ( + <button + className="text-left text-muted-foreground text-sm leading-relaxed transition-colors cursor-pointer hover:text-foreground" + key={`${suggestion.text}-${index}`} + onClick={() => { + if (editor) { + highlightTextInEditor(editor, suggestion.textReference); + } + }} + type="button" + > + {/* content */} + </button> + ) : ( + <div + className="text-left text-muted-foreground text-sm leading-relaxed" + key={`${suggestion.text}-${index}`} + > + {/* content */} + </div> + )
75-75: Consider a more stable key prop.The key combines
suggestion.textwithindex. If suggestion text can be duplicated, this could cause React reconciliation issues. Consider using a unique identifier if available, or rely solely on index if the suggestions array is stable between renders.apps/web/src/layouts/Layout.astro (1)
2-5: Naming inconsistency with ScrollToTop.The
Footer,Head, andHeaderimports were renamed with aComponentsuffix, butScrollToTopwas not changed toScrollToTopComponent. While this might be intentional (sinceScrollToTopdoesn't conflict with any HTML element name), it creates an inconsistent naming pattern.Consider either:
- Renaming
ScrollToToptoScrollToTopComponentfor consistency- Documenting why the naming convention differs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
apps/cms/src/components/editor/ai/readability-suggestions.tsx(2 hunks)apps/cms/src/components/editor/extensions/image-upload/hooks.ts(1 hunks)apps/cms/src/components/editor/extensions/markdown-paste/utils.ts(1 hunks)apps/cms/src/components/editor/extensions/table/TableCell.ts(1 hunks)apps/cms/src/components/editor/extensions/table/TableHeader.ts(1 hunks)apps/cms/src/components/editor/slash-command/index.ts(1 hunks)apps/web/src/components/BlogHeader.astro(1 hunks)apps/web/src/components/CategoryCard.astro(1 hunks)apps/web/src/components/CategoryFilter.astro(1 hunks)apps/web/src/components/ChangelogCard.astro(1 hunks)apps/web/src/components/Container.astro(1 hunks)apps/web/src/components/Footer.astro(1 hunks)apps/web/src/components/Head.astro(2 hunks)apps/web/src/components/Header.astro(1 hunks)apps/web/src/components/PostCard.astro(1 hunks)apps/web/src/components/PricingCard.astro(1 hunks)apps/web/src/components/Prose.astro(1 hunks)apps/web/src/components/ScrollToTop.astro(1 hunks)apps/web/src/components/Welcome.astro(1 hunks)apps/web/src/components/icons/Discord.astro(1 hunks)apps/web/src/components/icons/Github.astro(1 hunks)apps/web/src/components/icons/Logo.astro(1 hunks)apps/web/src/components/icons/WordMark.astro(1 hunks)apps/web/src/components/icons/X.astro(1 hunks)apps/web/src/components/icons/brand/Bounty.astro(1 hunks)apps/web/src/components/icons/brand/Databuddy.astro(1 hunks)apps/web/src/components/icons/brand/Helix.astro(1 hunks)apps/web/src/components/icons/brand/Ia.astro(1 hunks)apps/web/src/components/icons/brand/Mantlz.astro(1 hunks)apps/web/src/components/icons/brand/Opencut.astro(1 hunks)apps/web/src/components/icons/sponsors/Neon.astro(1 hunks)apps/web/src/components/icons/sponsors/Upstash.astro(1 hunks)apps/web/src/components/ui/AccordionItem.astro(1 hunks)apps/web/src/layouts/BlogLayout.astro(1 hunks)apps/web/src/layouts/Layout.astro(1 hunks)apps/web/src/pages/404.astro(1 hunks)apps/web/src/pages/blog/[slug].astro(1 hunks)apps/web/src/pages/blog/category/[slug].astro(1 hunks)apps/web/src/pages/blog/index.astro(1 hunks)apps/web/src/pages/changelog/[slug].astro(1 hunks)apps/web/src/pages/changelog/index.astro(1 hunks)apps/web/src/pages/contributors/index.astro(1 hunks)apps/web/src/pages/index.astro(1 hunks)apps/web/src/pages/pricing/index.astro(1 hunks)apps/web/src/pages/privacy/index.astro(1 hunks)apps/web/src/pages/sponsors/index.astro(1 hunks)apps/web/src/pages/terms/index.astro(1 hunks)
✅ Files skipped from review due to trivial changes (32)
- apps/web/src/components/PricingCard.astro
- apps/web/src/components/Header.astro
- apps/web/src/components/Welcome.astro
- apps/web/src/components/BlogHeader.astro
- apps/web/src/components/icons/sponsors/Neon.astro
- apps/web/src/components/icons/brand/Opencut.astro
- apps/web/src/components/ScrollToTop.astro
- apps/web/src/pages/changelog/index.astro
- apps/web/src/pages/blog/[slug].astro
- apps/web/src/components/CategoryCard.astro
- apps/web/src/components/icons/brand/Ia.astro
- apps/web/src/components/icons/brand/Helix.astro
- apps/web/src/pages/terms/index.astro
- apps/web/src/pages/index.astro
- apps/web/src/components/icons/Github.astro
- apps/web/src/components/icons/brand/Bounty.astro
- apps/web/src/components/icons/sponsors/Upstash.astro
- apps/web/src/pages/pricing/index.astro
- apps/web/src/pages/blog/category/[slug].astro
- apps/web/src/pages/404.astro
- apps/web/src/components/Footer.astro
- apps/web/src/components/ui/AccordionItem.astro
- apps/web/src/components/Container.astro
- apps/web/src/pages/changelog/[slug].astro
- apps/web/src/components/icons/brand/Mantlz.astro
- apps/web/src/components/icons/WordMark.astro
- apps/web/src/components/Head.astro
- apps/web/src/components/PostCard.astro
- apps/web/src/components/icons/X.astro
- apps/web/src/components/ChangelogCard.astro
- apps/web/src/components/icons/Discord.astro
- apps/web/src/components/icons/brand/Databuddy.astro
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/cms/src/components/editor/extensions/markdown-paste/utils.ts
- apps/cms/src/components/editor/extensions/image-upload/hooks.ts
- apps/cms/src/components/editor/slash-command/index.ts
- apps/cms/src/components/editor/extensions/table/TableHeader.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Avoid functions exceeding allowed Cognitive Complexity
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of over Array.forEach
Don't create classes that only have static members
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules
Don't use unnecessary escape sequences in regex literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports/exports/destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when no escapes are present
Don't use useless case clauses in switch
Avoid ternaries when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use the void operator
Prefer arrow functions over function expressions
Use Date.now() for epoch milliseconds
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed when possible
Don't use parseInt/Number.parseInt when numeric literals suffice (bin/oct/hex)
Use optional chaining instead of chained logical expressions
Prefer regex literals over RegExp constructor when possible
Don't use non-base-10 or underscore separators in number member names
Remove redundant terms from logical expressions
Prefer while loops when initializer/update not needed in for
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp when result is constant
Don't return a value from a constructor
Don't use empty character classes in regex literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions/vars a...
Files:
apps/cms/src/components/editor/extensions/table/TableCell.tsapps/cms/src/components/editor/ai/readability-suggestions.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations when initialized with literals
Don't use TypeScript namespaces
Don't use non-null assertions (postfix !)
Don't use parameter properties in class constructors
Don't use user-defined types (type predicates abuse)
Prefer as const over literal types/type annotations
Use array type consistently (either T[] or Array)
Initialize each enum member value explicitly
Use export type for types
Use import type for types
All enum members must be literal values
Don't use const enum
Don't declare empty interfaces
Don't let variables evolve into any via reassignments
Don't use the any type
Don't misuse non-null assertion operator
No implicit any in variable declarations
Don't merge interfaces and classes unsafely
Place overload signatures adjacent
Use namespace keyword instead of module to declare TS namespaces
Use consistent accessibility modifiers on class members
Use function types instead of object types with call signatures
Don't use void type outside of generic or return types
Files:
apps/cms/src/components/editor/extensions/table/TableCell.tsapps/cms/src/components/editor/ai/readability-suggestions.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't use the accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, or properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex
Don't include "image", "picture", or "photo" in img alt text
Don't use an explicit role identical to the element's implicit role
Static elements with click handlers must have a valid role
Always include a <title> element for SVGs
Provide meaningful alt text for elements requiring it
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include type attribute for button elements
Elements with interactive roles/handlers must be focusable
Headings must have content accessible to screen readers (not aria-hidden)
Always include lang attribute on the html element
Always include title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp/onKeyDown/onKeyPress
Pair onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements over role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles
Use valid values for the autocomplete attribute on inputs
Use correct ISO codes for the lang attribute
Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Ensure all ...Files:
apps/cms/src/components/editor/ai/readability-suggestions.tsx**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't destructure props inside JSX components in Solid projects
Files:
apps/cms/src/components/editor/ai/readability-suggestions.tsx🧠 Learnings (20)
📓 Common learnings
Learnt from: brandonmcconnell Repo: usemarble/marble PR: 113 File: apps/web/src/components/BlogHeader.astro:69-0 Timestamp: 2025-08-20T06:19:19.253Z Learning: Focus review comments on the specific scope of Tailwind CSS upgrade PRs. Pre-existing accessibility, HTML validity, or design issues should be noted for separate PRs rather than being flagged in upgrade-focused PRs, even if they're encountered during the review.Learnt from: brandonmcconnell Repo: usemarble/marble PR: 113 File: apps/cms/src/components/editor/editor-page.tsx:0-0 Timestamp: 2025-08-20T06:13:45.705Z Learning: When reviewing Tailwind CSS upgrade PRs, focus on upgrade-related issues rather than flagging pre-existing accessibility or design concerns that should be addressed in separate PRs focused on those specific improvements.📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{tsx,jsx} : Don't destructure props inside JSX components in Solid projectsApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astroapps/web/src/layouts/Layout.astroapps/web/src/components/CategoryFilter.astro📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{jsx,tsx} : Suffix icon components with "Icon" (e.g., UploadSimpleIcon)Applied to files:
apps/web/src/components/icons/Logo.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Include all required ARIA attributes for elements with ARIA rolesApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't use dangerous JSX propsApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Always include type attribute for button elementsApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Always include a <title> element for SVGsApplied to files:
apps/web/src/components/icons/Logo.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Ensure ARIA properties are valid for the element's supported rolesApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Prefer semantic elements over role attributes in JSXApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Ensure all aria-* properties are validApplied to files:
apps/web/src/components/icons/Logo.astroapps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{ts,tsx,js,jsx} : Use standard constants over approximated literalsApplied to files:
apps/web/src/components/icons/Logo.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't assign to React component propsApplied to files:
apps/web/src/components/icons/Logo.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't add ARIA roles, states, or properties to elements that don't support themApplied to files:
apps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Use valid, non-abstract ARIA rolesApplied to files:
apps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Always include title attribute for iframe elementsApplied to files:
apps/web/src/components/Prose.astro📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to **/*.{jsx,tsx} : In Next.js, don't use <head> elementsApplied to files:
apps/web/src/layouts/Layout.astroapps/web/src/layouts/BlogLayout.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Don't define React components inside other componentsApplied to files:
apps/web/src/layouts/Layout.astro📚 Learning: 2025-10-08T13:24:41.703Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.703Z Learning: Applies to pages/_document.@(js|jsx|ts|tsx) : Don't use next/head in pages/_document.*Applied to files:
apps/web/src/layouts/Layout.astroapps/web/src/layouts/BlogLayout.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Don't import next/document outside of pages/_document.jsxApplied to files:
apps/web/src/layouts/Layout.astroapps/web/src/layouts/BlogLayout.astro📚 Learning: 2025-10-08T13:24:41.702Z
Learnt from: CR Repo: usemarble/marble PR: 0 File: .cursor/rules/ultracite.mdc:0-0 Timestamp: 2025-10-08T13:24:41.702Z Learning: Applies to **/*.{jsx,tsx} : Use <>...</> instead of <Fragment>...</Fragment>Applied to files:
apps/web/src/layouts/Layout.astro🧬 Code graph analysis (1)
apps/cms/src/components/editor/extensions/table/TableCell.ts (1)
apps/cms/src/components/editor/extensions/table/utils.ts (3)
getCellsInColumn(82-117)isRowSelected(52-65)selectRow(252-252)🔇 Additional comments (8)
apps/web/src/pages/sponsors/index.astro (1)
2-30: Formatting-only changes unrelated to PR scope.These changes are purely cosmetic (whitespace, indentation, and semicolon terminator) with no functional impact. The code is correct and well-structured.
Note: This sponsors page is outside the scope of the Tiptap v3 editor migration. These appear to be incidental formatting changes, possibly from a linter or formatter run across the codebase.
apps/web/src/pages/blog/index.astro (1)
2-10: LGTM — formatting normalization.The indentation changes to the Astro frontmatter section are purely cosmetic with no functional impact. The imports and data fetching logic remain correct.
apps/web/src/components/Prose.astro (1)
1-13: LGTM! Clean implementation of class composition pattern.The addition of the
cn()utility and its integration into the component follows best practices for composable styling. The implementation correctly:
- Imports and uses the
cn()utility to merge base Tailwind classes with consumer-provided classes- Destructures props to extract the
classattribute asclassNameswhile spreading remaining attributes- Maintains backward compatibility (components not passing custom classes work identically)
- Uses valid Tailwind v4 syntax throughout (no breaking changes patterns detected)
This enhancement adds styling flexibility while keeping the component clean and maintainable.
apps/cms/src/components/editor/ai/readability-suggestions.tsx (2)
5-5: Type migration to Tiptap v3 Editor is correct.The import and type changes properly migrate from
EditorInstancetoEditorfrom@tiptap/core, aligning with the Tiptap v3 upgrade.Also applies to: 15-15
21-54: Editor API usage is compatible with Tiptap v3.The function signature correctly uses the new
Editortype, and all ProseMirror/Tiptap APIs (state.doc.descendants,chain,focus,setTextSelection) remain compatible with v3.apps/web/src/pages/contributors/index.astro (1)
2-181: LGTM! Enhanced type definitions and formatting improvements.The changes to this file enhance type safety with more complete GitHub API type definitions and apply consistent formatting. The type structures correctly match the GitHub API v3 response schema, and the data fetching logic remains functionally equivalent.
Note: This file appears unrelated to the Tiptap v3 editor migration PR objective. The changes likely resulted from broader repository maintenance (TypeScript improvements, formatting updates).
apps/web/src/layouts/Layout.astro (2)
12-20: Component usage looks correct.The template correctly uses the renamed components. The structure is clean and follows standard Astro patterns:
HeadComponentproperly receives the required props (title, description, image)- Layout hierarchy is semantic and well-organized
- All component invocations match the updated import names
2-4: No issues found. The code changes are correct and all references have been updated consistently.The import names in Layout.astro are valid Astro syntax. In Astro, default exports can be imported with any local alias name—you don't need the component files to export those specific names. The imports
import FooterComponent from "@/components/Footer.astro"are correct and import the default exports with local aliases.Verification confirms all usages across the codebase (Layout.astro and BlogLayout.astro) have been updated to use the new component names consistently. No breaking changes exist.
Minor note: ScrollToTop was not renamed to ScrollToTopComponent (naming inconsistency), though this has no functional impact.
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.
Since this adds support for tables it would also be good to make sure the parser supports them too and has the proper tests to make sure files with tables can be imported.
Description
This PR migrates the Marble editor from Novel (a Tiptap v2 wrapper) to native Tiptap v3, bringing significant
improvements to the editor experience and removing an unnecessary abstraction layer.
Key Changes:
enhanced table support with row/column menus
Package Changes:
[email protected]@tiptap/react@^3.x,@tiptap/core@^3.x,@tiptap/pm@^3.x,@tiptap/starter-kit@^3.x@tiptap/extension-*(image, youtube, list, table, code-block-lowlight, etc.)@floating-ui/dom@^1.6.0for menu positioningArchitecture Changes:
EditorRootwith native TiptapuseEditorhookcreateImageUploadMotivation and Context
Why migrate from Novel to Tiptap v3?
Breaking changes handled:
shouldRerenderOnTransactionnow explicit)How to Test
Basic Editor Functionality
strikethrough, underlineRich Content Features
/image→ paste URL → verify embed works/youtube→ paste video URL → verify embedNew Features
/columns→ verify 2-column layoutSlash Commands
/in editor → verify command menu appears/text,/h2,/h3,/h4,/list,/numbered,/todo,/quote,/code,/image,/youtube,/hr,/columnsData Persistence & Compatibility
contentfield)contentJsonfield)Form Integration & Metadata
Sidebar Functionality
UI/UX & Bubble Menu
Edge Cases
Browser & Responsive Testing
Build & Type Safety
pnpm build→ verify no build errorspnpm lint→ verify no linting errorsTypes of Changes
Additional Notes
Migration Complexity: This was a medium-high complexity migration involving:
Backwards Compatibility:
Testing Status:
Rollback Plan:
[email protected]if critical issues foundFollow-up Tasks (if any):
closes #183
Summary by CodeRabbit
New Features
Improvements
Removed