feat(files): inline rich markdown editor#5133
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview The new editor adds bubble formatting,
Smaller UX: breadcrumb path popover closes before navigate (no pointer-move re-open), Share icon → Send, files loading background tweak. Adds @tiptap/*, @floating-ui/dom, and extensive unit tests for round-trip and editor behavior. Reviewed by Cursor Bugbot for commit b7d87c8. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@greptile review |
|
@cursor review |
Greptile SummaryThis PR replaces the raw-markdown/preview split for
Confidence Score: 4/5Safe to merge with the round-trip fallback gap addressed; no data-loss or corruption path exists, but users with complex markdown lose edit access. The autosave hardening, content-reconciliation fix, and round-trip safety gate are all well-designed and correctly implemented. The one concrete regression is in file-viewer.tsx: the PR routes all markdown through RichMarkdownEditor, but files that fail the round-trip probe (footnotes, HTML entities, HTML comments, raw HTML blocks) open read-only with no fallback to Monaco. Users who previously edited those files in Monaco now have a read-only surface and no alternative editing path from the UI. apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx — the dispatch logic that routes all markdown to RichMarkdownEditor needs to additionally route round-trip-unsafe files to TextEditor (Monaco) when canEdit is true. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
FV[FileViewer] -->|isMarkdownFile?| MD{Markdown?}
MD -->|No| TE[TextEditor / Monaco]
MD -->|Yes| RME[RichMarkdownEditor]
RME --> UEFC[useEditableFileContent]
UEFC --> UA[useAutosave]
UEFC --> Fetch[useWorkspaceFileContent]
RME --> Loading{isContentLoading?}
Loading -->|Yes| LoadFrame[PreviewLoadingFrame]
Loading -->|No| Streaming{isStreamInteractionLocked?}
Streaming -->|Yes| PP[PreviewPanel read-only]
Streaming -->|No| LRME[LoadedRichMarkdownEditor]
LRME --> RTS{isRoundTripSafe?}
RTS -->|Yes + canEdit| Editable[TipTap editor — editable]
RTS -->|No or !canEdit| ReadOnly[TipTap editor — read-only no Monaco fallback]
Editable --> BM[EditorBubbleMenu]
Editable --> SC[SlashCommand]
Editable --> CB[CodeBlockWithLanguage]
Editable --> RI[ResizableImage]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
FV[FileViewer] -->|isMarkdownFile?| MD{Markdown?}
MD -->|No| TE[TextEditor / Monaco]
MD -->|Yes| RME[RichMarkdownEditor]
RME --> UEFC[useEditableFileContent]
UEFC --> UA[useAutosave]
UEFC --> Fetch[useWorkspaceFileContent]
RME --> Loading{isContentLoading?}
Loading -->|Yes| LoadFrame[PreviewLoadingFrame]
Loading -->|No| Streaming{isStreamInteractionLocked?}
Streaming -->|Yes| PP[PreviewPanel read-only]
Streaming -->|No| LRME[LoadedRichMarkdownEditor]
LRME --> RTS{isRoundTripSafe?}
RTS -->|Yes + canEdit| Editable[TipTap editor — editable]
RTS -->|No or !canEdit| ReadOnly[TipTap editor — read-only no Monaco fallback]
Editable --> BM[EditorBubbleMenu]
Editable --> SC[SlashCommand]
Editable --> CB[CodeBlockWithLanguage]
Editable --> RI[ResizableImage]
Reviews (7): Last reviewed commit: "feat(file-viewer): linked images, typed-..." | Re-trigger Greptile |
|
@cursor review |
|
@greptile review |
|
@cursor review |
|
@greptile review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2ca63c2. Configure here.
Replace the raw/preview split for markdown files with a Linear-style inline WYSIWYG editor (TipTap/ProseMirror): bubble + slash menus, code-block language picker with Prism highlighting and line-wrap, resizable images (HTML <img>), GFM tables, and frontmatter held byte-exact out of band. A round-trip preflight gate (decided once per open) falls back to the raw Monaco editor for any file that can't be edited losslessly, so the rich editor never silently corrupts a file.
The unmount flush no longer fires a concurrent PUT alongside an in-flight save; it awaits the in-flight save and then writes the latest content sequentially, so an out-of-order completion can't clobber newer edits with a stale snapshot (addresses Cursor Bugbot).
Some browsers expose a pasted or copied image only via DataTransfer.items (with an empty files list), so screenshot paste was silently ignored. extractImageFiles now falls back to items; moved to a testable module with unit tests (addresses Cursor Bugbot).
Wrap the probe serialize() in try/finally so the throwaway Editor is always destroyed even if setContent/getMarkdown throws (addresses Greptile). Adds a test proving PipeSafeTable escapes only interior cell pipes, not structural delimiters.
|
@greptile |
…rkflowActions router)
… a locked stale snapshot The gate locked isRoundTripSafe on the first post-stream snapshot, which is often the empty create_file buffer before the agent's server write lands — wrongly leaving an unsafe document editable. Derive the verdict from the current content (memoized on the bytes) so canEdit tracks the real payload.
|
@cursor review |
|
@greptile review |
…ever strand dirty edits The round-trip-safety verdict now gates editability only at open time — computed once, on the exact content the editor mounts with, and locked for its lifetime. A dirty document is round-trip-safe by construction (the editor only emits safe markdown), so the verdict must never flip off mid-edit: doing so disabled autosave, ⌘S, the toolbar Save and the unmount flush, stranding unsaved edits. Locking on the opened (reconciled) content also fixes the stale post-stream empty-buffer snapshot, and lets the redundant MarkdownFileEditor gate (plus its duplicate content fetch) be deleted.
|
@cursor review |
|
@greptile review |
- code-block: replace hand-rolled copy-with-timeout with shared useCopyToClipboard - rich-markdown-editor: compute frontmatter split once via lazy ref, drop redundant frontmatterRef - round-trip-safety: correct stale comments (read-only, not raw editor fallback)
|
@cursor review |
|
@greptile review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 89a269e. Configure here.
…der, churn fixes - image: round-trip linked images/badges via an href attr + custom markdown tokenizer; make the image a drag handle so it can be grabbed and reordered - link-input-rule: convert typed [text](url) to a link on the closing paren (normalized href) - markdown-paste: render pasted markdown as rich content, guarded against code blocks - round-trip-safety: behavioral link-count check replaces the static linked-image rejection - extensions: trim the table serializer's blank lines to stop interior-table whitespace churn
|
@cursor review |
|
@greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b7d87c8. Configure here.
| event.preventDefault() | ||
| const dropPos = view.posAtCoords({ left: event.clientX, top: event.clientY })?.pos | ||
| void insertImagesRef.current(images, dropPos ?? view.state.selection.from) | ||
| return true |
There was a problem hiding this comment.
Paste uploads when editor read-only
Medium Severity
The custom handlePaste and handleDrop handlers intercept image clipboard/drop events and call insertImagesRef (workspace upload) without checking whether the editor is editable. When canEdit is false or the round-trip gate sets editable: false, ProseMirror may still deliver paste/drop to these handlers, so uploads can run even though the document is read-only and the insert may not belong in the file.
Reviewed by Cursor Bugbot for commit b7d87c8. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return <CsvTablePreview key={file.id} file={file} workspaceId={workspaceId} /> | ||
| } | ||
|
|
||
| if (isMarkdownFile(file)) { | ||
| return ( | ||
| <RichMarkdownEditor | ||
| key={file.id} | ||
| file={file} | ||
| workspaceId={workspaceId} | ||
| canEdit={canEdit} | ||
| autoFocus={autoFocus} | ||
| onDirtyChange={onDirtyChange} | ||
| onSaveStatusChange={onSaveStatusChange} |
There was a problem hiding this comment.
The PR description states "falls back to the raw Monaco editor for any file that can't be edited losslessly," but the code routes all markdown files through RichMarkdownEditor. When isRoundTripSafe returns false (files with footnotes, HTML entities, HTML comments, raw HTML blocks, etc.), LoadedRichMarkdownEditor sets isEditable = false — opening a read-only rich-editor view instead of the previously working Monaco editor.
This regresses editability for any user whose markdown contains constructs matched by STABLE_LOSS_PATTERNS. Before this PR they had a fully editable Monaco surface; now they get a read-only rich editor with no edit path. A check of the round-trip verdict (or the isMarkdownFile decision) should dispatch to TextEditor (Monaco) for the unsafe case.
…to a paragraph Notion-style: ProseMirror's default joins or no-ops at a heading boundary, stranding the heading style. A second Backspace then merges as usual.
1 participant
Summary
/slash menu, code-block language picker with Prism syntax highlighting + line-wrap, resizable images (sized images serialize to HTML<img>), GFM tables, task lists<img>/entity/heading-hardbreak/table-<br>data-loss paths are all closed and gatedType of Change
Testing
Checklist