feat: use tiptap for the editor implementation #11
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
editor | f5f353c | Commit Preview URL Branch Preview URL |
Apr 02 2026, 11:07 AM |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces the custom SolidJS contentEditable editor with a Tiptap/ProseMirror-backed editor (StarterKit, Highlight). Moves search/highlight/selection/replace logic into App using ProseMirror APIs, removes legacy caret/highlight helpers and Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Editor as TiptapEditor
participant Doc as ProseMirrorDoc
User->>App: type / invoke search / trigger replace
App->>Editor: ensure editorInstance (onEditorReady)
Editor->>Doc: read editor.state.doc.descendants to find matches
App->>Editor: apply transaction to add/remove highlight marks
User->>App: navigate to match / request replace
App->>Editor: setTextSelection(from,to)
App->>Editor: insertContent(replacement)
Editor->>Doc: apply transaction -> update document
App->>Editor: clear highlights / update selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/components/Editor.tsx (4)
219-231: Direct state mutation in onMount bypasses model API.Lines 222-223 set cursor then directly mutate
state.text. This works but is order-dependent — if the order were reversed, the cursor wouldn't be properly clamped to the new text length. Consider extracting a helper that ensures consistency, or document whysetText()isn't used (to preserve undo history).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor.tsx` around lines 219 - 231, The onMount block is directly mutating modelInstance.state.text after calling setCursor which creates an order-dependent bug; replace the direct mutation with a small helper on the model (or a local helper function) that sets the text via the model API and then clamps/sets the cursor (e.g., setTextAndClampCursor(modelInstance, props.content)) or call an existing modelInstance.setText(...) followed by setCursor(modelInstance, newPos), and if you intentionally bypass setText to preserve undo history, add a clear comment on modelInstance.state.text explaining why and what invariants are required; update references in the onMount flow (model(), setCursor, textareaRef, setInitialized) accordingly.
284-287: Controlled textarea with manual value assignments.The textarea uses
value={props.content}(controlled), but handlers also assigntextareaRef.valuedirectly. This works becauseonChangesynchronizes state before the next render, but the dual control approach could be fragile. Consider using either fully controlled (relying on props.content) or uncontrolled (using refs only) for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor.tsx` around lines 284 - 287, Textarea is currently mixed-controlled: it uses value={props.content} while handlers (handleInput/handleKeyDown) write directly to textareaRef.value; pick one approach—prefer fully controlled—by removing direct DOM writes in handleInput and handleKeyDown, ensure these handlers call the provided updater (e.g., onChange/onContentChange prop) to update parent state so props.content drives the textarea, and keep textareaRef for reading cursor/selection only; alternatively switch to uncontrolled by replacing value with defaultValue={props.content} and keeping ref-based writes. Update references to textareaRef, handleInput, and handleKeyDown accordingly so only one control strategy is used.
55-57: Fixed character width assumes monospace font.The
0.6 * fontSizecalculation works for monospace fonts but will cause cursor misalignment with variable-width fonts. Consider documenting this limitation or dynamically measuring character width using a canvas or hidden element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor.tsx` around lines 55 - 57, getCharWidth currently returns a fixed props.settings.fontSize * 0.6 which only works for monospace fonts; change getCharWidth to dynamically measure text width (e.g., create an offscreen canvas or hidden element, set its font using props.settings.fontSize/fontFamily/fontWeight, call context.measureText for a representative character or average of characters, cache the result) so cursor calculations work with variable-width fonts, or alternatively add a clear comment on getCharWidth documenting the monospace limitation and where to change it.
40-42: Document minimum browser requirements or add a polyfill forRegExp.escape.
RegExp.escaperequires modern browsers: Chrome/Edge 136+, Firefox 134+, Safari 18.2+. Without explicit browser compatibility targets documented in the project, consider either (1) documenting the minimum required browser versions, or (2) adding a polyfill (e.g.,regexp.escapefrom npm) for broader compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Editor.tsx` around lines 40 - 42, The code in Editor.tsx uses RegExp.escape when building the search regex (see RegExp.escape and variable searchEscaped which wraps escapeHtml(props.searchTerm)), which is unsupported in older browsers; either document the minimum browser versions in the project README or add a runtime polyfill: install/import a stable polyfill (e.g., the regexp.escape package or a small local fallback that defines RegExp.escape if missing) and ensure it is loaded before Editor.tsx executes so the RegExp.escape call is safe across targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Editor.tsx`:
- Around line 59-71: The cursor positioning in updateCursorPosition (and the
duplicate logic around lines 263-270) ignores the container's padding, causing
misalignment; fix by reading the container/textarea computed padding (via
textareaRef or its parent) and add the vertical padding-top to the computed top
and the horizontal padding-left to the computed left before setting
cursorRef.style.top/left, preserving existing use of getLineHeight(),
getCharWidth(), and model() so the offset is applied consistently wherever
updateCursorPosition logic appears.
- Around line 73-89: handleInput currently assumes a single-char insert (using
text.slice(currentOffset - 1, currentOffset)); change it to compute the actual
inserted substring by comparing text.length with the model text length: let
delta = text.length - modelText.length; if delta > 0, compute inserted =
text.slice(currentOffset - delta, currentOffset) and call
insertAtCursor(modelInstance, inserted); (handle delta <= 0 as no-insert or
deletion cases). Keep setCursor(modelInstance, currentOffset),
props.onChange(getText(modelInstance)), and updateCursorPosition() as before and
use the same symbols: handleInput, textareaRef, model(), getText, setCursor,
insertAtCursor, props.onChange, updateCursorPosition.
- Around line 91-211: The keydown handler handleKeyDown is missing handling for
the Delete key: import and call deleteForward(model()) analogous to
deleteBackward usage, then update the textarea and state; specifically add
deleteForward to the imports, and insert a new block after the Backspace case
that e.preventDefault(), calls deleteForward(modelInstance), updates
textareaRef.value via getText(modelInstance), sets selection with
getCursorOffset(modelInstance), calls props.onChange(getText(modelInstance)),
and calls updateCursorPosition().
---
Nitpick comments:
In `@src/components/Editor.tsx`:
- Around line 219-231: The onMount block is directly mutating
modelInstance.state.text after calling setCursor which creates an
order-dependent bug; replace the direct mutation with a small helper on the
model (or a local helper function) that sets the text via the model API and then
clamps/sets the cursor (e.g., setTextAndClampCursor(modelInstance,
props.content)) or call an existing modelInstance.setText(...) followed by
setCursor(modelInstance, newPos), and if you intentionally bypass setText to
preserve undo history, add a clear comment on modelInstance.state.text
explaining why and what invariants are required; update references in the
onMount flow (model(), setCursor, textareaRef, setInitialized) accordingly.
- Around line 284-287: Textarea is currently mixed-controlled: it uses
value={props.content} while handlers (handleInput/handleKeyDown) write directly
to textareaRef.value; pick one approach—prefer fully controlled—by removing
direct DOM writes in handleInput and handleKeyDown, ensure these handlers call
the provided updater (e.g., onChange/onContentChange prop) to update parent
state so props.content drives the textarea, and keep textareaRef for reading
cursor/selection only; alternatively switch to uncontrolled by replacing value
with defaultValue={props.content} and keeping ref-based writes. Update
references to textareaRef, handleInput, and handleKeyDown accordingly so only
one control strategy is used.
- Around line 55-57: getCharWidth currently returns a fixed
props.settings.fontSize * 0.6 which only works for monospace fonts; change
getCharWidth to dynamically measure text width (e.g., create an offscreen canvas
or hidden element, set its font using
props.settings.fontSize/fontFamily/fontWeight, call context.measureText for a
representative character or average of characters, cache the result) so cursor
calculations work with variable-width fonts, or alternatively add a clear
comment on getCharWidth documenting the monospace limitation and where to change
it.
- Around line 40-42: The code in Editor.tsx uses RegExp.escape when building the
search regex (see RegExp.escape and variable searchEscaped which wraps
escapeHtml(props.searchTerm)), which is unsupported in older browsers; either
document the minimum browser versions in the project README or add a runtime
polyfill: install/import a stable polyfill (e.g., the regexp.escape package or a
small local fallback that defines RegExp.escape if missing) and ensure it is
loaded before Editor.tsx executes so the RegExp.escape call is safe across
targets.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d40e886-6d47-4bd7-9f95-e403d918b528
📒 Files selected for processing (3)
src/components/Editor.tsxsrc/lib/editor-model.tstests/editor-model.lib.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/App.tsx (1)
266-267:⚠️ Potential issue | 🟠 MajorHighlights not updated when case sensitivity changes.
When
caseSensitiveis toggled viaonCaseSensitiveChange, thematches()memo recomputes, buthighlightMatches()is never called. This leaves stale highlight marks in the editor that don't reflect the new case-sensitive match results.Consider adding an effect to sync highlights when
caseSensitivechanges, or wrapping the toggle handler:const handleCaseSensitiveChange = (value: boolean) => { setCaseSensitive(value); highlightMatches(); };Then use
onCaseSensitiveChange={handleCaseSensitiveChange}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/App.tsx` around lines 266 - 267, The highlights aren't refreshed when case sensitivity changes because although the matches() memo updates, highlightMatches() isn't invoked; update the component to call highlightMatches() whenever case sensitivity toggles by either (a) wrapping setCaseSensitive in a handler (e.g., handleCaseSensitiveChange(value) { setCaseSensitive(value); highlightMatches(); }) and passing that to onCaseSensitiveChange, or (b) adding a useEffect that watches caseSensitive() and calls highlightMatches(); ensure you reference the existing setCaseSensitive, caseSensitive(), matches() memo, and highlightMatches() functions when making the change.
🧹 Nitpick comments (1)
src/components/App.tsx (1)
169-178: Consider batching replacements into a single transaction.Each iteration dispatches a separate transaction, creating N undo steps for N matches. This degrades undo/redo UX—users would need to undo each replacement individually.
Batched replacement approach
- for (const match of sortedMatches) { - editor - .chain() - .focus() - .setTextSelection({ from: match.from, to: match.to }) - .insertContent(replacement) - .run(); - } + const { tr } = editor.state; + for (const match of sortedMatches) { + tr.replaceWith(match.from, match.to, editor.schema.text(replacement)); + } + editor.view.dispatch(tr); + editor.commands.focus();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/App.tsx` around lines 169 - 178, The loop creates one transaction per match (using editor.chain().focus()...run()), producing N separate undo steps; instead, create a single transaction, iterate sortedMatches and apply all replacements to that transaction (e.g., build a tr via editor.state.tr and call tr.replaceRange(match.from, match.to, <replacement node or schema.text(replacement)>) for each match in descending order), then dispatch once with editor.view.dispatch(tr) so all replacements are one undoable step; reference sortedMatches, allMatches, editor.state.tr / editor.view.dispatch, and replaceRange (or equivalent) when locating the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/App.tsx`:
- Around line 48-71: The memo for createMemo that computes matches (matches)
must also depend on editor document changes because editor.state.doc isn't
reactive; add a reactive document-version signal (e.g.,
docVersion()/setDocVersion) updated from the editor's onUpdate callback whenever
the document changes, then include docVersion() as a dependency inside the
createMemo along with editorInstance(), searchTerm(), and caseSensitive() so
matches recomputes on edits and produces fresh from/to positions; update
references to editor.state.doc usage within the matches computation to rely on
the current editor returned by editorInstance() and the latest docVersion
signal.
- Around line 73-91: highlightMatches currently only checks for a falsy editor
but can still attempt to use a destroyed editor instance; update
highlightMatches (and clearHighlights) to return early if the editor has been
destroyed or its view/dispatch method is missing: check a reliable indicator
such as a destroyed flag (e.g., editor.destroyed === true) or verify editor.view
and typeof editor.view.dispatch === "function" before using editor.state,
editor.schema, or calling editor.view.dispatch, and skip processing when those
guards fail so we never dispatch to a destroyed editor.
- Around line 158-159: The current setCurrentMatchIndex(Math.min(idx,
matchPositions.length - 2)) can produce -1 when matchPositions.length is 1;
update the logic in the component where setCurrentMatchIndex and
highlightMatches are called to clamp the computed index to the valid range (0 to
matchPositions.length - 1) and handle the empty-array case (when
matchPositions.length === 0) by setting an appropriate sentinel (e.g., 0 or
null) before calling highlightMatches; reference the setCurrentMatchIndex call,
matchPositions array and the subsequent highlightMatches() invocation when
applying this clamp.
---
Outside diff comments:
In `@src/components/App.tsx`:
- Around line 266-267: The highlights aren't refreshed when case sensitivity
changes because although the matches() memo updates, highlightMatches() isn't
invoked; update the component to call highlightMatches() whenever case
sensitivity toggles by either (a) wrapping setCaseSensitive in a handler (e.g.,
handleCaseSensitiveChange(value) { setCaseSensitive(value); highlightMatches();
}) and passing that to onCaseSensitiveChange, or (b) adding a useEffect that
watches caseSensitive() and calls highlightMatches(); ensure you reference the
existing setCaseSensitive, caseSensitive(), matches() memo, and
highlightMatches() functions when making the change.
---
Nitpick comments:
In `@src/components/App.tsx`:
- Around line 169-178: The loop creates one transaction per match (using
editor.chain().focus()...run()), producing N separate undo steps; instead,
create a single transaction, iterate sortedMatches and apply all replacements to
that transaction (e.g., build a tr via editor.state.tr and call
tr.replaceRange(match.from, match.to, <replacement node or
schema.text(replacement)>) for each match in descending order), then dispatch
once with editor.view.dispatch(tr) so all replacements are one undoable step;
reference sortedMatches, allMatches, editor.state.tr / editor.view.dispatch, and
replaceRange (or equivalent) when locating the code to change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c875d8ee-5b1e-4190-891f-72c909916e4e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
AGENTS.mdpackage.jsonsrc/components/App.tsxsrc/components/Editor.tsxsrc/lib/escape-html.ts
💤 Files with no reviewable changes (1)
- src/lib/escape-html.ts
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Editor.tsx
Comment on lines
+48
to
+71
| const matches = createMemo(() => { | ||
| const editor = editorInstance(); | ||
| const term = searchTerm(); | ||
| if (!editor || !term) return []; | ||
|
|
||
| const results: { from: number; to: number }[] = []; | ||
| const doc = editor.state.doc; | ||
| const searchText = caseSensitive() ? term : term.toLowerCase(); | ||
|
|
||
| doc.descendants((node, pos) => { | ||
| if (!node.isText || !node.text) return; | ||
| const text = caseSensitive() ? node.text : node.text.toLowerCase(); | ||
| let idx = text.indexOf(searchText); | ||
| while (idx !== -1) { | ||
| results.push({ | ||
| from: pos + idx, | ||
| to: pos + idx + term.length, | ||
| }); | ||
| idx = text.indexOf(searchText, idx + 1); | ||
| } | ||
| }); | ||
|
|
||
| return results; | ||
| }); |
There was a problem hiding this comment.
Match positions become stale after document edits.
The matches memo depends on editorInstance(), searchTerm(), and caseSensitive() signals, but editor.state.doc is not reactive. If a user edits the document while search is open, the memo won't recompute, leading to stale match positions. Navigating or replacing with stale positions could select/modify the wrong text or corrupt the document.
Consider triggering a recomputation when the document changes, for example by tracking a version signal updated in the editor's onUpdate callback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/App.tsx` around lines 48 - 71, The memo for createMemo that
computes matches (matches) must also depend on editor document changes because
editor.state.doc isn't reactive; add a reactive document-version signal (e.g.,
docVersion()/setDocVersion) updated from the editor's onUpdate callback whenever
the document changes, then include docVersion() as a dependency inside the
createMemo along with editorInstance(), searchTerm(), and caseSensitive() so
matches recomputes on edits and produces fresh from/to positions; update
references to editor.state.doc usage within the matches computation to rely on
the current editor returned by editorInstance() and the latest docVersion
signal.
Comment on lines
+73
to
+91
| const highlightMatches = () => { | ||
| const editor = editorInstance(); | ||
| if (!editor) return; | ||
|
|
||
| clearHighlights(); | ||
|
|
||
| const allMatches = matches(); | ||
| if (allMatches.length === 0) return; | ||
|
|
||
| const { tr } = editor.state; | ||
| const highlightMark = editor.schema.marks.highlight; | ||
| if (!highlightMark) return; | ||
|
|
||
| for (const match of allMatches) { | ||
| tr.addMark(match.from, match.to, highlightMark.create()); | ||
| } | ||
|
|
||
| editor.view.dispatch(tr); | ||
| }; |
There was a problem hiding this comment.
Consider guarding against destroyed editor instances.
Per the context snippet, if the Editor component unmounts and remounts, the old editor is destroyed. Operations like highlightMatches and clearHighlights only check if (!editor) but don't verify the editor hasn't been destroyed. Dispatching to a destroyed editor's view could throw.
Consider adding a guard:
const highlightMatches = () => {
const editor = editorInstance();
- if (!editor) return;
+ if (!editor || editor.isDestroyed) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/App.tsx` around lines 73 - 91, highlightMatches currently only
checks for a falsy editor but can still attempt to use a destroyed editor
instance; update highlightMatches (and clearHighlights) to return early if the
editor has been destroyed or its view/dispatch method is missing: check a
reliable indicator such as a destroyed flag (e.g., editor.destroyed === true) or
verify editor.view and typeof editor.view.dispatch === "function" before using
editor.state, editor.schema, or calling editor.view.dispatch, and skip
processing when those guards fail so we never dispatch to a destroyed editor.
Comment on lines
+158
to
+159
| setCurrentMatchIndex(Math.min(idx, matchPositions.length - 2)); | ||
| highlightMatches(); |
There was a problem hiding this comment.
Potential negative index when replacing the last match.
When matchPositions.length is 1, Math.min(idx, matchPositions.length - 2) evaluates to Math.min(0, -1) = -1. This invalid index could cause issues in subsequent navigation or replace operations.
Proposed fix
- setCurrentMatchIndex(Math.min(idx, matchPositions.length - 2));
+ const newLength = matchPositions.length - 1;
+ setCurrentMatchIndex(newLength > 0 ? Math.min(idx, newLength - 1) : 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/App.tsx` around lines 158 - 159, The current
setCurrentMatchIndex(Math.min(idx, matchPositions.length - 2)) can produce -1
when matchPositions.length is 1; update the logic in the component where
setCurrentMatchIndex and highlightMatches are called to clamp the computed index
to the valid range (0 to matchPositions.length - 1) and handle the empty-array
case (when matchPositions.length === 0) by setting an appropriate sentinel
(e.g., 0 or null) before calling highlightMatches; reference the
setCurrentMatchIndex call, matchPositions array and the subsequent
highlightMatches() invocation when applying this clamp.
gameroman
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Editor.tsx`:
- Around line 43-60: createEffect that applies styles uses the plain variable
editor and runs before onMount, so it returns early and never re-runs to apply
initial settings; either apply the initial DOM updates inside onMount (use
onMount to query elementRef (".ProseMirror" and '[contenteditable="true"]') and
set fontSize/fontFamily/textAlign/spellcheck from props.settings) or convert
editor to a signal (createSignal) so the existing createEffect tracks editor
changes and re-runs when editor is set; update references in the effect to the
signal getter if you choose the signal approach.
- Around line 90-99: The placeholder CSS targets Tiptap's placeholder attributes
but the Placeholder extension isn't enabled in the Editor, so add the extension
import and configure it: import Placeholder from '@tiptap/extension-placeholder'
and include Placeholder({ placeholder: 'Type here…' }) in the editor's
extensions array (the Editor component / editor setup where extensions are
declared). Also remove the manually added data-placeholder attribute from the
static paragraph rendering (the place where you set data-placeholder on the
element) so the extension controls the attribute and the CSS will work as
intended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
43
to
60
| createEffect(() => { | ||
| if (!editorRef) return; | ||
| const selection = window.getSelection(); | ||
| let savedOffset = 0; | ||
| let hadSelection = false; | ||
|
|
||
| if (selection && selection.rangeCount > 0) { | ||
| const range = selection.getRangeAt(0); | ||
| const preCaretRange = range.cloneRange(); | ||
| preCaretRange.selectNodeContents(editorRef); | ||
| preCaretRange.setEnd(range.endContainer, range.endOffset); | ||
| savedOffset = preCaretRange.toString().length; | ||
| hadSelection = true; | ||
| } | ||
|
|
||
| const html = renderContent(); | ||
| if (html !== editorRef.innerHTML) { | ||
| editorRef.innerHTML = html; | ||
| if (!editor || !elementRef) return; | ||
| const { fontSize, fontFamily, textAlign, spellcheck } = props.settings; | ||
|
|
||
| const proseMirror = elementRef.querySelector(".ProseMirror") as HTMLElement; | ||
| if (proseMirror) { | ||
| proseMirror.style.fontSize = `${fontSize}px`; | ||
| proseMirror.style.fontFamily = fontFamily; | ||
| proseMirror.style.textAlign = textAlign; | ||
| } | ||
|
|
||
| if (document.activeElement === editorRef && hadSelection) { | ||
| setCaretPosition(editorRef, savedOffset); | ||
| const editableElement = elementRef.querySelector( | ||
| '[contenteditable="true"]', | ||
| ) as HTMLElement; | ||
| if (editableElement) { | ||
| editableElement.spellcheck = spellcheck; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Initial settings won't be applied due to execution order.
The createEffect runs before onMount, so on the first execution editor is undefined and the effect returns early. Since editor is a plain variable (not a signal), the effect won't re-run when the editor is assigned in onMount. Initial fontSize, fontFamily, textAlign, and spellcheck settings will only be applied if props.settings changes after mount.
🐛 Proposed fix: Apply initial styles in onMount
props.onEditorReady(editor);
+
+ // Apply initial settings
+ const proseMirror = elementRef.querySelector(".ProseMirror") as HTMLElement;
+ if (proseMirror) {
+ proseMirror.style.fontSize = `${props.settings.fontSize}px`;
+ proseMirror.style.fontFamily = props.settings.fontFamily;
+ proseMirror.style.textAlign = props.settings.textAlign;
+ }
+ const editableElement = elementRef.querySelector(
+ '[contenteditable="true"]',
+ ) as HTMLElement;
+ if (editableElement) {
+ editableElement.spellcheck = props.settings.spellcheck;
+ }
});
createEffect(() => {Alternatively, convert editor to a signal using createSignal so the effect properly tracks when the editor becomes available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Editor.tsx` around lines 43 - 60, createEffect that applies
styles uses the plain variable editor and runs before onMount, so it returns
early and never re-runs to apply initial settings; either apply the initial DOM
updates inside onMount (use onMount to query elementRef (".ProseMirror" and
'[contenteditable="true"]') and set fontSize/fontFamily/textAlign/spellcheck
from props.settings) or convert editor to a signal (createSignal) so the
existing createEffect tracks editor changes and re-runs when editor is set;
update references in the effect to the signal getter if you choose the signal
approach.
Comment on lines
+90
to
+99
| .ProseMirror { | ||
| min-height: 100%; | ||
| } | ||
| .ProseMirror p.is-editor-empty:first-child::before { | ||
| content: attr(data-placeholder); | ||
| float: left; | ||
| color: #999; | ||
| pointer-events: none; | ||
| height: 0; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if `@tiptap/extension-placeholder` is already a dependency
cat package.json | jq '.dependencies, .devDependencies' | grep -i placeholderRepository: rmandotdev/editor
Length of output: 43
🏁 Script executed:
cat -n src/components/Editor.tsx | head -120Repository: rmandotdev/editor
Length of output: 3798
Placeholder CSS won't work without the Placeholder extension.
The CSS at lines 93-99 targets .ProseMirror p.is-editor-empty:first-child::before using content: attr(data-placeholder), but Tiptap only adds the is-editor-empty class and data-placeholder attribute to paragraph elements when the @tiptap/extension-placeholder extension is configured. The extension is neither imported nor configured in the editor setup (lines 1-26), so the placeholder functionality will not work.
Proposed fix: Add Placeholder extension
Add the import:
import { Editor as TiptapEditor } from "@tiptap/core";
import Highlight from "@tiptap/extension-highlight";
+import Placeholder from "@tiptap/extension-placeholder";
import StarterKit from "@tiptap/starter-kit";Configure the extension:
extensions: [
StarterKit.configure({ heading: false, codeBlock: false, link: false }),
Highlight,
+ Placeholder.configure({
+ placeholder: "Start writing...",
+ }),
],Then remove the unused data-placeholder attribute from line 74.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Editor.tsx` around lines 90 - 99, The placeholder CSS targets
Tiptap's placeholder attributes but the Placeholder extension isn't enabled in
the Editor, so add the extension import and configure it: import Placeholder
from '@tiptap/extension-placeholder' and include Placeholder({ placeholder:
'Type here…' }) in the editor's extensions array (the Editor component / editor
setup where extensions are declared). Also remove the manually added
data-placeholder attribute from the static paragraph rendering (the place where
you set data-placeholder on the element) so the extension controls the attribute
and the CSS will work as intended.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #9
closes #10
Summary by CodeRabbit
New Features
Improvements
Chores
Documentation