setup file handler extension & tiptap styling#1748
Conversation
📝 WalkthroughWalkthroughThe changes introduce file handling capabilities to the TipTap editor by adding a Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Editor as Editor Component
participant FileHandler as FileHandler Extension
participant Callbacks as fileHandlerConfig Callbacks
participant ImageOps as Image Operations
User->>Editor: Drag/Drop or Paste Files
Editor->>FileHandler: Trigger file event
FileHandler->>Callbacks: Call onDrop/onPaste (if provided)
alt Custom Handler Returns False
Callbacks-->>FileHandler: false
FileHandler->>FileHandler: Skip default handling
else Custom Handler Proceeds
Callbacks-->>FileHandler: true or void
FileHandler->>ImageOps: Read dropped/pasted files as Data URLs
ImageOps->>ImageOps: Filter for image MIME types
ImageOps->>Editor: Insert AttachmentImage nodes
Editor->>User: Display images with attachmentId
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tiptap/src/editor/index.tsx (1)
19-27: Add documentation about stablefileHandlerConfigto prevent unnecessary editor re-creationThe concern in the original comment is validated: if the extensions array changes identity, useEditor will create a new editor instance, with side effects like lost selection/focus/undo stack. Since
fileHandlerConfigis included in theuseMemodeps forextensions(lines 58–64), callers must pass a stable config object or memoize their handlers.Add a JSDoc note to
EditorProps(lines 19–27) or a comment above the component's extension setup explicitly recommending that callers memoize or stabilizefileHandlerConfigto avoid unintended editor re-instantiation. Reference that Tiptap React docs explain useEditor behavior and recommend keeping values stable to avoid recreation.
🧹 Nitpick comments (3)
packages/tiptap/src/styles/base.css (1)
35-39: Clarify the intent of the 240px image width constraintStyling
.tiptap-imagewithmax-width: 240px; height: auto; display: block;is fine, but it hard‑caps images to a relatively small width. If these are meant to behave more like inline attachments/thumbnails this is good; if they should be responsive content images, considermax-width: 100%(optionally with a separate thumbnail style) so they can expand to the editor width on larger screens.packages/tiptap/src/shared/extensions/index.ts (2)
129-185: Consider using batch insertion or position advancement for multiple filesThe FileHandler integration delegates correctly to user handlers, but the multi-file insertion pattern could align better with Tiptap's documented approach:
posis documented as always a number (the position the file was dropped at), so a defensive guard is unnecessary.- For inserting multiple files, Tiptap recommends either: (1) batch insert with
insertContentAt(pos, [nodeA, nodeB, ...]), or (2) iterate while advancingposby each inserted node's size. The current code inserts all files at the sameposasynchronously, which works but may not provide the insertion ordering you intend if user edits interleave.FileReader.resultis safe afterreadAsDataURL, but a defensivetypeofcheck is reasonable if you want to be extra cautious against API misuse.
20-23: Strengthen typing forFileHandlerConfigand make MIME types configurableVerification confirms both recommendations are valid and actionable:
Type
editorparameter: ImportEditorfrom@tiptap/core(already used inplaceholder.tsxin the same package) and updateFileHandlerConfigto useEditorinstead ofany:import { Editor } from "@tiptap/core"; export type FileHandlerConfig = { onDrop?: (files: File[], editor: Editor, position?: number) => boolean | void; onPaste?: (files: File[], editor: Editor) => boolean | void; };Make
allowedMimeTypesconfigurable: Add an optionalallowedMimeTypesfield toFileHandlerConfigand use it inFileHandler.configure(lines 123–126). This allows callers to override the hard-coded image types without modifying this central file:export type FileHandlerConfig = { onDrop?: (files: File[], editor: Editor, position?: number) => boolean | void; onPaste?: (files: File[], editor: Editor) => boolean | void; allowedMimeTypes?: string[]; };Then in the
FileHandler.configurecall, use:allowedMimeTypes: fileHandlerConfig?.allowedMimeTypes ?? ["image/png", "image/jpeg", "image/gif", "image/webp"]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/tiptap/src/editor/index.tsx(5 hunks)packages/tiptap/src/shared/extensions/index.ts(3 hunks)packages/tiptap/src/styles/base.css(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/tiptap/src/editor/index.tsx (1)
packages/tiptap/src/shared/extensions/index.ts (1)
FileHandlerConfig(20-23)
packages/tiptap/src/shared/extensions/index.ts (1)
packages/tiptap/src/shared/extensions/placeholder.tsx (1)
PlaceholderFunction(8-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (2)
packages/tiptap/src/shared/extensions/index.ts (1)
25-41: AttachmentImage extension is correct and verified against Tiptap v3.10.x documentationThe extension properly extends
Imageand merges parent attributes viathis.parent?.()to addattachmentIdmapped todata-attachment-id, following Tiptap's recommended pattern. Configuration withallowBase64: trueandHTMLAttributes: { class: "tiptap-image" }correctly enables data: URLs produced byFileReader.readAsDataURL()and applies custom styling to rendered<img>tags, as documented in Tiptap v3.10.x.packages/tiptap/src/editor/index.tsx (1)
160-162: EditorContent class merging confirmed—CSS layout assumptions are soundVerification confirms that
EditorContentmerges itsclassName="tiptap-root"prop witheditorProps.attributes.class = "tiptap-normal"onto the same DOM element, producing a single element with both classes (e.g.,class="tiptap-root tiptap-normal"). This flat structure means your flex layout and min-height styles inbase.csswill apply as expected.
| .tiptap-root { | ||
| height: 100%; | ||
| display: flex; | ||
| flex-direction: column; | ||
| } | ||
|
|
||
| .tiptap-root .tiptap { | ||
| flex: 1 1 auto; | ||
| min-height: 100%; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm .tiptap-root / inner editor structure so flex+min-height actually apply
The layout assumes a .tiptap-root container with a nested .tiptap element, and .tiptap-normal inside with min-height: 100%. Please double‑check the actual DOM that EditorContent + editorProps.attributes.class = "tiptap-normal" produce; if there isn’t a real .tiptap-root .tiptap descendant chain, the flex growth and min-height rules here won’t take effect and you may want to target .tiptap-root and/or .tiptap-normal directly instead.
Also applies to: 19-19
🌐 Web query:
Check how `@tiptap/react`’s `EditorContent` applies `className` and `editorProps.attributes.class` in version 3.10.7. Specifically: does passing `className="tiptap-root"` plus `editorProps.attributes.class = "tiptap-normal"` result in a `.tiptap-root .tiptap-normal` structure, or just a single root element with both classes?
💡 Result:
Short answer: a single root element with both classes (not nested). EditorContent renders one element and applies your component className together with the editor's editorProps.attributes.class to that same root, so you'll see one element with both classes (e.g. class="tiptap-root tiptap-normal"). Source: Tiptap React docs and the Editor API note on editorProps/attributes. [1][2]
Sources
- React integration / EditorContent usage (EditorContent is the React wrapper for the editor DOM). [1]
- Editor API — editorProps.attributes.class (how the editor's attributes/classes are applied). [2]
Fix CSS selectors: .tiptap-root .tiptap descendant rule won't apply (dead code)
Based on @tiptap/react's EditorContent implementation, both className="tiptap-root" and editorProps.attributes.class = "tiptap-normal" are applied to a single root element (e.g., class="tiptap-root tiptap-normal"), not nested elements. The CSS rule .tiptap-root .tiptap on lines 7–10 is unreachable dead code because there is no .tiptap child element inside .tiptap-root.
To fix: either target .tiptap-root directly with the flex and min-height rules, or adjust selectors to match your actual DOM structure. If .tiptap-normal is on the root, use .tiptap-root.tiptap-normal or simplify to .tiptap-normal.
🤖 Prompt for AI Agents
packages/tiptap/src/styles/base.css lines 1-10: the descendant selector
`.tiptap-root .tiptap` is dead because EditorContent applies both classes on the
same root element (e.g., `class="tiptap-root tiptap-normal"`); replace the
unreachable selector with one that matches the actual DOM (e.g., target
`.tiptap-root` directly for flex/min-height, or use the combined selector
`.tiptap-root.tiptap-normal` or `.tiptap-normal`) so the flex layout and
min-height rules apply to the root element.
1 participant
No description provided.