fix: text compose papercuts #270

2026-06-11T18:30:40Z

talksik commented

2026-06-11 18:30:40 +00:00

(Migrated from github.com)

Closes #267

Summary by CodeRabbit

New Features
- Fallback link-preview UI so links show actions even without metadata
- Clickable external-link action added to link thumbnails
- Image-block support in the markdown composer (uploads disabled) and improved editor autofocus behavior
- Composer now auto-switches to card mode for long text, attachments, or detected markdown; adaptive immersive text sizing
Bug Fixes
- Improved clipboard paste handling for file-only pastes
Style
- Enhanced markdown editor and message-card styling, scrollbars, and layout variables

Closes #267  ## Summary by CodeRabbit * **New Features** * Fallback link-preview UI so links show actions even without metadata * Clickable external-link action added to link thumbnails * Image-block support in the markdown composer (uploads disabled) and improved editor autofocus behavior * Composer now auto-switches to card mode for long text, attachments, or detected markdown; adaptive immersive text sizing * **Bug Fixes** * Improved clipboard paste handling for file-only pastes * **Style** * Enhanced markdown editor and message-card styling, scrollbars, and layout variables

coderabbitai[bot] commented

2026-06-11 18:30:53 +00:00

(Migrated from github.com)

📝 Walkthrough

Walkthrough

Refactors desktop compose: adds URL/markdown helpers, ensures link-preview fallbacks, enables Milkdown ImageBlock (uploads disabled), improves autofocus, replaces forceCardMode with session-latched carded logic, refines paste handling, and updates editor/card styling.

Changes

Compose Editor Improvements

Layer / File(s)	Summary
Utility helpers for URL and markdown detection `js/desktop/src/lib/link-metadata.ts`, `js/desktop/src/lib/markdown.ts`, `js/desktop/package.json`	New `domainFromUrl` extracts normalized hostnames; `hasMarkdownFormatting` detects markdown via regex. `@milkdown/kit` added (7.21.1).
Markdown editor and global styling `js/desktop/src/features/compose/markdown-editor.css`, `js/desktop/src/styles/globals.css`	Sets ProseMirror caret color, heading typography overrides, hides Milkdown image uploader UI, suppresses read-only image controls, and adds `.scrollbar-card` styling plus layout CSS variables.
Markdown editor ImageBlock & focus behavior `js/desktop/src/features/compose/markdown-editor.tsx`	Enables Milkdown `ImageBlock` (uploads rejected), constrains image block height, and changes autofocus to dispatch an editor action moving cursor to document end.
Text editor card-mode state refactoring `js/desktop/src/features/compose/text-editor.tsx`	Replaces `forceCardMode` with `carded` state; auto-switches to card mode when length, attachments, or markdown formatting indicate; updates shortcut and adds entry animations.
Link preview fallback UI and integration `js/desktop/src/components/link-preview-card.tsx`, `js/desktop/src/features/compose/attachment-strip.tsx`, `js/desktop/src/features/particles/text-particle-view.tsx`	Adds `LinkPreviewCardFallback` and uses it where metadata is missing so links remain visible and actionable (open/copy).
Paste event capture-phase handling `js/desktop/src/hooks/use-file-input.ts`	Registers paste listener in capture phase; intercepts only file-only pastes and calls stopPropagation() after preventing default before forwarding files.
Immersive text sizing helper `js/desktop/src/lib/immersive-text.ts`	Adds `getImmersiveTextStyle(length)` to centralize size/weight choices for immersive text display.

🎯 3 (Moderate) | ⏱️ ~25 minutes

"🐰
I hop through links when metadata's gone,
I pad the editor until the caret's dawn.
Paste gently, focus to the doc's last line,
Carded modes and fallbacks light the sign.
Compose is steady — nibble, copy, open, shine."

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix: text compose papercuts' is vague and generic, using the non-descriptive term 'papercuts' without clearly conveying what specific issues are being addressed in the compose functionality.	Consider a more specific title that mentions key fixes, such as 'fix: text editor padding, cursor placement, and link fallback rendering' or similar to better convey the scope of changes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The pull request successfully addresses all seven coding requirements from issue `#267`: padding fixes (CSS variables and styles), cursor placement at document end, heading size reduction, width consistency via shared styling variables, improved paste handling via file-only detection, and link metadata fallback rendering.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing text compose issues specified in issue `#267`. Supporting additions like the Milkdown library dependency and ImageBlock configuration are necessary to implement the required fixes and are not out of scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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-text-compose-papercuts

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

[![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/flowy-live/llink/pull/270?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)   <details> <summary>📝 Walkthrough</summary> ## Walkthrough Refactors desktop compose: adds URL/markdown helpers, ensures link-preview fallbacks, enables Milkdown ImageBlock (uploads disabled), improves autofocus, replaces forceCardMode with session-latched carded logic, refines paste handling, and updates editor/card styling. ## Changes **Compose Editor Improvements** |Layer / File(s)|Summary| |---|---| |**Utility helpers for URL and markdown detection** `js/desktop/src/lib/link-metadata.ts`, `js/desktop/src/lib/markdown.ts`, `js/desktop/package.json`|New `domainFromUrl` extracts normalized hostnames; `hasMarkdownFormatting` detects markdown via regex. `@milkdown/kit` added (7.21.1).| |**Markdown editor and global styling** `js/desktop/src/features/compose/markdown-editor.css`, `js/desktop/src/styles/globals.css`|Sets ProseMirror caret color, heading typography overrides, hides Milkdown image uploader UI, suppresses read-only image controls, and adds `.scrollbar-card` styling plus layout CSS variables.| |**Markdown editor ImageBlock & focus behavior** `js/desktop/src/features/compose/markdown-editor.tsx`|Enables Milkdown `ImageBlock` (uploads rejected), constrains image block height, and changes autofocus to dispatch an editor action moving cursor to document end.| |**Text editor card-mode state refactoring** `js/desktop/src/features/compose/text-editor.tsx`|Replaces `forceCardMode` with `carded` state; auto-switches to card mode when length, attachments, or markdown formatting indicate; updates shortcut and adds entry animations.| |**Link preview fallback UI and integration** `js/desktop/src/components/link-preview-card.tsx`, `js/desktop/src/features/compose/attachment-strip.tsx`, `js/desktop/src/features/particles/text-particle-view.tsx`|Adds `LinkPreviewCardFallback` and uses it where metadata is missing so links remain visible and actionable (open/copy).| |**Paste event capture-phase handling** `js/desktop/src/hooks/use-file-input.ts`|Registers paste listener in capture phase; intercepts only file-only pastes and calls stopPropagation() after preventing default before forwarding files.| |**Immersive text sizing helper** `js/desktop/src/lib/immersive-text.ts`|Adds `getImmersiveTextStyle(length)` to centralize size/weight choices for immersive text display.| 🎯 3 (Moderate) | ⏱️ ~25 minutes > "🐰 > I hop through links when metadata's gone, > I pad the editor until the caret's dawn. > Paste gently, focus to the doc's last line, > Carded modes and fallbacks light the sign. > Compose is steady — nibble, copy, open, shine." </details>   <details> <summary>🚥 Pre-merge checks | ✅ 3 | ❌ 2</summary> ### ❌ Failed checks (1 warning, 1 inconclusive) | Check name | Status | Explanation | Resolution | | :----------------: | :------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | Docstring Coverage | ⚠️ Warning | Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | | Title check | ❓ Inconclusive | The title 'fix: text compose papercuts' is vague and generic, using the non-descriptive term 'papercuts' without clearly conveying what specific issues are being addressed in the compose functionality. | Consider a more specific title that mentions key fixes, such as 'fix: text editor padding, cursor placement, and link fallback rendering' or similar to better convey the scope of changes. | <details> <summary>✅ Passed checks (3 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Linked Issues check | ✅ Passed | The pull request successfully addresses all seven coding requirements from issue `#267`: padding fixes (CSS variables and styles), cursor placement at document end, heading size reduction, width consistency via shared styling variables, improved paste handling via file-only detection, and link metadata fallback rendering. | | Out of Scope Changes check | ✅ Passed | All changes are directly related to fixing text compose issues specified in issue `#267`. Supporting additions like the Milkdown library dependency and ImageBlock configuration are necessary to implement the required fixes and are not out of scope. | </details> ✏️ Tip: You can configure your own custom pre-merge checks in the settings. </details>   <details> <summary>✨ Finishing Touches</summary> <details> <summary>📝 Generate docstrings</summary> - [ ]  Create stacked PR - [ ]  Commit on current branch </details> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Commit unit tests in branch `fix-text-compose-papercuts` </details> </details>   --- Comment `@coderabbitai help` to get the list of available commands and usage tips.

coderabbitai[bot] (Migrated from github.com) reviewed 2026-06-11 18:36:58 +00:00

coderabbitai[bot] (Migrated from github.com) left a 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)

js/desktop/src/features/compose/markdown-editor.tsx (1)

87-107: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add error handling for editor creation.

The crepe.create() promise has no .catch() handler. If editor initialization fails (e.g., due to invalid markdown, missing dependencies, or runtime errors), the rejection will be unhandled and could crash the component or leave it in an inconsistent state.

🛡️ Proposed fix to add error boundary

     crepe.create().then(() => {
       if (cancelled) {
         crepe.destroy();
         return;
       }
       created = crepe;
       if (autoFocus && !readOnly) {
         // Place the caret at the end of the document — the editor often mounts
         // mid-typing (immersive → card flip), where start-of-doc would strand
         // the user. Selection-only transactions don't echo markdownUpdated.
         crepe.editor.action((ctx) => {
           const view = ctx.get(editorViewCtx);
           view.dispatch(
             view.state.tr
               .setSelection(Selection.atEnd(view.state.doc))
               .scrollIntoView(),
           );
           view.focus();
         });
       }
-    });
+    }).catch((error) => {
+      console.error('Failed to create markdown editor:', error);
+      // Optionally: notify user or fallback to textarea
+    });

🤖 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 `@js/desktop/src/features/compose/markdown-editor.tsx` around lines 87 - 107,
Attach a .catch to the crepe.create() promise to handle initialization failures:
in the catch handler call crepe.destroy(), clear/leave unset the created
variable, perform any component cleanup (e.g., set cancelled or call the
existing cleanup path), log the error with a descriptive message (or call a
provided onError callback if available) so the rejection is not unhandled, and
avoid running the success-path logic (autoFocus/readOnly/crepe.editor.action)
when creation fails.

🧹 Nitpick comments (2)

js/desktop/src/lib/markdown.ts (1)
1-5: 💤 Low value

Consider detecting markdown links and images.

The regex currently detects headings, lists, code, bold, italic, and blockquotes, but doesn't detect markdown links [text](url) or images ![alt](url). These are common markdown patterns that might warrant switching to card mode.

Since this is a heuristic for UI mode switching, the current implementation is acceptable, but adding link/image detection would make it more comprehensive.
📝 Suggested enhancement
 export function hasMarkdownFormatting(content: string): boolean {
-  return /^#{1,6} |^\s*[-*+] |^\s*\d+\. |^```|`[^`]+`|\*\*|__|\*\S[^*]*\*|\b_\S[^_]*_\b|^>/m.test(
+  return /^#{1,6} |^\s*[-*+] |^\s*\d+\. |^```|`[^`]+`|\*\*|__|\*\S[^*]*\*|\b_\S[^_]*_\b|^>|!?\[.+?\]\(.+?\)/m.test(
     content,
   );
 }
The pattern !?\[.+?\]\(.+?\) matches both [text](url) and ![alt](url).
🤖 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 `@js/desktop/src/lib/markdown.ts` around lines 1 - 5, The regex in
hasMarkdownFormatting currently misses markdown links and images; update the
pattern returned by hasMarkdownFormatting to also match link/image syntax (e.g.
include a branch like !?\[.+?\]\(.+?\) in the alternation) so both [text](url)
and ![alt](url) trigger true; ensure the new subpattern is added to the existing
alternation in the returned RegExp (preserving the /m flag and other checks) so
the function continues to act as a comprehensive heuristic for switching UI
modes.
js/desktop/package.json (1)
67-67: ⚡ Quick win

Verify version pinning consistency across Milkdown packages.

The newly added @milkdown/kit is pinned to exact version 7.21.1, while the existing @milkdown/crepe on line 66 uses a caret range ^7.21.1.

For packages from the same monorepo, it's typically recommended to use consistent version ranges to ensure compatibility. Was the exact pinning intentional, or should both packages use the same versioning strategy?
🔧 Suggested fix for consistency
-    "`@milkdown/kit`": "7.21.1",
+    "`@milkdown/kit`": "^7.21.1",
🤖 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 `@js/desktop/package.json` at line 67, The package versioning is inconsistent:
`@milkdown/kit` is pinned to 7.21.1 while `@milkdown/crepe` uses ^7.21.1; update the
version spec so both Milkdown packages use the same range (e.g., change
"`@milkdown/kit`" to "^7.21.1" to match "`@milkdown/crepe`") and then
reinstall/update the lockfile (npm/yarn/pnpm) to ensure the dependency tree is
consistent.

🤖 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.

Inline comments:
In `@js/desktop/src/features/compose/text-editor.tsx`:
- Around line 67-72: The state update setCarded(true) must not run during
render; move the conditional that checks shouldCard (computed from textContent,
IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) into a useEffect
that watches the dependencies (textContent, hasEnrichments, carded or
shouldCard) and calls setCarded(true) when shouldCard becomes true and carded is
false; keep the existing immersive = !carded logic in render but ensure all
state mutations happen inside that useEffect to avoid React render-phase
updates.

---

Outside diff comments:
In `@js/desktop/src/features/compose/markdown-editor.tsx`:
- Around line 87-107: Attach a .catch to the crepe.create() promise to handle
initialization failures: in the catch handler call crepe.destroy(), clear/leave
unset the created variable, perform any component cleanup (e.g., set cancelled
or call the existing cleanup path), log the error with a descriptive message (or
call a provided onError callback if available) so the rejection is not
unhandled, and avoid running the success-path logic
(autoFocus/readOnly/crepe.editor.action) when creation fails.

---

Nitpick comments:
In `@js/desktop/package.json`:
- Line 67: The package versioning is inconsistent: `@milkdown/kit` is pinned to
7.21.1 while `@milkdown/crepe` uses ^7.21.1; update the version spec so both
Milkdown packages use the same range (e.g., change "`@milkdown/kit`" to "^7.21.1"
to match "`@milkdown/crepe`") and then reinstall/update the lockfile
(npm/yarn/pnpm) to ensure the dependency tree is consistent.

In `@js/desktop/src/lib/markdown.ts`:
- Around line 1-5: The regex in hasMarkdownFormatting currently misses markdown
links and images; update the pattern returned by hasMarkdownFormatting to also
match link/image syntax (e.g. include a branch like !?\[.+?\]\(.+?\) in the
alternation) so both [text](url) and ![alt](url) trigger true; ensure the new
subpattern is added to the existing alternation in the returned RegExp
(preserving the /m flag and other checks) so the function continues to act as a
comprehensive heuristic for switching UI modes.

🪄 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 Plus

Run ID: c025be3d-4a0c-46a6-8ada-baf969e9803b

📥 Commits

Reviewing files that changed from the base of the PR and between 66d53497c5 and 5b199434bf.

📒 Files selected for processing (10)

js/desktop/package.json
js/desktop/src/components/link-preview-card.tsx
js/desktop/src/features/compose/attachment-strip.tsx
js/desktop/src/features/compose/markdown-editor.css
js/desktop/src/features/compose/markdown-editor.tsx
js/desktop/src/features/compose/text-editor.tsx
js/desktop/src/features/particles/text-particle-view.tsx
js/desktop/src/hooks/use-file-input.ts
js/desktop/src/lib/link-metadata.ts
js/desktop/src/lib/markdown.ts

**Actionable comments posted: 1** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>js/desktop/src/features/compose/markdown-editor.tsx (1)</summary><blockquote> > > `87-107`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ > > **Add error handling for editor creation.** > > The `crepe.create()` promise has no `.catch()` handler. If editor initialization fails (e.g., due to invalid markdown, missing dependencies, or runtime errors), the rejection will be unhandled and could crash the component or leave it in an inconsistent state. > > > > > > <details> > <summary>🛡️ Proposed fix to add error boundary</summary> > > ```diff > crepe.create().then(() => { > if (cancelled) { > crepe.destroy(); > return; > } > created = crepe; > if (autoFocus && !readOnly) { > // Place the caret at the end of the document — the editor often mounts > // mid-typing (immersive → card flip), where start-of-doc would strand > // the user. Selection-only transactions don't echo markdownUpdated. > crepe.editor.action((ctx) => { > const view = ctx.get(editorViewCtx); > view.dispatch( > view.state.tr > .setSelection(Selection.atEnd(view.state.doc)) > .scrollIntoView(), > ); > view.focus(); > }); > } > - }); > + }).catch((error) => { > + console.error('Failed to create markdown editor:', error); > + // Optionally: notify user or fallback to textarea > + }); > ``` > </details> > > <details> > <summary>🤖 Prompt for AI Agents</summary> > > ``` > Verify each finding against current code. Fix only still-valid issues, skip the > rest with a brief reason, keep changes minimal, and validate. > > In `@js/desktop/src/features/compose/markdown-editor.tsx` around lines 87 - 107, > Attach a .catch to the crepe.create() promise to handle initialization failures: > in the catch handler call crepe.destroy(), clear/leave unset the created > variable, perform any component cleanup (e.g., set cancelled or call the > existing cleanup path), log the error with a descriptive message (or call a > provided onError callback if available) so the rejection is not unhandled, and > avoid running the success-path logic (autoFocus/readOnly/crepe.editor.action) > when creation fails. > ``` > > </details> > >  > > </blockquote></details> > > </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>js/desktop/src/lib/markdown.ts (1)</summary><blockquote> `1-5`: _💤 Low value_ **Consider detecting markdown links and images.** The regex currently detects headings, lists, code, bold, italic, and blockquotes, but doesn't detect markdown links `[text](url)` or images `![alt](url)`. These are common markdown patterns that might warrant switching to card mode. Since this is a heuristic for UI mode switching, the current implementation is acceptable, but adding link/image detection would make it more comprehensive. <details> <summary>📝 Suggested enhancement</summary> ```diff export function hasMarkdownFormatting(content: string): boolean { - return /^#{1,6} |^\s*[-*+] |^\s*\d+\. |^```|`[^`]+`|\*\*|__|\*\S[^*]*\*|\b_\S[^_]*_\b|^>/m.test( + return /^#{1,6} |^\s*[-*+] |^\s*\d+\. |^```|`[^`]+`|\*\*|__|\*\S[^*]*\*|\b_\S[^_]*_\b|^>|!?\[.+?\]\(.+?\)/m.test( content, ); } ``` The pattern `!?\[.+?\]\(.+?\)` matches both `[text](url)` and `![alt](url)`. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/lib/markdown.ts` around lines 1 - 5, The regex in hasMarkdownFormatting currently misses markdown links and images; update the pattern returned by hasMarkdownFormatting to also match link/image syntax (e.g. include a branch like !?\[.+?\]\(.+?\) in the alternation) so both [text](url) and ![alt](url) trigger true; ensure the new subpattern is added to the existing alternation in the returned RegExp (preserving the /m flag and other checks) so the function continues to act as a comprehensive heuristic for switching UI modes. ``` </details>  </blockquote></details> <details> <summary>js/desktop/package.json (1)</summary><blockquote> `67-67`: _⚡ Quick win_ **Verify version pinning consistency across Milkdown packages.** The newly added `@milkdown/kit` is pinned to exact version `7.21.1`, while the existing `@milkdown/crepe` on line 66 uses a caret range `^7.21.1`. For packages from the same monorepo, it's typically recommended to use consistent version ranges to ensure compatibility. Was the exact pinning intentional, or should both packages use the same versioning strategy? <details> <summary>🔧 Suggested fix for consistency</summary> ```diff - "`@milkdown/kit`": "7.21.1", + "`@milkdown/kit`": "^7.21.1", ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/package.json` at line 67, The package versioning is inconsistent: `@milkdown/kit` is pinned to 7.21.1 while `@milkdown/crepe` uses ^7.21.1; update the version spec so both Milkdown packages use the same range (e.g., change "`@milkdown/kit`" to "^7.21.1" to match "`@milkdown/crepe`") and then reinstall/update the lockfile (npm/yarn/pnpm) to ensure the dependency tree is consistent. ``` </details>  </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@js/desktop/src/features/compose/text-editor.tsx`: - Around line 67-72: The state update setCarded(true) must not run during render; move the conditional that checks shouldCard (computed from textContent, IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) into a useEffect that watches the dependencies (textContent, hasEnrichments, carded or shouldCard) and calls setCarded(true) when shouldCard becomes true and carded is false; keep the existing immersive = !carded logic in render but ensure all state mutations happen inside that useEffect to avoid React render-phase updates. --- Outside diff comments: In `@js/desktop/src/features/compose/markdown-editor.tsx`: - Around line 87-107: Attach a .catch to the crepe.create() promise to handle initialization failures: in the catch handler call crepe.destroy(), clear/leave unset the created variable, perform any component cleanup (e.g., set cancelled or call the existing cleanup path), log the error with a descriptive message (or call a provided onError callback if available) so the rejection is not unhandled, and avoid running the success-path logic (autoFocus/readOnly/crepe.editor.action) when creation fails. --- Nitpick comments: In `@js/desktop/package.json`: - Line 67: The package versioning is inconsistent: `@milkdown/kit` is pinned to 7.21.1 while `@milkdown/crepe` uses ^7.21.1; update the version spec so both Milkdown packages use the same range (e.g., change "`@milkdown/kit`" to "^7.21.1" to match "`@milkdown/crepe`") and then reinstall/update the lockfile (npm/yarn/pnpm) to ensure the dependency tree is consistent. In `@js/desktop/src/lib/markdown.ts`: - Around line 1-5: The regex in hasMarkdownFormatting currently misses markdown links and images; update the pattern returned by hasMarkdownFormatting to also match link/image syntax (e.g. include a branch like !?\[.+?\]\(.+?\) in the alternation) so both [text](url) and ![alt](url) trigger true; ensure the new subpattern is added to the existing alternation in the returned RegExp (preserving the /m flag and other checks) so the function continues to act as a comprehensive heuristic for switching UI modes. ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ]  Push a commit to this branch (recommended) - [ ]  Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `c025be3d-4a0c-46a6-8ada-baf969e9803b` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 66d53497c5c0c67cb162e1f61a66ee449875d973 and 5b199434bfc0d26413e27ad1837cc14e659ca054. </details> <details> <summary>📒 Files selected for processing (10)</summary> * `js/desktop/package.json` * `js/desktop/src/components/link-preview-card.tsx` * `js/desktop/src/features/compose/attachment-strip.tsx` * `js/desktop/src/features/compose/markdown-editor.css` * `js/desktop/src/features/compose/markdown-editor.tsx` * `js/desktop/src/features/compose/text-editor.tsx` * `js/desktop/src/features/particles/text-particle-view.tsx` * `js/desktop/src/hooks/use-file-input.ts` * `js/desktop/src/lib/link-metadata.ts` * `js/desktop/src/lib/markdown.ts` </details> </details>

js/desktop/src/features/compose/text-editor.tsx

coderabbitai[bot] (Migrated from github.com) commented

2026-06-11 18:36:57 +00:00

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix setState during render — causes React warnings and potential bugs.

The pattern if (shouldCard && !carded) setCarded(true); calls a state setter directly during the render phase, which violates React's rules and will trigger warnings like "Cannot update a component from inside the function body of a different component."

This must be moved into a useEffect to update state safely after render.

🔒 Required fix

  const hasEnrichments = attachmentCount > 0 || linkPreviews.length > 0;
  const shouldCard =
    textContent.length >= IMMERSIVE_CHAR_LIMIT ||
    hasEnrichments ||
    hasMarkdownFormatting(textContent);
-  if (shouldCard && !carded) setCarded(true);
  const immersive = !carded;
+
+  useEffect(() => {
+    if (shouldCard && !carded) {
+      setCarded(true);
+    }
+  }, [shouldCard, carded]);

🤖 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 `@js/desktop/src/features/compose/text-editor.tsx` around lines 67 - 72, The
state update setCarded(true) must not run during render; move the conditional
that checks shouldCard (computed from textContent, IMMERSIVE_CHAR_LIMIT,
hasEnrichments, hasMarkdownFormatting) into a useEffect that watches the
dependencies (textContent, hasEnrichments, carded or shouldCard) and calls
setCarded(true) when shouldCard becomes true and carded is false; keep the
existing immersive = !carded logic in render but ensure all state mutations
happen inside that useEffect to avoid React render-phase updates.

_⚠️ Potential issue_ | _🔴 Critical_ | _⚡ Quick win_ **Fix setState during render — causes React warnings and potential bugs.** The pattern `if (shouldCard && !carded) setCarded(true);` calls a state setter directly during the render phase, which violates React's rules and will trigger warnings like "Cannot update a component from inside the function body of a different component." This must be moved into a `useEffect` to update state safely after render. <details> <summary>🔒 Required fix</summary> ```diff const hasEnrichments = attachmentCount > 0 || linkPreviews.length > 0; const shouldCard = textContent.length >= IMMERSIVE_CHAR_LIMIT || hasEnrichments || hasMarkdownFormatting(textContent); - if (shouldCard && !carded) setCarded(true); const immersive = !carded; + + useEffect(() => { + if (shouldCard && !carded) { + setCarded(true); + } + }, [shouldCard, carded]); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/features/compose/text-editor.tsx` around lines 67 - 72, The state update setCarded(true) must not run during render; move the conditional that checks shouldCard (computed from textContent, IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) into a useEffect that watches the dependencies (textContent, hasEnrichments, carded or shouldCard) and calls setCarded(true) when shouldCard becomes true and carded is false; keep the existing immersive = !carded logic in render but ensure all state mutations happen inside that useEffect to avoid React render-phase updates. ``` </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2026-06-11 19:08:06 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

♻️ Duplicate comments (1)

js/desktop/src/features/compose/text-editor.tsx (1)
62-67: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

setState during render still present — must use useEffect.

The pattern if (shouldCard) setCarded(true); calls a state setter directly during the render phase, which violates React's core rule: "Never call set functions during rendering." While the !carded guard prevents infinite loops, this still triggers warnings and can cause issues with React 19's concurrent rendering and batching.

This is the same issue flagged in the previous review. Move the state update into a useEffect as originally recommended:
🔒 Required fix
  const hasEnrichments = attachmentCount > 0 || linkPreviews.length > 0;
  const shouldCard =
-   !carded &&
-   (textContent.length >= IMMERSIVE_CHAR_LIMIT ||
-     hasEnrichments ||
-     hasMarkdownFormatting(textContent));
- if (shouldCard) setCarded(true);
+   textContent.length >= IMMERSIVE_CHAR_LIMIT ||
+   hasEnrichments ||
+   hasMarkdownFormatting(textContent);
  const immersive = !carded;
+
+ useEffect(() => {
+   if (shouldCard && !carded) {
+     setCarded(true);
+   }
+ }, [shouldCard, carded]);
🤖 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 `@js/desktop/src/features/compose/text-editor.tsx` around lines 62 - 67, The
check that computes shouldCard (using textContent, IMMERSIVE_CHAR_LIMIT,
hasEnrichments, hasMarkdownFormatting) must not call setCarded during render;
move the side-effect into a useEffect that watches the same dependencies plus
carded and calls setCarded(true) when shouldCard is true (and optionally
setCarded(false) when it becomes false, if desired). Replace the inline if
(shouldCard) setCarded(true) with a useEffect that computes shouldCard inside it
and invokes setCarded accordingly to avoid calling the state setter during
rendering.

🤖 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.

Duplicate comments:
In `@js/desktop/src/features/compose/text-editor.tsx`:
- Around line 62-67: The check that computes shouldCard (using textContent,
IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) must not call
setCarded during render; move the side-effect into a useEffect that watches the
same dependencies plus carded and calls setCarded(true) when shouldCard is true
(and optionally setCarded(false) when it becomes false, if desired). Replace the
inline if (shouldCard) setCarded(true) with a useEffect that computes shouldCard
inside it and invokes setCarded accordingly to avoid calling the state setter
during rendering.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 705610ec-394f-4e6b-922b-c8eb76b6c858

📥 Commits

Reviewing files that changed from the base of the PR and between 5b199434bf and e821cd02ef.

📒 Files selected for processing (9)

js/desktop/src/components/link-preview-card.tsx
js/desktop/src/features/compose/attachment-strip.tsx
js/desktop/src/features/compose/markdown-editor.css
js/desktop/src/features/compose/markdown-editor.tsx
js/desktop/src/features/compose/text-editor.tsx
js/desktop/src/features/particles/text-particle-view.tsx
js/desktop/src/hooks/use-file-input.ts
js/desktop/src/lib/immersive-text.ts
js/desktop/src/styles/globals.css

🚧 Files skipped from review as they are similar to previous changes (4)

js/desktop/src/hooks/use-file-input.ts
js/desktop/src/features/compose/attachment-strip.tsx
js/desktop/src/features/compose/markdown-editor.tsx
js/desktop/src/features/particles/text-particle-view.tsx

<details> <summary>♻️ Duplicate comments (1)</summary><blockquote> <details> <summary>js/desktop/src/features/compose/text-editor.tsx (1)</summary><blockquote> `62-67`: _⚠️ Potential issue_ | _🔴 Critical_ | _⚡ Quick win_ **setState during render still present — must use useEffect.** The pattern `if (shouldCard) setCarded(true);` calls a state setter directly during the render phase, which violates React's core rule: "Never call set functions during rendering." While the `!carded` guard prevents infinite loops, this still triggers warnings and can cause issues with React 19's concurrent rendering and batching. This is the same issue flagged in the previous review. Move the state update into a `useEffect` as originally recommended: <details> <summary>🔒 Required fix</summary> ```diff const hasEnrichments = attachmentCount > 0 || linkPreviews.length > 0; const shouldCard = - !carded && - (textContent.length >= IMMERSIVE_CHAR_LIMIT || - hasEnrichments || - hasMarkdownFormatting(textContent)); - if (shouldCard) setCarded(true); + textContent.length >= IMMERSIVE_CHAR_LIMIT || + hasEnrichments || + hasMarkdownFormatting(textContent); const immersive = !carded; + + useEffect(() => { + if (shouldCard && !carded) { + setCarded(true); + } + }, [shouldCard, carded]); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/desktop/src/features/compose/text-editor.tsx` around lines 62 - 67, The check that computes shouldCard (using textContent, IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) must not call setCarded during render; move the side-effect into a useEffect that watches the same dependencies plus carded and calls setCarded(true) when shouldCard is true (and optionally setCarded(false) when it becomes false, if desired). Replace the inline if (shouldCard) setCarded(true) with a useEffect that computes shouldCard inside it and invokes setCarded accordingly to avoid calling the state setter during rendering. ``` </details>  </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Duplicate comments: In `@js/desktop/src/features/compose/text-editor.tsx`: - Around line 62-67: The check that computes shouldCard (using textContent, IMMERSIVE_CHAR_LIMIT, hasEnrichments, hasMarkdownFormatting) must not call setCarded during render; move the side-effect into a useEffect that watches the same dependencies plus carded and calls setCarded(true) when shouldCard is true (and optionally setCarded(false) when it becomes false, if desired). Replace the inline if (shouldCard) setCarded(true) with a useEffect that computes shouldCard inside it and invokes setCarded accordingly to avoid calling the state setter during rendering. ``` </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `705610ec-394f-4e6b-922b-c8eb76b6c858` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5b199434bfc0d26413e27ad1837cc14e659ca054 and e821cd02ef994f38079134f28b80d2324c0ac6d7. </details> <details> <summary>📒 Files selected for processing (9)</summary> * `js/desktop/src/components/link-preview-card.tsx` * `js/desktop/src/features/compose/attachment-strip.tsx` * `js/desktop/src/features/compose/markdown-editor.css` * `js/desktop/src/features/compose/markdown-editor.tsx` * `js/desktop/src/features/compose/text-editor.tsx` * `js/desktop/src/features/particles/text-particle-view.tsx` * `js/desktop/src/hooks/use-file-input.ts` * `js/desktop/src/lib/immersive-text.ts` * `js/desktop/src/styles/globals.css` </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (4)</summary> * js/desktop/src/hooks/use-file-input.ts * js/desktop/src/features/compose/attachment-strip.tsx * js/desktop/src/features/compose/markdown-editor.tsx * js/desktop/src/features/particles/text-particle-view.tsx </details> </details>

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: FlowyLabs/llink#270