fix(ui): stop destroying the comment editor mid-edit on refetch

[martian56]

Summary

Closes #138.

The inline comment editor destroyed its own TipTap instance whenever initialHtml changed. The [editor, initialHtml] effect's cleanup called editor.destroy(), so a comment-list refetch landing while a comment was being edited tore down the live editor and left a dead ProseMirror mounted: typing and formatting stopped working and the console filled with destroyed-instance errors.

Changes:

• Removed the manual editor.destroy(). useEditor already tears the instance down on unmount, so an initialHtml change no longer kills a live editor.
• Reseeding now happens only when the incoming HTML genuinely differs from what's shown (compared via a small normalize helper, matching DescriptionEditor), and uses { emitUpdate: false } so seeding no longer dirties the doc or fires onUpdate.
• The empty state is seeded from initialHtml at mount instead of via a mount-time effect, so the Send button is enabled immediately when editing existing content.

Testing

• npm run typecheck and npm run lint pass.
• Browser-verified end-to-end (React StrictMode on): posted a comment, clicked Edit, and the inline editor mounted seeded with the comment, contenteditable, Send enabled, and no console errors. Edited the text, saved, and the update persisted; the editor unmounted cleanly (ProseMirror instance count returned to baseline). Deleted the test comment afterward.

AI assistance

This change was produced with the help of Claude Code (Claude Opus 4.8). See the Co-Authored-By trailer on the commit.

Summary by CodeRabbit

• Bug Fixes
  • Improved comment editor empty-state handling so blank, whitespace-only, and visually empty paragraph content are treated consistently.
  • Prevented the editor from unnecessarily resetting when incoming content hasn’t actually changed, reducing content flicker and preserving the current draft more reliably.

[strix-security]

Strix Security Review

No security issues found.

_{Updated for 03b0d61.}

---

Reviewed by Strix
_{Re-run review · Configure security review settings}

[coderabbitai]

📝 Walkthrough

Walkthrough

CommentEditor.tsx now derives initial emptiness state from a normalized version of initialHtml and rewrites the reseeding effect to compare normalized HTML before resetting editor content, avoiding unnecessary resets and removing the editor.destroy() cleanup call.

Changes

Comment Editor Reseed Fix

Layer / File(s)	Summary
Normalize helper and reseed effect apps/web/src/components/work-item/CommentEditor.tsx	Adds normalize(html) to treat empty/whitespace/empty-paragraph HTML as equivalent, uses it to set initial isEmpty, and rewrites the initialHtml effect to only call setContent(..., { emitUpdate: false }) when normalized HTML differs, removing the editor.destroy() cleanup.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant IssueDetailPage
  participant CommentEditor
  participant TipTapEditor

  IssueDetailPage->>CommentEditor: initialHtml update (e.g. refetch)
  CommentEditor->>CommentEditor: normalize(initialHtml) vs normalize(editor.getHTML())
  alt content differs
    CommentEditor->>TipTapEditor: setContent(initialHtml, emitUpdate false)
    TipTapEditor-->>CommentEditor: updated HTML
    CommentEditor->>CommentEditor: recompute isEmpty
  else content same
    CommentEditor-->>IssueDetailPage: editor untouched, no destroy
  end

Loading

Poem

A rabbit typed a note mid-hop,
Refetch came— but edits didn't stop!
No more destroy, no more despair,
Just normalize and compare with care. 🐇✍️
Hooray, the editor lives to type another day!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is conventional, concise, and matches the main fix: preventing the comment editor from being destroyed during refetch.
Description check	✅ Passed	The PR description covers the bug, the fix, testing, and linked issue, and mostly follows the template despite some unchecked non-critical sections.
Linked Issues check	✅ Passed	The changes implement the linked bugfix: remove manual destroy, reseed only on real HTML changes, and use emitUpdate:false.
Out of Scope Changes check	✅ Passed	The added normalize helper and initial empty-state seeding support the stated fix and no unrelated changes are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/comment-editor-destroy

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/web/src/components/work-item/CommentEditor.tsx (1)

276-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse normalize in the other empty-checks to avoid divergent semantics.

normalize now maps <p><br></p> to empty, but onUpdate (Line 86) and handleSubmit (Line 109) still test only html === '<p></p>' || html === ''. So content of <p><br></p> is treated as empty for the initial isEmpty seed / reseed path yet non-empty by onUpdate (Send gets enabled) and would be submitted by handleSubmit. Routing all three through normalize keeps emptiness detection consistent.

♻️ Suggested consolidation (applies to unchanged Lines 84-87 and 106-113)

     onUpdate: ({ editor: ed }) => {
-      const html = ed.getHTML().trim();
-      setIsEmpty(html === '<p></p>' || html === '');
+      setIsEmpty(normalize(ed.getHTML()) === '');
     },

   const handleSubmit = () => {
     if (isSubmitting) return;
     const html = editor.getHTML().trim();
-    if (html === '<p></p>' || html === '') return;
+    if (normalize(html) === '') return;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/work-item/CommentEditor.tsx` around lines 276 - 281,
`normalize` now treats `<p><br></p>` as empty, but `CommentEditor` still uses
separate empty checks in `onUpdate` and `handleSubmit`, causing inconsistent
behavior. Update those paths to reuse `normalize` for determining emptiness so
the editor’s enabled state, reseed logic, and submit logic all agree on what
counts as empty; use the existing `normalize` helper in `CommentEditor` rather
than duplicating the HTML comparisons.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/web/src/components/work-item/CommentEditor.tsx`:
- Around line 276-281: `normalize` now treats `<p><br></p>` as empty, but
`CommentEditor` still uses separate empty checks in `onUpdate` and
`handleSubmit`, causing inconsistent behavior. Update those paths to reuse
`normalize` for determining emptiness so the editor’s enabled state, reseed
logic, and submit logic all agree on what counts as empty; use the existing
`normalize` helper in `CommentEditor` rather than duplicating the HTML
comparisons.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d92c4820-bcd7-4d05-ad92-28026bee7cf8

📥 Commits

Reviewing files that changed from the base of the PR and between c370080 and 03b0d61.

📒 Files selected for processing (1)

• apps/web/src/components/work-item/CommentEditor.tsx