Add Tiptap table support#3719
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes introduce markdown table support to the markdown editor component by configuring TableKit with styling options, implementing a clipboard-aware paste handler that detects and inserts markdown table content, and updating related CSS styling and TipTap extension configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR wires Tiptap's table extensions (
Confidence Score: 5/5Safe to merge; all findings are P2 suggestions with no blocking correctness issues. The extension wiring, CSS, slash-command, and schema-guard are all implemented correctly. The only notable issue — table controls being inaccessible without a text selection — is a UX gap but not a data-integrity or crash risk, making it a P2. All other findings are minor style suggestions. apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx — the shouldShow callback is worth revisiting to allow table control access without a selection
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx | Registers Table/TableRow/TableHeader/TableCell extensions before Markdown serializer; BubbleMenu shouldShow doesn't expose table controls without a text selection |
| apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsx | Adds TableDropdown with full column/row/header/delete operations, conditionally rendered via schema.nodes.table guard; "Insert table" always shows even when already in a table |
| apps/desktop/src/renderer/components/MarkdownEditor/components/SlashCommand/SlashCommand.tsx | Adds Table slash command inserting a 3x3 table with header row; fits cleanly into the blocks group |
| apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css | Adds .markdown-table styles for borders, radius, min-width, and selectedCell highlight; consistent with existing CSS variables |
| apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts | Source-file string matching tests verify extension wiring; fragile to quote-style or import-format changes |
| apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts | Adds cellMinWidth: 192 to the existing Table configuration; minor additive change, no issues |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User types slash in editor] --> B[SlashCommand popup]
B --> C{Select Table}
C --> D[insertTable rows3 cols3 withHeaderRow]
D --> E[Table node rendered]
F[User selects text in table cell] --> G[BubbleMenuToolbar shown]
G --> H{supportsTables}
H -- yes --> I[TableDropdown rendered]
I --> K{isTableActive}
K -- yes --> L[Add col/row, toggle header, delete]
K -- no --> M[Insert table only]
N[User clicks in table without selection] --> O[shouldShow returns false]
O --> P[BubbleMenu hidden - table controls inaccessible]
style P fill:#fca5a5,stroke:#ef4444
style D fill:#86efac,stroke:#22c55e
style L fill:#86efac,stroke:#22c55e
Comments Outside Diff (1)
-
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx, line 346-350 (link)Table controls unreachable without text selection
The
shouldShowcallback returnsfalsewheneverfrom === to(cursor placed without a selection). This means theTableDropdowninsideBubbleMenuToolbar—which is the only UI surface for adding/removing columns and rows—is completely inaccessible when a user just clicks into a table cell. To edit table structure, the user must first drag-select some text, then open the dropdown. Consider returningtruewhen the cursor is inside a table node even without a selection:shouldShow={({ editor: e, from, to }) => { if (e.isActive("codeBlock")) return false; if (e.isActive("table")) return true; if (from === to) return false; return true; }}Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx Line: 346-350 Comment: **Table controls unreachable without text selection** The `shouldShow` callback returns `false` whenever `from === to` (cursor placed without a selection). This means the `TableDropdown` inside `BubbleMenuToolbar`—which is the only UI surface for adding/removing columns and rows—is completely inaccessible when a user just clicks into a table cell. To edit table structure, the user must first drag-select some text, then open the dropdown. Consider returning `true` when the cursor is inside a table node even without a selection: ``` shouldShow={({ editor: e, from, to }) => { if (e.isActive("codeBlock")) return false; if (e.isActive("table")) return true; if (from === to) return false; return true; }} ``` How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
Line: 346-350
Comment:
**Table controls unreachable without text selection**
The `shouldShow` callback returns `false` whenever `from === to` (cursor placed without a selection). This means the `TableDropdown` inside `BubbleMenuToolbar`—which is the only UI surface for adding/removing columns and rows—is completely inaccessible when a user just clicks into a table cell. To edit table structure, the user must first drag-select some text, then open the dropdown. Consider returning `true` when the cursor is inside a table node even without a selection:
```
shouldShow={({ editor: e, from, to }) => {
if (e.isActive("codeBlock")) return false;
if (e.isActive("table")) return true;
if (from === to) return false;
return true;
}}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts
Line: 22-35
Comment:
**Source-text matching is fragile**
These tests read raw `.tsx` source files and call `toContain` on string literals (e.g., `'from "@tiptap/extension-table"'`). They will silently break on any quote-style change (`'` vs `"`), import re-ordering by a formatter, or a barrel re-export that hides the import from the individual file. If the goal is to guard against the table extensions being removed or reordered relative to `Markdown.configure`, a unit test that instantiates the actual Tiptap editor and inspects `editor.schema.nodes` (or extension order) would be more robust and would survive source formatting changes.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsx
Line: 331-348
Comment:
**"Insert table" always shown while already inside a table**
The "Insert table" button is always rendered as the first item, even when `isTableActive` is true. While Tiptap's `insertTable` command may silently reject the call inside an existing table, surfacing the button in that context is confusing alongside the structural-edit actions. Consider hiding (or disabling) "Insert table" when a table is already active.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Add Tiptap table support" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+22
to
+35
| test("registers Tiptap table extensions before markdown serialization", () => { | ||
| const source = readSource(join(MARKDOWN_EDITOR_DIR, "MarkdownEditor.tsx")); | ||
|
|
||
| expect(source).toContain('from "@tiptap/extension-table"'); | ||
| expect(source).toContain('from "@tiptap/extension-table-cell"'); | ||
| expect(source).toContain('from "@tiptap/extension-table-header"'); | ||
| expect(source).toContain('from "@tiptap/extension-table-row"'); | ||
| expect(source.indexOf("Table.configure")).toBeGreaterThan(-1); | ||
| expect(source.indexOf("Markdown.configure")).toBeGreaterThan( | ||
| source.indexOf("Table.configure"), | ||
| ); | ||
| }); | ||
|
|
||
| test("exposes table commands in the shared bubble toolbar", () => { |
There was a problem hiding this comment.
These tests read raw .tsx source files and call toContain on string literals (e.g., 'from "@tiptap/extension-table"'). They will silently break on any quote-style change (' vs "), import re-ordering by a formatter, or a barrel re-export that hides the import from the individual file. If the goal is to guard against the table extensions being removed or reordered relative to Markdown.configure, a unit test that instantiates the actual Tiptap editor and inspects editor.schema.nodes (or extension order) would be more robust and would survive source formatting changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts
Line: 22-35
Comment:
**Source-text matching is fragile**
These tests read raw `.tsx` source files and call `toContain` on string literals (e.g., `'from "@tiptap/extension-table"'`). They will silently break on any quote-style change (`'` vs `"`), import re-ordering by a formatter, or a barrel re-export that hides the import from the individual file. If the goal is to guard against the table extensions being removed or reordered relative to `Markdown.configure`, a unit test that instantiates the actual Tiptap editor and inspects `editor.schema.nodes` (or extension order) would be more robust and would survive source formatting changes.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+331
to
+348
| <button | ||
| type="button" | ||
| className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent" | ||
| onMouseDown={(e) => | ||
| runAction( | ||
| () => | ||
| editor | ||
| .chain() | ||
| .focus() | ||
| .insertTable({ rows: 3, cols: 3, withHeaderRow: true }) | ||
| .run(), | ||
| e, | ||
| ) | ||
| } | ||
| > | ||
| <RiTableLine className="size-4" /> | ||
| Insert table | ||
| </button> |
There was a problem hiding this comment.
The "Insert table" button is always rendered as the first item, even when isTableActive is true. While Tiptap's insertTable command may silently reject the call inside an existing table, surfacing the button in that context is confusing alongside the structural-edit actions. Consider hiding (or disabling) "Insert table" when a table is already active.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsx
Line: 331-348
Comment:
**"Insert table" always shown while already inside a table**
The "Insert table" button is always rendered as the first item, even when `isTableActive` is true. While Tiptap's `insertTable` command may silently reject the call inside an existing table, surfacing the button in that context is confusing alongside the structural-edit actions. Consider hiding (or disabling) "Insert table" when a table is already active.
How can I resolve this? If you propose a fix, please make it concise.
saddlepaddle
force-pushed
the
tiptap-table-support-2
branch
from
bfe63e9 to
e5b4562
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts (2)
7-15: Minor: preferimport.meta.dirover__dirnamein Bun test files.Bun supports
__dirnamefor compatibility, butimport.meta.diris the idiomatic ESM choice and avoids any future issues if the project's TS module setting changes. Trivial — feel free to skip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts` around lines 7 - 15, Replace the use of CommonJS __dirname with the ESM idiomatic import.meta.dir: change the declaration of MARKDOWN_EDITOR_DIR used by BUBBLE_MENU_TOOLBAR_PATH and SLASH_COMMAND_PATH from "const MARKDOWN_EDITOR_DIR = __dirname" to use import.meta.dir so MARKDOWN_EDITOR_DIR is derived from import.meta.dir while leaving the join calls and the BUBBLE_MENU_TOOLBAR_PATH and SLASH_COMMAND_PATH variables unchanged.
21-52: Source‑string assertions are brittle and don't catch behavioral regressions.These tests will break on innocuous refactors (e.g. moving
Table.configure({...})into a helper, switching to a default export, or renaming a class) and won't catch real issues like the keyboard‑shortcut conflict with table cell navigation. Consider replacing them with a behavior‑level test that mounts the actual extensions and asserts on the editor's schema/commands, e.g.:import { Editor } from "@tiptap/core"; // import the same extension list MarkdownEditor uses const editor = new Editor({ extensions: [...tableExtensions] }); expect(editor.schema.nodes.table).toBeDefined(); expect(editor.can().insertTable({ rows: 3, cols: 3, withHeaderRow: true })).toBe(true);That validates intent (table works) rather than implementation (the file contains a substring), and survives reorganization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts` around lines 21 - 52, Replace the brittle source-string assertions in the "MarkdownEditor table support" tests with a behavior-level test that mounts the actual Tiptap extensions and asserts on the editor schema and commands: import Editor from "@tiptap/core" and the same extension list used by MarkdownEditor (referencing the extension collection used in MarkdownEditor), create a new Editor({ extensions: [...] }), then assert editor.schema.nodes.table is defined and that editor.can().insertTable({ rows: 3, cols: 3, withHeaderRow: true }) (and other commands like addColumnAfter/addRowAfter/toggleHeaderRow/deleteTable) return true; remove the expect(...toContain(...)) checks that look for strings such as 'from "@tiptap/extension-table"' or "Table.configure" and the BUBBLE_MENU_TOOLBAR_PATH/SLASH_COMMAND_PATH substring assertions, and replace them with these runtime capability checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css`:
- Around line 89-95: The .markdown-table rule uses width: max-content which,
together with th/td { min-width: 12rem }, lets wide tables overflow the editor
because neither the table nor its ProseMirror parent enables horizontal
scrolling; change the table wrapper produced by the Table extension (or the
.markdown-table container) to allow horizontal scrolling (e.g., remove or avoid
width: max-content, ensure the wrapper or .ProseMirror has overflow-x: auto and
max-width: 100%, and keep the table itself max-width: none or display: block so
inner min-widths can scroll) so wide tables scroll horizontally instead of
overflowing the editor chrome.
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 248-265: The KeyboardHandler extension currently swallows
Tab/Shift-Tab and blocks the Table extension's navigation; update the
KeyboardHandler's key handling for "Tab" and "Shift-Tab" to detect when the
cursor is inside a table (e.g., selection is within a table/tableCell/tableRow
node or using editor.isActive('table')) and in that case return false so the
Table extension can handle goToNextCell / goToNextCell(-1); otherwise keep the
existing behavior. Locate the KeyboardHandler registration/handler and add this
conditional early in its Tab/Shift-Tab branches so the Table extension's
handlers run when inside tables.
In
`@apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsx`:
- Around line 329-348: The "Insert table" button should be hidden or disabled
when the current selection is already inside a table; update the
BubbleMenuToolbar component to check the existing isTableActive state before
rendering or enabling the insert-table button (the block that calls runAction(()
=> editor.chain().focus().insertTable({...}).run(), e)). Use the isTableActive
flag from the editor state (or the same helper used elsewhere in
BubbleMenuToolbar) to either skip rendering that button or render it with
disabled styling/attribute so it is not clickable when isTableActive is true.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts`:
- Around line 7-15: Replace the use of CommonJS __dirname with the ESM idiomatic
import.meta.dir: change the declaration of MARKDOWN_EDITOR_DIR used by
BUBBLE_MENU_TOOLBAR_PATH and SLASH_COMMAND_PATH from "const MARKDOWN_EDITOR_DIR
= __dirname" to use import.meta.dir so MARKDOWN_EDITOR_DIR is derived from
import.meta.dir while leaving the join calls and the BUBBLE_MENU_TOOLBAR_PATH
and SLASH_COMMAND_PATH variables unchanged.
- Around line 21-52: Replace the brittle source-string assertions in the
"MarkdownEditor table support" tests with a behavior-level test that mounts the
actual Tiptap extensions and asserts on the editor schema and commands: import
Editor from "@tiptap/core" and the same extension list used by MarkdownEditor
(referencing the extension collection used in MarkdownEditor), create a new
Editor({ extensions: [...] }), then assert editor.schema.nodes.table is defined
and that editor.can().insertTable({ rows: 3, cols: 3, withHeaderRow: true })
(and other commands like addColumnAfter/addRowAfter/toggleHeaderRow/deleteTable)
return true; remove the expect(...toContain(...)) checks that look for strings
such as 'from "@tiptap/extension-table"' or "Table.configure" and the
BUBBLE_MENU_TOOLBAR_PATH/SLASH_COMMAND_PATH substring assertions, and replace
them with these runtime capability checks.
🪄 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: ece85bd9-6213-4239-b36b-100c686f3fcd
📒 Files selected for processing (6)
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.tsapps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxapps/desktop/src/renderer/components/MarkdownEditor/components/SlashCommand/SlashCommand.tsxapps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.cssapps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsxapps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts
Comment on lines
+89
to
+95
| .markdown-table { | ||
| width: max-content; | ||
| min-width: 100%; | ||
| overflow: hidden; | ||
| border: 1px solid hsl(var(--border)); | ||
| border-radius: 0.375rem; | ||
| } |
Contributor
There was a problem hiding this comment.
Wide tables can overflow the editor with no horizontal scroll.
width: max-content combined with th/td { min-width: 12rem } (192px) means a 3‑column table needs ≥576px and a 4‑column table needs ≥768px just for the cell minimums. Since neither .markdown-table nor the parent .ProseMirror enables overflow-x: auto, anything wider than the editor pane will visually overflow into surrounding chrome (and overflow: hidden on the table only clips its own descendants, not its outer box). Consider wrapping tables in a horizontally scrollable container, e.g. setting the wrapper produced by the Table extension to overflow-x: auto:
♻️ Suggested CSS
.markdown-table {
- width: max-content;
- min-width: 100%;
- overflow: hidden;
- border: 1px solid hsl(var(--border));
- border-radius: 0.375rem;
+ width: max-content;
+ min-width: 100%;
+ border: 1px solid hsl(var(--border));
+ border-radius: 0.375rem;
+}
+
+/* Tiptap wraps tables in a div.tableWrapper */
+.ProseMirror .tableWrapper {
+ overflow-x: auto;
+ margin: 1rem 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .markdown-table { | |
| width: max-content; | |
| min-width: 100%; | |
| overflow: hidden; | |
| border: 1px solid hsl(var(--border)); | |
| border-radius: 0.375rem; | |
| } | |
| .markdown-table { | |
| width: max-content; | |
| min-width: 100%; | |
| border: 1px solid hsl(var(--border)); | |
| border-radius: 0.375rem; | |
| } | |
| /* Tiptap wraps tables in a div.tableWrapper */ | |
| .ProseMirror .tableWrapper { | |
| overflow-x: auto; | |
| margin: 1rem 0; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css`
around lines 89 - 95, The .markdown-table rule uses width: max-content which,
together with th/td { min-width: 12rem }, lets wide tables overflow the editor
because neither the table nor its ProseMirror parent enables horizontal
scrolling; change the table wrapper produced by the Table extension (or the
.markdown-table container) to allow horizontal scrolling (e.g., remove or avoid
width: max-content, ensure the wrapper or .ProseMirror has overflow-x: auto and
max-width: 100%, and keep the table itself max-width: none or display: block so
inner min-widths can scroll) so wide tables scroll horizontally instead of
overflowing the editor chrome.
Comment on lines
+329
to
+348
| {open && ( | ||
| <div className="absolute top-full right-0 mt-1 bg-popover text-popover-foreground border rounded-md shadow-md p-1 w-44 z-50"> | ||
| <button | ||
| type="button" | ||
| className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent" | ||
| onMouseDown={(e) => | ||
| runAction( | ||
| () => | ||
| editor | ||
| .chain() | ||
| .focus() | ||
| .insertTable({ rows: 3, cols: 3, withHeaderRow: true }) | ||
| .run(), | ||
| e, | ||
| ) | ||
| } | ||
| > | ||
| <RiTableLine className="size-4" /> | ||
| Insert table | ||
| </button> |
Contributor
There was a problem hiding this comment.
"Insert table" is shown even when the selection is already inside a table.
insertTable inside an existing table is a no‑op (prosemirror‑tables doesn't allow nesting) but the entry remains clickable, which is confusing alongside the row/column mutation actions. Hide it (or disable it) when isTableActive so the menu only offers actions that will actually run.
♻️ Proposed change
- <div className="absolute top-full right-0 mt-1 bg-popover text-popover-foreground border rounded-md shadow-md p-1 w-44 z-50">
- <button
- type="button"
- className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent"
- onMouseDown={(e) =>
- runAction(
- () =>
- editor
- .chain()
- .focus()
- .insertTable({ rows: 3, cols: 3, withHeaderRow: true })
- .run(),
- e,
- )
- }
- >
- <RiTableLine className="size-4" />
- Insert table
- </button>
-
- {isTableActive && (
- <>
- <div className="my-1 h-px bg-border" />
+ <div className="absolute top-full right-0 mt-1 bg-popover text-popover-foreground border rounded-md shadow-md p-1 w-44 z-50">
+ {!isTableActive && (
+ <button
+ type="button"
+ className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent"
+ onMouseDown={(e) =>
+ runAction(
+ () =>
+ editor
+ .chain()
+ .focus()
+ .insertTable({ rows: 3, cols: 3, withHeaderRow: true })
+ .run(),
+ e,
+ )
+ }
+ >
+ <RiTableLine className="size-4" />
+ Insert table
+ </button>
+ )}
+
+ {isTableActive && (
+ <>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {open && ( | |
| <div className="absolute top-full right-0 mt-1 bg-popover text-popover-foreground border rounded-md shadow-md p-1 w-44 z-50"> | |
| <button | |
| type="button" | |
| className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent" | |
| onMouseDown={(e) => | |
| runAction( | |
| () => | |
| editor | |
| .chain() | |
| .focus() | |
| .insertTable({ rows: 3, cols: 3, withHeaderRow: true }) | |
| .run(), | |
| e, | |
| ) | |
| } | |
| > | |
| <RiTableLine className="size-4" /> | |
| Insert table | |
| </button> | |
| {open && ( | |
| <div className="absolute top-full right-0 mt-1 bg-popover text-popover-foreground border rounded-md shadow-md p-1 w-44 z-50"> | |
| {!isTableActive && ( | |
| <button | |
| type="button" | |
| className="flex items-center gap-2 w-full px-2 py-1.5 text-sm rounded-sm hover:bg-accent" | |
| onMouseDown={(e) => | |
| runAction( | |
| () => | |
| editor | |
| .chain() | |
| .focus() | |
| .insertTable({ rows: 3, cols: 3, withHeaderRow: true }) | |
| .run(), | |
| e, | |
| ) | |
| } | |
| > | |
| <RiTableLine className="size-4" /> | |
| Insert table | |
| </button> | |
| )} | |
| {isTableActive && ( | |
| <> | |
| <div className="my-1 h-px bg-border" /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/BubbleMenuToolbar/BubbleMenuToolbar.tsx`
around lines 329 - 348, The "Insert table" button should be hidden or disabled
when the current selection is already inside a table; update the
BubbleMenuToolbar component to check the existing isTableActive state before
rendering or enabling the insert-table button (the block that calls runAction(()
=> editor.chain().focus().insertTable({...}).run(), e)). Use the isTableActive
flag from the editor state (or the same helper used elsewhere in
BubbleMenuToolbar) to either skip rendering that button or render it with
disabled styling/attribute so it is not clickable when isTableActive is true.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
saddlepaddle
force-pushed
the
tiptap-table-support-2
branch
from
e5b4562 to
6eaab88
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx (1)
125-130:⚠️ Potential issue | 🟡 MinorShift‑Tab at the first table cell will escape the editor.
editor.commands.goToPreviousCell()returnsfalsewhen there is no previous cell (first cell of the table). The handler then returnsfalse, letting the browser's default Tab focus navigation move focus out of the editor — inconsistent with Tab (which always consumes the event) and likely surprising while the caret is still inside a table.Consume the key in that case:
🔧 Suggested fix
"Shift-Tab": ({ editor }) => { - if (editor.isActive("table")) return editor.commands.goToPreviousCell(); + if (editor.isActive("table")) { + editor.commands.goToPreviousCell(); + return true; + } if (editor.commands.liftListItem("listItem")) return true; if (editor.commands.liftListItem("taskItem")) return true; return true; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx` around lines 125 - 130, The Shift-Tab handler currently calls editor.commands.goToPreviousCell() and returns its boolean, which allows the browser to move focus out when goToPreviousCell() returns false (first table cell); change the table branch in the "Shift-Tab" keymap in MarkdownEditor.tsx so that when editor.isActive("table") you call editor.commands.goToPreviousCell() but always return true (i.e., consume the event even if there is no previous cell); keep the existing liftListItem("listItem") and liftListItem("taskItem") branches as-is.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx (1)
250-269: TableKit configuration looks correct; double‑checkcellMinWidth: 192.Shape matches
TableKit.configure(sub‑optionstable/tableHeader/tableCell) in@tiptap/extension-table3.18.0, andresizable: false+ HTMLAttributes wiring is consistent with the CSS inmarkdown-table.One thing worth confirming:
cellMinWidthis in pixels, and 192 px is ~7.7× the library default (25). Combined withmin-w-fullon the table, this forces any table with more than a few columns to overflow horizontally on typical editor widths. If that's the intended "readable columns" behavior, ignore; otherwise consider a smaller floor (e.g., 80–120).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx` around lines 250 - 269, The TableKit.configure call sets cellMinWidth: 192 which is in pixels and significantly larger than the library default; if this causes unwanted horizontal overflow reduce the floor (e.g., to ~80–120) or make it configurable, otherwise leave as-is; locate the TableKit.configure block in MarkdownEditor.tsx (references: TableKit.configure, table.cellMinWidth) and change cellMinWidth to the smaller value or wire a prop/setting so the minimum cell width can be adjusted at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 125-130: The Shift-Tab handler currently calls
editor.commands.goToPreviousCell() and returns its boolean, which allows the
browser to move focus out when goToPreviousCell() returns false (first table
cell); change the table branch in the "Shift-Tab" keymap in MarkdownEditor.tsx
so that when editor.isActive("table") you call
editor.commands.goToPreviousCell() but always return true (i.e., consume the
event even if there is no previous cell); keep the existing
liftListItem("listItem") and liftListItem("taskItem") branches as-is.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx`:
- Around line 250-269: The TableKit.configure call sets cellMinWidth: 192 which
is in pixels and significantly larger than the library default; if this causes
unwanted horizontal overflow reduce the floor (e.g., to ~80–120) or make it
configurable, otherwise leave as-is; locate the TableKit.configure block in
MarkdownEditor.tsx (references: TableKit.configure, table.cellMinWidth) and
change cellMinWidth to the smaller value or wire a prop/setting so the minimum
cell width can be adjusted at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11dd2bfd-012c-4a85-955d-789d1fb77d92
📒 Files selected for processing (5)
apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.tsapps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxapps/desktop/src/renderer/components/MarkdownEditor/components/SlashCommand/SlashCommand.tsxapps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.cssapps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/components/MarkdownEditor/markdown-editor.css
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.test.ts
- apps/desktop/src/renderer/components/MarkdownEditor/components/SlashCommand/SlashCommand.tsx
- apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/createMarkdownExtensions.ts
saddlepaddle
force-pushed
the
tiptap-table-support-2
branch
from
6eaab88 to
5d524d6
Compare
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
12 tasks
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 25, 2026
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 25, 2026
upstream の 10 commits は #426 と #427 で fork 固有の差分を保ちながら個別に cherry-pick 取り込み済み。本 merge は ours strategy で **記録だけ** マージ済みに することで behind=0 を達成し、git 履歴上の追跡を正しくする。 Cherry-pick 取り込み済 (PR #426): - 5aab22a fix closed picker filters (superset-sh#3702) → cdb52f9 - 99db5be [codex] simplify workspace controls (superset-sh#3714) → f079606 - 186078a fix(chat): prevent ask_user question from shadowing sandbox access prompt (superset-sh#3662) → 09d6b57 - 47893c2 fix desktop workspace creation title clamp (superset-sh#3718) → 6a8c4ae - 09323ff Add diff pane file viewer action (superset-sh#3715) → 817ed8d - a5891c6 remove pending launch log (superset-sh#3721) → 0764d03 - c83de0c Add Tiptap table support (superset-sh#3719) → e67a885 - 486b621 [codex] Fix v2 terminal lifecycle after sleep (superset-sh#3711) → b71fbbb (+ #426 内 review fixups) Cherry-pick 取り込み済 (PR #427): - e07aef6 feat(desktop): play v2 notification hooks client-side (superset-sh#3675) → 27ac18a - eae6008 [codex] Port v2 terminal hotkeys to v1 (superset-sh#3724) → 05a77b8 (+ #427 内 Windows .ps1 v2 化) Fork 固有領域は変更ゼロで保持: 19 tRPC procedures (workspaces.githubExtended)、 AudioScheduler / Aivis TTS / notification-manager、terminal suggestion handler (新 terminalKeyboardHandler.ts に移植)、TERMINAL_OPTIONS、SUPERSET_WORKSPACE_NAME、 MainWindowEffects、INCEPTION_AUTH_PROVIDER_ID、v1MigrationState、TiptapPromptEditor、 electron-builder.ts (dmg.size="4g", fileAssociations)、Service Status Dashboard、 Linux daemon systemd、Worktree auto-sync、Windows support、DnD scratch route 他。
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.
Summary
Fixes SUPER-489
Tests
Summary by CodeRabbit