Repository navigation
docs: Tiptap extension audit and paragraph node strategy (Phase 1) - #281
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Phase 1 decision document for Tiptap integration: audits Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/phase-1-tiptap-strategy.md (2)
127-128: Verify direct dependency imports for custom node implementation.Lines 127-128 state that
@tiptap/coreand@tiptap/pmbecome direct dependencies becauseBlockParagraphimports from them (specificallyNode.create,Plugin,TextSelection). Consider verifying these specific imports when implementing#272to ensure:
- The exact imports needed are documented
- No additional transitive dependencies need to become direct
- The import patterns follow Tiptap's current best practices for custom node extensions
If you'd like, I can help draft a verification checklist or code snippet template for the BlockParagraph implementation that confirms these imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phase-1-tiptap-strategy.md` around lines 127 - 128, Confirm and document the exact direct imports required by your custom node implementation: inspect BlockParagraph (and BlockHeading) for any usage of Node.create, Plugin, TextSelection and list them explicitly (e.g., import { Node } from '@tiptap/core'; import { Plugin } from '@tiptap/pm/state' or whichever precise paths are used), update the docs to show those import lines, scan the BlockParagraph code for any other symbols that reference transitive packages and promote them to direct deps if referenced, and ensure the import patterns match current Tiptap best practices for custom Node/Extension implementations (adjust import sources and named exports to the latest Tiptap API where necessary).
64-68: Consider explicitly documenting required store operations for split/merge.The keymap handlers described here assume the store supports block splitting and merging. While
insertBlockandremoveBlockexist (per context snippet 1), the document could explicitly note whether additional store operations or helper methods are needed to support:
- Splitting a block's content at a caret position
- Merging content from one block into a previous block at a specific position
- Managing focus/caret positioning across block boundaries
This would help implementers in
#272understand the full scope of store changes required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phase-1-tiptap-strategy.md` around lines 64 - 68, The doc should explicitly list the store operations and contracts that BlockParagraph's keymap handlers require: describe a split operation (e.g. a store method like splitBlockAt(blockId, offset) that takes a block id and caret offset and returns/dispatches the left/right content or new block id), a merge operation (e.g. mergeBlockInto(prevBlockId, blockId, joinOffset) that appends content and resolves the resulting caret position), and focus/caret control (e.g. focusBlock(blockId, offset) or setCaret(blockId, offset)) in addition to the existing insertBlock and removeBlock; also specify expected inputs/outputs and whether insertBlock/removeBlock should accept initial content and a desired caret position so implementers know how to wire splitting, merging and focus movement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Line 37: Confirm whether a concrete tracking issue for store-level undo/redo
exists under the `#237` breakdown; if it does not, create one before `#267` is
closed and link it from the document (reference the Phase 1 doc's "Open
Follow-ups"/section 5 and the line noting "tracked outside `#267`") so the
requirement for a single store-level undo/redo (and disabling Tiptap History
per-editor) is explicitly tracked and prioritized for Phase 1 completion;
include the issue number in the doc where line 37 currently references `#267`.
---
Nitpick comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Around line 127-128: Confirm and document the exact direct imports required by
your custom node implementation: inspect BlockParagraph (and BlockHeading) for
any usage of Node.create, Plugin, TextSelection and list them explicitly (e.g.,
import { Node } from '@tiptap/core'; import { Plugin } from '@tiptap/pm/state'
or whichever precise paths are used), update the docs to show those import
lines, scan the BlockParagraph code for any other symbols that reference
transitive packages and promote them to direct deps if referenced, and ensure
the import patterns match current Tiptap best practices for custom
Node/Extension implementations (adjust import sources and named exports to the
latest Tiptap API where necessary).
- Around line 64-68: The doc should explicitly list the store operations and
contracts that BlockParagraph's keymap handlers require: describe a split
operation (e.g. a store method like splitBlockAt(blockId, offset) that takes a
block id and caret offset and returns/dispatches the left/right content or new
block id), a merge operation (e.g. mergeBlockInto(prevBlockId, blockId,
joinOffset) that appends content and resolves the resulting caret position), and
focus/caret control (e.g. focusBlock(blockId, offset) or setCaret(blockId,
offset)) in addition to the existing insertBlock and removeBlock; also specify
expected inputs/outputs and whether insertBlock/removeBlock should accept
initial content and a desired caret position so implementers know how to wire
splitting, merging and focus movement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 311b6841-0e51-46d0-b6d4-6b3f254830c4
📒 Files selected for processing (1)
docs/phase-1-tiptap-strategy.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/phase-1-tiptap-strategy.md (1)
86-86: Tighten wording for precision.At Line 86, “very easy” is vague; replacing it with a more specific qualifier will read cleaner in a decision doc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phase-1-tiptap-strategy.md` at line 86, The phrase "very easy" in the sentence starting with "Option A makes it very easy to accidentally ship..." is vague; update that sentence to use a more precise qualifier (for example "likely," "highly likely," or "prone to") to convey the specific risk—edit the sentence under "Option A" so it reads something like "Option A makes it likely to accidentally ship..." (referencing the "Option A" paragraph) to tighten the wording for precision.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Line 40: Update the summary sentence so its counts and categories exactly
match the audit table in §1: change the classification of History from “drops”
to “replace” (or whatever the table specifies), adjust the drop/keep/replace
totals so the summed numbers (out of ~18 items) are correct, and ensure the
listed items (Document, Text, HardBreak, Bold, Italic, Paragraph →
BlockParagraph, Link, History) reflect the same categories/labels used in the
table to avoid ambiguity for PR `#272`.
---
Nitpick comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Line 86: The phrase "very easy" in the sentence starting with "Option A makes
it very easy to accidentally ship..." is vague; update that sentence to use a
more precise qualifier (for example "likely," "highly likely," or "prone to") to
convey the specific risk—edit the sentence under "Option A" so it reads
something like "Option A makes it likely to accidentally ship..." (referencing
the "Option A" paragraph) to tighten the wording for precision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a06963d2-40c2-43e3-8f80-8060fe435d1f
📒 Files selected for processing (1)
docs/phase-1-tiptap-strategy.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/phase-1-tiptap-strategy.md (1)
66-66: Clarify "block boundary" terminology.The phrase "Enter at block boundary" is ambiguous—boundary could mean start, end, or both. Based on the subsequent description ("split the current text at the caret"), this appears to mean "end of block." Consider replacing "at block boundary" with "at end of block" for clarity.
📝 Proposed clarification
-- **`Enter` at block boundary:** prevent PM's default, split the current text at the caret, dispatch `insertBlock({ type: 'paragraph', content: <right half> })` to the store with the current block reduced to the left half, and move focus/caret to the new block's editor. Shift+Enter still inserts a `HardBreak` (kept in §1). +- **`Enter` at end of block:** prevent PM's default, split the current text at the caret, dispatch `insertBlock({ type: 'paragraph', content: <right half> })` to the store with the current block reduced to the left half, and move focus/caret to the new block's editor. Shift+Enter still inserts a `HardBreak` (kept in §1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/phase-1-tiptap-strategy.md` at line 66, Replace the ambiguous phrase "Enter at block boundary" with "Enter at end of block" in the description so it clearly means caret at the end of the block; update the sentence that follows (the one mentioning splitting the current text at the caret and dispatching insertBlock({ type: 'paragraph', content: <right half> })) to use the new wording and ensure Shift+Enter behavior (HardBreak) remains noted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Line 37: Update the documentation text about Tiptap's History to past tense to
reflect that store-level undo/redo is already implemented: change phrases like
"must land before Phase 1 closes" and "must be in place" (the History table row
and the two other mentions) to indicate the feature landed in `#266` or "is
implemented at the store level"; reference the existing implementation in
editorStore.ts and its tests in editorStoreHistory.test.ts when choosing the
replacement wording.
---
Nitpick comments:
In `@docs/phase-1-tiptap-strategy.md`:
- Line 66: Replace the ambiguous phrase "Enter at block boundary" with "Enter at
end of block" in the description so it clearly means caret at the end of the
block; update the sentence that follows (the one mentioning splitting the
current text at the caret and dispatching insertBlock({ type: 'paragraph',
content: <right half> })) to use the new wording and ensure Shift+Enter behavior
(HardBreak) remains noted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 44f8dce6-675d-4e4c-8f8b-00a90b978791
📒 Files selected for processing (1)
docs/phase-1-tiptap-strategy.md
Description
Adds
docs/phase-1-tiptap-strategy.md, a decision document that resolves Phase 0 finding #4. It audits every extension in@tiptap/starter-kit, decides the paragraph node strategy, and produces the final Tiptap dependency list that #272 will consume when it ships the paragraph and heading blocks.Closes: #267
Type of Change
Related Issue
Issue: #267
Motivation and Context
Phase 0 shipped the editor spike with
@tiptap/starter-kitbecause it was the fastest path to a working paragraph. The starter kit drags in ~15 extensions, most of which conflict with our block-aware architecture — headings, lists, blockquote, and code block are block types in our system, not ProseMirror nodes living inside a paragraph. Without an explicit audit, every paragraph's Tiptap editor can produce document structure the block store cannot represent.This decision needed to land before #272 so the paragraph and heading blocks can be implemented against a locked extension list and a clear answer on split/merge behavior.
Changes Made
@tiptap/starter-kitwith keep / drop / replace decisions and reasoningParagraphwith a customBlockParagraphnode (Option B) that dispatches split/merge to the Zustand block store, with a sharedcreateBlockAwareKeymaphelper reused by the heading blockstarter-kitremoval plan and final dependency list for Phase 1.10: Paragraph block, heading block, shared rich-text toolbar #272 (individual@tiptap/extension-*packages, no meta-package)History, and deferred marks/extensions (strike,code,dropcursor) for later phasesHow Has This Been Tested?
Testing Environment:
add/phase-one)Tests Performed:
PHASE-0-FINDINGS.mdfinding Create A Fluid Editor Section In The Visual Editor #4 to ensure every concern raised is addressedgit diff --statshows one markdown file added)Accessibility Tests Run
Details:
n/a — documentation-only change. No UI or runtime behavior is affected. The decisions in the doc (particularly disabling Tiptap's per-editor
Historyin favor of store-level undo/redo) protect future keyboard UX by avoiding confusing per-block undo stacks.Tests Added
Test details:
No tests added or run — this is a decision-only document with no code changes. The split/merge and toolbar behavior called out in the doc will be tested by #272 when the paragraph and heading blocks are implemented.
Documentation
Documentation details:
New file:
docs/phase-1-tiptap-strategy.md. Follows the existingdocs/plans/conventions. Supersedes Phase 0 finding #4 inPHASE-0-FINDINGS.mdand is referenced as the input to #272.Screenshots
n/a
Pre-Submission Checklist
Summary by CodeRabbit