[WIKI-498] [WIKI-567] feat: ability to rearrange columns and rows in table#7624
[WIKI-498] [WIKI-567] feat: ability to rearrange columns and rows in table#7624sriramveeraghanta merged 7 commits intopreviewfrom
Conversation
WalkthroughAdds comprehensive table drag-and-drop: new row/column drag-handle UI, plugins, DOM markers, preview utilities, move/duplicate actions, color selector, hideContent cell/header attribute and CSS, helpers for table geometry/selection, clearSelectedCells command, and removes legacy tableControls plugin. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Handle as Column/Row Drag Handle (React)
participant Plugin as Drag-Handle Plugin
participant Utils as DnD Utils & Markers
participant Actions as Table Actions
participant PM as ProseMirror Transaction
participant View as EditorView
User->>Handle: mousedown (start drag)
Handle->>Plugin: locate table & selection
Handle->>Utils: create markers + construct preview
loop mousemove
Handle->>Utils: measure DOM, compute drop index
Utils-->>Handle: dropIndex, preview layout
Handle->>Utils: update drag & drop markers
end
User-->>Handle: mouseup (finish)
alt order changed
Handle->>Actions: moveSelectedRows/Columns(...)
Actions->>PM: build transaction
PM->>View: dispatch(tr)
else no change
Handle->>Utils: hide markers/cleanup
end
sequenceDiagram
autonumber
actor User
participant Handle as Handle Dropdown
participant UI as Color Selector
participant Editor as TipTap Editor
User->>Handle: open dropdown
Handle->>UI: render color swatches
User->>UI: select/erase color
UI->>Editor: chain().focus().updateAttributes(TABLE_CELL, { background / null })
Editor-->>User: cell background updated
UI->>Handle: onSelect/onClose
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
| // states | ||
| const [isDropdownOpen, setIsDropdownOpen] = useState(false); | ||
| // floating ui | ||
| const { refs, floatingStyles, context } = useFloating({ |
There was a problem hiding this comment.
move floating ui logic to different file.
iam-vipin
left a comment
iam-vipin
left a comment
There was a problem hiding this comment.
move floating ui changes in new file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (1)
278-286: Off-by-one in TableRect boundaries (bottom/right) will skip the last row/columnTableRect.right and .bottom are exclusive bounds in prosemirror-tables. Using width - 1 / height - 1 will cause add/remove operations to miss the last row/column. This can lead to malformed tables (e.g., last row not updated on addColumn).
Fix: set bottom to tableMapData.height and right to tableMapData.width in all rect constructions (insert and remove).
const rect: TableRect = { map: tableMapData, tableStart: tablePos, table: tableNode, top: 0, left: 0, - bottom: tableMapData.height - 1, - right: tableMapData.width - 1, + bottom: tableMapData.height, + right: tableMapData.width, };const rect = { map: tableMapData, tableStart: tablePos, table: tableNode, top: 0, left: 0, - bottom: tableMapData.height - 1, - right: tableMapData.width - 1, + bottom: tableMapData.height, + right: tableMapData.width, };Also applies to: 350-358, 310-318, 382-390
packages/editor/src/core/plugins/drag-handle.ts (1)
148-175: Animation frame bookkeeping: remove confusing type-cast to null and dead check
- getScrollParent no longer returns null; the subsequent null-guard is dead.
- Casting requestAnimationFrame(...) as unknown as null is misleading; it still returns a number at runtime. Keep the numeric id and cancel it normally.
- const scrollableParent = getScrollParent(dragHandleElement!); - if (!scrollableParent) return; + const scrollableParent = getScrollParent(dragHandleElement!); ... - scrollAnimationFrame = requestAnimationFrame(scroll) as unknown as null; + scrollAnimationFrame = requestAnimationFrame(scroll);
🧹 Nitpick comments (38)
packages/editor/src/core/constants/meta.ts (1)
1-5: Document semantics and expected value types for new meta keysPlease add a short JSDoc above CORE_EDITOR_META explaining:
- Typical value types (e.g., ADD_TO_HISTORY expects boolean, INTENTIONAL_DELETION likely boolean).
- Which subsystems read them (history plugin, table-drag-handles), and whether they are consumed or just advisory.
This will help prevent accidental misuse in future contributors’ code.
export enum CORE_EDITOR_META { SKIP_FILE_DELETION = "skipFileDeletion", + /** + * Indicates a user-initiated delete action (not programmatic cleanup). + * Value: boolean + * Readers: commands/plugins that customize delete behavior or analytics. + */ INTENTIONAL_DELETION = "intentionalDeletion", + /** + * Controls whether a transaction is added to the undo/redo history. + * Value: boolean + * Readers: history plugin integrations (e.g. table drag preview). + */ ADD_TO_HISTORY = "addToHistory", }packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (4)
325-340: Potential cell lookup mismatch using TableMap.map offsetsUsing tableNode.nodeAt(cellPos) assumes TableMap.map positions are relative to the table node and directly usable with nodeAt. In prosemirror-tables, offsets are relative to the start of the table, but fetching cells can be tricky with rowspans/colspans.
If you observe false negatives in emptiness checks, consider reading with TableMap and tableNode.child traversal or use existing helpers (e.g., from prosemirror-tables) to locate cells by row/column.
Also applies to: 397-412
34-40: UX: very different action thresholds (150px columns vs 40px rows)Columns require ~150px drag to trigger while rows require 40px. If intentional, ignore. Otherwise consider:
- Make thresholds configurable via options.
- Normalize values, or base on button size and device pixel ratio for predictable UX.
- const ACTION_THRESHOLD = 150; // pixels total distance to trigger action + const ACTION_THRESHOLD = 80; // TODO: tweak or inject via options- const ACTION_THRESHOLD = 40; // pixels total distance to trigger action + const ACTION_THRESHOLD = 80; // TODO: tweak or inject via optionsAlso applies to: 136-143
112-118: Cleanup: body cursor style is reset but never setonMouseUp resets document.body.style.cursor = "" but no code sets it during drag. Either set a cursor during drag (col/row-resize) or remove the reset for clarity.
- document.body.style.cursor = ""; document.body.style.userSelect = "";Also applies to: 210-217
219-261: DOM mapping for tables is fragile; prefer view.domAtPos + nearest table or helpersThe manual walk from domAtPos(pos + 1) to find the TABLE element may fail with wrappers or widgets. If available in your helpers, prefer a robust “findTable” that maps PM node positions to DOM tables using view.nodeDOM(pos) and fallback heuristics, or store a WeakMap from tableNode to its DOM in setup.
packages/editor/src/core/extensions/table/table-header.ts (1)
42-45: Hide-content attribute and inline background style: minor hardening
- hideContent: Looks good and matches CSS .content-hidden.
- Background style: Always emitting background-color: ${value}; means “none” becomes an inline style. If you want CSS defaults to apply when unset, only render when value !== "none".
- class: node.attrs.hideContent ? "content-hidden" : "", - style: `background-color: ${node.attrs.background};`, + class: node.attrs.hideContent ? "content-hidden" : "", + style: node.attrs.background !== "none" ? `background-color: ${node.attrs.background};` : undefined,Also applies to: 60-62
packages/editor/src/core/plugins/drag-handle.ts (2)
29-31: Type the scroll parent cache and return type for clarityExplicit generics improve readability and IntelliSense, and ensure callers get an Element back.
-const scrollParentCache = new WeakMap(); +const scrollParentCache = new WeakMap<HTMLElement | SVGElement, Element>(); ... -export const getScrollParent = (node: HTMLElement | SVGElement) => { +export const getScrollParent = (node: HTMLElement | SVGElement): Element => {Also applies to: 68-86
132-141: Pointer events would simplify cross-input behaviorThe plugin uses HTML5 drag events. Pointer events (pointerdown/move/up + setPointerCapture) can reduce edge cases (e.g., lost events, platform inconsistencies) and unify mouse/touch.
Optional follow-up: migrate to pointer events in a separate PR to minimize risk here.
Also applies to: 210-273
packages/editor/src/styles/table.css (3)
58-73: Hover-to-reveal handles: add a mobile-friendly fallbackOn touch devices, :hover doesn’t apply, making the controls effectively invisible. Consider forcing opacity: 1 under (hover: none).
@media (hover: none) { .table-wrapper table td .table-col-handle-container > button, .table-wrapper table th .table-col-handle-container > button, .table-wrapper table td .table-row-handle-container > button, .table-wrapper table th .table-row-handle-container > button { opacity: 1; } }
95-122: Z-index and marker stacking contextsMarkers use z-index: 10. Ensure they appear above table borders/resize handles (z-index: 5) but below floating UI (menus, tooltips). If conflicts arise, centralize z-indices in variables.
209-271: Insert buttons visibility and interactionThe buttons are pointer-events: none until table hover, which can make access harder on touch devices. Consider the same (hover: none) override to ensure they remain usable on mobile.
packages/editor/src/core/extensions/table/table/table.ts (1)
264-268: Avoid runtime casts by widening TableView typingCasting
decorations as Decoration[]is harmless but avoidable. Acceptingreadonly Decoration[]inTableViewaligns with ProseMirror and removes the need for the cast.Apply this change in
table-view.tsx:- constructor( - node: ProseMirrorNode, - cellMinWidth: number, - decorations: Decoration[], - editor: Editor, - getPos: () => number - ) { + constructor( + node: ProseMirrorNode, + cellMinWidth: number, + decorations: readonly Decoration[], + editor: Editor, + getPos: () => number + ) {Then you can drop the
as Decoration[]here.packages/editor/src/core/extensions/table/plugins/drag-handles/color-selector.tsx (2)
15-23: Use null/transparent consistently; avoid writing invalid CSS downstreamYou’re correctly clearing with
null. Ensure the styling layer (TableCell renderHTML) skips emitting CSS when attrs are null (see related comment). If your palette includes a “None” option, prefernullortransparentover"none"for CSS correctness.If
COLORS_LISTincludes a “None” item, update it to usetransparentor handle it vianulland UI-only label. Verify no code paths setbackground: "none"ortextColor: "none".
86-99: A11y: add accessible names for swatches and the clear buttonThe color buttons lack accessible labels. Screen readers will announce them as “button.” Add
aria-label/title, and consideraria-pressedto reflect selection, if tracked.Apply this diff to both the swatches and clear button:
- <button + <button key={color.key} type="button" className="flex-shrink-0 size-6 rounded border-[0.5px] border-custom-border-400 hover:opacity-60 transition-opacity" style={{ backgroundColor: color.backgroundColor, }} + aria-label={`Set background color to ${color.key}`} + title={`Set background color to ${color.key}`} onClick={() => { handleBackgroundColorChange(editor, color.backgroundColor); onSelect(color.backgroundColor); }} /> @@ - <button + <button type="button" className="flex-shrink-0 size-6 grid place-items-center rounded text-custom-text-300 border-[0.5px] border-custom-border-400 hover:bg-custom-background-80 transition-colors" + aria-label="Clear background color" + title="Clear background color" onClick={() => { handleBackgroundColorChange(editor, null); onSelect(null); }} >Also applies to: 100-110
packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (2)
77-86: Also update on selection/editable changes to keep markers in sync
updateAllTables()only runs whendocchanges. Markers may get out of sync when toggling editability or when tables enter/exit the viewport due to selection/scroll without doc changes. CallingupdateAllTables()on any view update is cheap and more robust, or at least when selection changes.Apply one of these:
- Minimal change:
- update(view, prevState) { - // Update when document changes - if (!prevState.doc.eq(view.state.doc)) { - updateAllTables(); - } - }, + update(view, prevState) { + if ( + !prevState.doc.eq(view.state.doc) || + !prevState.selection.eq(view.state.selection) || + view.editable !== (prevState as any).editable // may not exist; triggers update when toggled + ) { + updateAllTables(); + } + },
- Or simplest (safe in practice):
- update(view, prevState) { - // Update when document changes - if (!prevState.doc.eq(view.state.doc)) { - updateAllTables(); - } - }, + update() { + updateAllTables(); + },
99-107: Minor: mark marker container as inert to assistive techConsider adding
aria-hidden="true"androle="presentation"to the container to keep it out of the a11y tree.Apply this diff:
const createMarkerContainer = (): HTMLElement => { const el = document.createElement("div"); el.classList.add("table-drag-marker-container"); el.contentEditable = "false"; + el.setAttribute("aria-hidden", "true"); + el.setAttribute("role", "presentation"); el.appendChild(createDropMarker()); el.appendChild(createColDragMarker()); el.appendChild(createRowDragMarker()); return el; };packages/editor/src/core/extensions/table/plugins/drag-handles/row/dropdown.tsx (3)
32-41: Guard duplicate action when no row selection is activeIf a user opens the dropdown without a row CellSelection (e.g., caret selection),
getSelectedRows(...)may return an empty array. Depending onduplicateRows’s behavior, this could be a silent no-op. Consider falling back to duplicating the anchor row of the current selection or bailing with a toast. Also remove debug logs ingetSelectedRows(helpers.ts) to avoid console noise.Example minimal hardening:
-import { TableMap } from "@tiptap/pm/tables"; +import { TableMap, getSelectedRect } from "@tiptap/pm/tables"; -import { findTable, getSelectedRows } from "@/extensions/table/table/utilities/helpers"; +import { findTable, getSelectedRows, isCellSelection } from "@/extensions/table/table/utilities/helpers"; const tableMap = TableMap.get(table.node); let tr = editor.state.tr; - const selectedRows = getSelectedRows(editor.state.selection, tableMap); + let selectedRows = getSelectedRows(editor.state.selection, tableMap); + if (selectedRows.length === 0 && isCellSelection(editor.state.selection)) { + const rect = getSelectedRect(editor.state.selection, tableMap); + selectedRows = [rect.top]; + } tr = duplicateRows(table, selectedRows, tr); editor.view.dispatch(tr);
65-79: Header row toggle lacks state feedbackThe “Header row” control always renders the same icon and text. Consider reflecting the current header-row state (checked/unchecked or toggled icon) for better UX.
83-97: i18n readiness for labelsLabels like “Insert above”, “Duplicate”, “Clear contents”, etc., are hard-coded. If the editor is localized, route these through your i18n layer.
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (3)
71-90: Open-menu click vs. drag start can conflictYou start drag logic on every
onMouseDown(select row, attach global listeners) while also using Floating UI’s click interaction to open the menu. A quick click to open the menu will run drag setup unnecessarily. Consider switching to PointerEvents with a small move-threshold before entering “drag mode,” or only attaching listeners after detecting movement beyond a threshold.
124-147: Preview creation: ensure we only hide content when preview is created
updateCellContentVisibility(editor, true)is called withinconstructRowDragPreview. IfpseudoRowis never created (e.g., markers absent), content remains visible, andhandleFinishstill attempts to restore it. That’s fine functionally, but a small boolean guard likedidHideContentcan avoid redundant state updates.
155-197: Minor: z-index layering and pointer-eventsBackdrop is zIndex 99 and panel is 100; the handle container is zIndex 20. If table toolbars or other overlays use comparable stacking contexts, consider centralizing z-index tokens to avoid overlap issues. Also, set
pointer-events: noneon the drag preview element to avoid capturing clicks during drag.packages/editor/src/core/extensions/table/plugins/drag-handles/column/dropdown.tsx (3)
32-41: Guard duplicate action and remove debug logs in helpersAs with rows, ensure
duplicateColumnshandles the case where no column CellSelection is active. Consider falling back to the anchor column or warning the user. Also,getSelectedColumnscurrently logs to console—remove these logs for production.
65-79: Header column toggle could reflect current stateRender an active/checked state if the current selection is a header column to improve affordance.
83-97: i18n readiness for labelsSame i18n note as the row dropdown; route button labels through localization.
packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts (2)
61-62: Consider ignoreSelection on widget decorationsIf the handle shouldn’t be part of selection mapping, pass
ignoreSelection: truein the widget spec to reduce unnecessary re-mapping churn during selection changes.Example:
- decorations.push(Decoration.widget(pos, () => dragHandleComponent.element)); + decorations.push(Decoration.widget(pos, () => dragHandleComponent.element, { ignoreSelection: true }));
27-31: Initial render timingHandles are only built when
haveTableRelatedChangesis true. If the editor mounts with a table already focused and no immediate transaction occurs, handles may not render until the next selection/doc change. If that’s undesirable, consider forcing an initial build when a table is present and there are no previous decorations.packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (2)
6-9: JSDoc return type doesn’t match implementationThe comment claims
HTMLTableElementis returned, but the function returns an object withtableElementandtableBodyElement. Update the doc for correctness.- * @returns {HTMLTableElement} The pseudo table. + * @returns {{ tableElement: HTMLTableElement, tableBodyElement: HTMLTableSectionElement }} The pseudo table and its tbody.
14-21: Preview table should ignore pointer eventsTo avoid the preview intercepting clicks/hover during drag, set
pointer-events: noneon the preview table.const tableElement = document.createElement("table"); tableElement.classList.add("table-drag-preview"); tableElement.classList.add("bg-custom-background-100"); tableElement.style.opacity = "0.9"; + tableElement.style.pointerEvents = "none";packages/editor/src/core/extensions/table/plugins/drag-handles/column/utils.ts (1)
21-80: Code duplication with row utilities - consider refactoring.The
calculateColumnDropIndexfunction has nearly identical logic tocalculateRowDropIndexfrom the row utilities file, just with different axis names (left/width vs top/height). This violates the DRY principle.Consider creating a generic drop index calculator that can work for both dimensions:
// In a shared utils file export const calculateDropIndex = ( index: number, items: Array<{ position: number; size: number }>, currentPosition: number, axis: 'horizontal' | 'vertical' ): number => { // Generic implementation that works for both rows and columns // ... }; // Then in column utils export const calculateColumnDropIndex = (col: number, columns: TableColumn[], left: number): number => { const items = columns.map(c => ({ position: c.left, size: c.width })); return calculateDropIndex(col, items, left, 'horizontal'); }; // And in row utils export const calculateRowDropIndex = (row: number, rows: TableRow[], top: number): number => { const items = rows.map(r => ({ position: r.top, size: r.height })); return calculateDropIndex(row, items, top, 'vertical'); };packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
133-136: Inefficient drop marker position calculation.The drop marker position calculation uses a ternary operator that could be simplified for better readability and performance.
- const dropMarkerLeftPx = - dropIndex <= col ? columns[dropIndex].left : columns[dropIndex].left + columns[dropIndex].width; + const dropColumn = columns[dropIndex]; + const dropMarkerLeftPx = dropColumn.left + (dropIndex > col ? dropColumn.width : 0);packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (1)
7-14: Consider null safety for querySelector results.The marker getter functions return
HTMLElement | nullbut the hide functions don't check for null before accessing classList.Add null checks to the hide functions:
export const hideDropMarker = (element: HTMLElement): void => { + if (!element) return; if (!element.classList.contains("hidden")) { element.classList.add("hidden"); } }; export const hideDragMarker = (element: HTMLElement): void => { + if (!element) return; if (!element.classList.contains("hidden")) { element.classList.add("hidden"); } };Also applies to: 48-58
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (6)
210-213: Use nodeDOM/closest('table') for robust table sizingdomAtPos(table.start) may resolve into a descendant (or a text node), making parentElement unreliable. Prefer nodeDOM at the table’s pos with a safe fallback to closest('table').
-export const getTableHeightPx = (table: TableNodeLocation, editor: Editor): number => { - const dom = editor.view.domAtPos(table.start); - return dom.node.parentElement?.offsetHeight ?? 0; -}; +export const getTableHeightPx = (table: TableNodeLocation, editor: Editor): number => { + const tableNode = editor.view.nodeDOM(table.pos) as HTMLElement | null; + if (tableNode instanceof HTMLElement) return tableNode.offsetHeight ?? 0; + const { node } = editor.view.domAtPos(table.start + 1); + const el = (node instanceof Element ? node : node.parentElement)?.closest("table") as HTMLElement | null; + return el?.offsetHeight ?? 0; +}; -export const getTableWidthPx = (table: TableNodeLocation, editor: Editor): number => { - const dom = editor.view.domAtPos(table.start); - return dom.node.parentElement?.offsetWidth ?? 0; -}; +export const getTableWidthPx = (table: TableNodeLocation, editor: Editor): number => { + const tableNode = editor.view.nodeDOM(table.pos) as HTMLElement | null; + if (tableNode instanceof HTMLElement) return tableNode.offsetWidth ?? 0; + const { node } = editor.view.domAtPos(table.start + 1); + const el = (node instanceof Element ? node : node.parentElement)?.closest("table") as HTMLElement | null; + return el?.offsetWidth ?? 0; +};Also applies to: 221-224
201-203: Clarify parameter naming to avoid confusion with TableMap.mapMinor readability: rename parameters so map.map[index] reads less awkwardly.
-export const getTableCellWidgetDecorationPos = (table: TableNodeLocation, map: TableMap, index: number): number => - table.start + map.map[index] + 1; +export const getTableCellWidgetDecorationPos = ( + table: TableNodeLocation, + tableMap: TableMap, + mapIndex: number +): number => table.start + tableMap.map[mapIndex] + 1;
169-176: Guard against out-of-range row/column indicesAdd bounds checks to prevent invalid access to TableMap.map when index is out of bounds (e.g., stale UI state).
export const selectColumn = (table: TableNodeLocation, index: number, tr: Transaction): Transaction => { - const { map } = TableMap.get(table.node); + const { map, width } = TableMap.get(table.node); + if (index < 0 || index >= width) return tr; const anchorCell = table.start + map[index]; const $anchor = tr.doc.resolve(anchorCell); return tr.setSelection(CellSelection.colSelection($anchor)); }; export const selectRow = (table: TableNodeLocation, index: number, tr: Transaction): Transaction => { - const { map, width } = TableMap.get(table.node); + const { map, width, height } = TableMap.get(table.node); + if (index < 0 || index >= height) return tr; const anchorCell = table.start + map[index * width]; const $anchor = tr.doc.resolve(anchorCell); return tr.setSelection(CellSelection.rowSelection($anchor)); };Also applies to: 185-192
124-131: Micro-optimization: use a Set for membership checksFor large selections, Set lookup avoids repeated includes() scans.
export const isRectSelected = (rect: Rect, selection: CellSelection): boolean => { const map = TableMap.get(selection.$anchorCell.node(-1)); const cells = map.cellsInRect(rect); const selectedCells = map.cellsInRect(getSelectedRect(selection, map)); - - return cells.every((cell) => selectedCells.includes(cell)); + const selected = new Set(selectedCells); + return cells.every((cell) => selected.has(cell)); };
63-71: Prefer tr.selectionSet for selection changes; optional simplificationYou can rely on Transaction.selectionSet instead of comparing new/old selections. Slightly cheaper and clearer.
): table is TableNodeLocation => - editor.isEditable && table !== undefined && (tr.docChanged || !newState.selection.eq(oldState.selection)); + editor.isEditable && table !== undefined && (tr.docChanged || tr.selectionSet);
40-44: Reuse NodeWithPos type instead of redefining TableNodeLocationfindParentNode returns a NodeWithPos (pos, start, depth, node). Reusing that type avoids drift and preserves depth when needed.
-import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; +import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; +import type { NodeWithPos } from "@tiptap/core"; -export type TableNodeLocation = { - pos: number; - start: number; - node: ProseMirrorNode; -}; +export type TableNodeLocation = NodeWithPos<ProseMirrorNode>; export const findTable = (selection: Selection): TableNodeLocation | undefined => findParentNode((node) => node.type.spec.tableRole === "table")(selection);Also applies to: 51-53
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
packages/editor/src/core/constants/meta.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/color-selector.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/dropdown.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/plugin.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/utils.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/dropdown.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts(4 hunks)packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts(4 hunks)packages/editor/src/core/extensions/table/table-cell.ts(2 hunks)packages/editor/src/core/extensions/table/table-header.ts(3 hunks)packages/editor/src/core/extensions/table/table/table-controls.ts(0 hunks)packages/editor/src/core/extensions/table/table/table.ts(6 hunks)packages/editor/src/core/extensions/table/table/utilities/helpers.ts(2 hunks)packages/editor/src/core/plugins/drag-handle.ts(4 hunks)packages/editor/src/styles/table.css(3 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/core/extensions/table/table/table-controls.ts
🧰 Additional context used
🧬 Code graph analysis (11)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/plugin.ts (2)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (3)
findTable(51-52)haveTableRelatedChanges(63-70)getTableCellWidgetDecorationPos(201-202)packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts (1)
decorations(68-70)
packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (1)
packages/editor/src/index.ts (1)
CORE_EXTENSIONS(24-24)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/dropdown.tsx (1)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (2)
findTable(51-52)getSelectedRows(106-116)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/dropdown.tsx (1)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (2)
findTable(51-52)getSelectedColumns(89-98)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts (2)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (3)
findTable(51-52)haveTableRelatedChanges(63-70)getTableCellWidgetDecorationPos(201-202)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (2)
RowDragHandle(47-199)RowDragHandleProps(42-45)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (5)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (5)
findTable(51-52)selectRow(185-192)getTableHeightPx(210-213)isCellSelection(13-13)getTableWidthPx(221-224)packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts (3)
getTableRowNodesInfo(88-105)calculateRowDropIndex(21-80)constructRowDragPreview(114-143)packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (7)
getDropMarker(7-8)getRowDragMarker(51-52)hideDropMarker(10-14)hideDragMarker(54-58)updateRowDropMarker(32-46)DROP_MARKER_THICKNESS(5-5)updateRowDragMarker(79-96)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (1)
updateCellContentVisibility(49-60)packages/editor/src/core/extensions/table/plugins/drag-handles/row/dropdown.tsx (1)
RowOptionsDropdown(62-100)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (1)
packages/editor/src/core/extensions/table/table/table-view.tsx (3)
selectColumn(471-480)selectRow(482-491)dom(274-276)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts (2)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (3)
TableNodeLocation(40-44)isCellSelection(13-13)getSelectedRect(78-81)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (3)
constructDragPreviewTable(10-22)cloneTableCell(29-41)updateCellContentVisibility(49-60)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/utils.ts (2)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (3)
TableNodeLocation(40-44)isCellSelection(13-13)getSelectedRect(78-81)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (3)
constructDragPreviewTable(10-22)cloneTableCell(29-41)updateCellContentVisibility(49-60)
packages/editor/src/core/extensions/table/table/table.ts (4)
packages/editor/src/core/extensions/table/table/table-view.tsx (1)
TableView(256-492)packages/editor/src/core/extensions/table/plugins/drag-handles/column/plugin.ts (2)
decorations(70-72)TableColumnDragHandlePlugin(21-74)packages/editor/src/core/extensions/table/plugins/drag-handles/row/plugin.ts (2)
decorations(68-70)TableRowDragHandlePlugin(21-72)packages/editor/src/core/extensions/table/plugins/selection-outline/plugin.ts (1)
decorations(55-57)
packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts (1)
packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (3)
DROP_MARKER_CLASS(1-1)COL_DRAG_MARKER_CLASS(2-2)ROW_DRAG_MARKER_CLASS(3-3)
🪛 ast-grep (0.38.6)
packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts
[warning] 74-74: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: element.innerHTML = pseudoColumn.outerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 93-93: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: element.innerHTML = pseudoRow.outerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 74-74: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: element.innerHTML = pseudoColumn.outerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 93-93: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: element.innerHTML = pseudoRow.outerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (19)
packages/editor/src/core/constants/meta.ts (1)
3-4: Good addition: explicit meta keys for deletion and history controlAdding INTENTIONAL_DELETION and ADD_TO_HISTORY clarifies transaction semantics and avoids magic strings scattered across the codebase.
packages/editor/src/core/extensions/table/plugins/insert-handlers/utils.ts (1)
274-289: Append behavior of addColumn/addRow supports passing width/heightThe ProseMirror/Tiptap utility functions
addColumn(tr: Transaction, rect: TableRect, index: number): Transaction addRow( tr: Transaction, rect: TableRect, index: number): Transactionaccept a zero-based insertion index and allow values up to
rect.map.width(for columns) orrect.map.height(for rows) to append at the end. There is no “after” boolean parameter—the index itself determines where the new column or row goes. (unpkg.com)As a result, using
const lastColumnIndex = tableMapData.width; const newTr = addColumn(tr, rect, lastColumnIndex); // … const lastRowIndex = tableMapData.height; const newTr = addRow(tr, rect, lastRowIndex);correctly appends a column or row. The original off-by-one suggestion can be ignored.
Likely an incorrect or invalid review comment.
packages/editor/src/core/extensions/table/table-header.ts (1)
20-21: Content model broadened to block+: verify interoperabilityMoving TABLE_HEADER content from paragraph+ to block+ aligns with Tiptap defaults and enables richer content, but can affect:
- Paste/HTML parsing rules expecting a paragraph wrapper.
- Any commands assuming a single paragraph child.
Please verify downstream commands (clearSelectedCells, duplicate, drag-preview) operate correctly with nested blocks.
packages/editor/src/core/plugins/drag-handle.ts (1)
19-20: Exclude drag preview tables from targeting — good callFiltering with table:not(.table-drag-preview) prevents accidental interactions with preview nodes.
packages/editor/src/styles/table.css (2)
28-57: Good: selection outline suppressed for content-hidden cellsselectedCell:not(.content-hidden) prevents double-highlighting during drag previews.
78-81: Hidden content via visibility: hidden — confirm caret behaviorUsing visibility: hidden preserves layout but also hides the caret/selection rendering inside the cell. Verify that keyboard navigation during drag preview doesn’t trap focus or lead to confusing UX.
packages/editor/src/core/extensions/table/table-cell.ts (1)
50-52: ✅ hideContent is ephemeral and safely cleaned upI’ve verified that:
- The
updateCellContentVisibilityhelper explicitly setsCORE_EDITOR_META.ADD_TO_HISTORYtofalse, so togglinghideContentnever pollutes the editor’s undo/redo history.- Both column and row drag handles call
updateCellContentVisibility(editor, true)at drag start andupdateCellContentVisibility(editor, false)at drag end, ensuring all cells are reset tohideContent: falseonce the interaction finishes.- The corresponding
.content-hiddenCSS rules exist inpackages/editor/src/styles/table.css, so the UI behavior is scoped to the drag-preview and doesn’t affect persisted visuals.No further changes are needed here.
packages/editor/src/core/extensions/table/table/table.ts (2)
180-183: clearSelectedCells command integrates cleanlyImporting
deleteCellSelectionand exposing it asclearSelectedCellsis a good, minimal surface for dropdown “Clear contents.” This should be a no-op when there’s no cell selection and return true when applied.Also applies to: 10-10
279-281: Plugin wiring looks correct and ordering is sensibleRegistering column/row drag-handle plugins alongside
TableInsertPluginand before/aftercolumnResizingfollows expected execution order. No issues.packages/editor/src/core/extensions/table/plugins/drag-handles/column/plugin.ts (1)
34-46: Smart staleness check to avoid unnecessary re-rendersMapping the previous DecorationSet and verifying one widget per expected position is a solid way to skip rebuilds. Nicely done.
packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (1)
49-60: History and focus: looks goodUsing
ADD_TO_HISTORY = falsewhile togglinghideContentavoids polluting undo history. The doubleupdateAttributesfor both cell and header is appropriate.packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts (1)
107-143: LGTM! Well-structured drag preview construction.The
constructRowDragPreviewfunction properly handles cell selection validation, clones cells safely, and manages content visibility. Good use of the shared utility functions.packages/editor/src/core/extensions/table/plugins/drag-handles/column/utils.ts (1)
115-143: LGTM! Consistent implementation with row preview.The
constructColumnDragPreviewfunction follows the same pattern as the row preview function, properly handling cell selection and content visibility.packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
175-197: LGTM! Well-implemented floating UI integration.The dropdown implementation properly uses FloatingUI hooks for positioning and interaction management, with appropriate z-index layering for the overlay and dropdown.
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
154-186: LGTM! Clean helper functions for table manipulation.The
tableToCellsandtableFromCellshelper functions are well-implemented with proper use of visited cell tracking and Fragment creation.packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (1)
1-6: LGTM! Well-organized constants.Good separation of concerns with clearly named CSS class constants and drop marker thickness configuration.
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (3)
78-81: LGTM: correct rect computation from CellSelectionUsing start(-1) with anchor/head cells and TableMap.rectBetween is the right approach.
138-145: LGTM: column selection detectionRect spanning full height for a single column and isRectSelected are used correctly.
152-160: LGTM: row selection detectionRect spanning full width for a single row is correctly validated via isRectSelected.
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts
Show resolved
Hide resolved
packages/editor/src/core/extensions/table/plugins/drag-handles/color-selector.tsx
Show resolved
Hide resolved
…t/table-entity-rearrrange
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (1)
92-114: Always detach listeners; don’t early-return when markers are missing (prevents leaks and stuck drag state).
handleFinishreturns early ifdropMarkerordragMarkeris falsy, skippingremoveEventListenercleanup. This can leave global listeners attached and keep cell content hidden if an error path is taken. Detach listeners first, then hide whichever markers exist. This also resolves the prior review comment.Apply this focused diff:
- const handleFinish = (): void => { - if (!dropMarker || !dragMarker) return; - hideDropMarker(dropMarker); - hideDragMarker(dragMarker); - - if (isCellSelection(editor.state.selection)) { - updateCellContentVisibility(editor, false); - } - - if (row !== dropIndex) { - let tr = editor.state.tr; - const selection = editor.state.selection; - if (isCellSelection(selection)) { - const table = findTable(selection); - if (table) { - tr = moveSelectedRows(editor, table, selection, dropIndex, tr); - } - } - editor.view.dispatch(tr); - } - window.removeEventListener("mouseup", handleFinish); - window.removeEventListener("mousemove", handleMove); - }; + const handleFinish = (): void => { + // Always detach listeners + window.removeEventListener("mouseup", handleFinish); + window.removeEventListener("mousemove", handleMove); + + // Hide markers if present + if (dropMarker) hideDropMarker(dropMarker); + if (dragMarker) hideDragMarker(dragMarker); + + // Restore content visibility if we hid it during preview + if (isCellSelection(editor.state.selection)) { + updateCellContentVisibility(editor, false); + } + + // Commit move if index changed + if (row !== dropIndex) { + let tr = editor.state.tr; + const selection = editor.state.selection; + if (isCellSelection(selection)) { + const t = findTable(selection); + if (t) { + tr = moveSelectedRows(editor, t, selection, dropIndex, tr); + } + } + editor.view.dispatch(tr); + } + };packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
92-114: Ensure cleanup even when markers are missing; avoid early-return before removing listeners.Same leak risk as the row handle:
handleFinishreturns early when markers are null, skippingremoveEventListener. Detach first, then conditionally hide markers. This is consistent with prior feedback.Apply this diff:
- const handleFinish = () => { - if (!dropMarker || !dragMarker) return; - hideDropMarker(dropMarker); - hideDragMarker(dragMarker); - if (isCellSelection(editor.state.selection)) { - updateCellContentVisibility(editor, false); - } - if (col !== dropIndex) { - let tr = editor.state.tr; - const selection = editor.state.selection; - if (isCellSelection(selection)) { - const table = findTable(selection); - if (table) { - tr = moveSelectedColumns(editor, table, selection, dropIndex, tr); - } - } - editor.view.dispatch(tr); - } - window.removeEventListener("mouseup", handleFinish); - window.removeEventListener("mousemove", handleMove); - }; + const handleFinish = () => { + // Always detach listeners + window.removeEventListener("mouseup", handleFinish); + window.removeEventListener("mousemove", handleMove); + + // Hide markers if present + if (dropMarker) hideDropMarker(dropMarker); + if (dragMarker) hideDragMarker(dragMarker); + + if (isCellSelection(editor.state.selection)) { + updateCellContentVisibility(editor, false); + } + if (col !== dropIndex) { + let tr = editor.state.tr; + const selection = editor.state.selection; + if (isCellSelection(selection)) { + const t = findTable(selection); + if (t) { + tr = moveSelectedColumns(editor, t, selection, dropIndex, tr); + } + } + editor.view.dispatch(tr); + } + };packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (2)
20-51: Optional guard: validate computed columnStart/columnEnd.
selectionis typed asCellSelection, so these are normally set. A light guard improves resilience against edge-cases and future refactors.Apply right after the forEachCell loop:
}); + if (columnStart === -1 || columnEnd === -1 || columnStart >= columnEnd) { + console.warn("Invalid column selection"); + return tr; + }
62-91: Optional guard: validate computed rowStart/rowEnd.Same rationale as columns.
}); + if (rowStart === -1 || rowEnd === -1 || rowStart >= rowEnd) { + console.warn("Invalid row selection"); + return tr; + }
🧹 Nitpick comments (5)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (1)
118-146: Defensive try/catch around move handler to guarantee cleanup on runtime errors.If an exception occurs inside
handleMove(e.g., DOM becomes unavailable during drag), listeners remain until mouseup. Consider catching errors and callinghandleFinish()to ensure cleanup.Apply locally inside
handleMove:- const handleMove = (moveEvent: MouseEvent): void => { + const handleMove = (moveEvent: MouseEvent): void => { + try { if (!dropMarker || !dragMarker) return; // ... existing logic ... updateRowDragMarker({ element: dragMarker, top: dragMarkerTopPx, height: dragMarkerHeightPx, pseudoRow, }); - }; + } catch (err) { + console.error("RowDragHandle move error:", err); + handleFinish(); + } + };packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (3)
136-140: Drop marker left offset has inconsistent centering vs row handle (-1 px).
left: dropMarkerLeftPx - Math.floor(DROP_MARKER_THICKNESS / 2) - 1shifts by an extra pixel compared to the row variant (which uses 0.5 thickness without the extra -1). This can cause visible misalignment on 1x/2x displays. Align both to the same centering math.Apply this minimal fix:
- left: dropMarkerLeftPx - Math.floor(DROP_MARKER_THICKNESS / 2) - 1, + left: dropMarkerLeftPx - DROP_MARKER_THICKNESS / 2,
118-147: Add defensive error handling in move handler to guarantee cleanup on exceptions.Mirror the row handle suggestion: wrap
handleMovebody in try/catch and callhandleFinish()on error.- const handleMove = (moveEvent: MouseEvent) => { + const handleMove = (moveEvent: MouseEvent) => { + try { if (!dropMarker || !dragMarker) return; // ... existing logic ... updateColDragMarker({ element: dragMarker, left: dragMarkerLeftPx, width: dragMarkerWidthPx, pseudoColumn, }); - }; + } catch (err) { + console.error("ColumnDragHandle move error:", err); + handleFinish(); + } + };
46-69: Consider extracting shared Floating UI + dropdown patterns into a small hook.Both column and row handles duplicate the same Floating UI setup and click/dismiss/role integrations. A
useHandleDropdown()hook would de-duplicate and simplify both components.Also applies to: 70-156
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
135-162: Duplicate columns: add optional normalization to indices.Current validation checks bounds but not duplicates/order. Normalizing indices (unique + ascending) avoids double-inserting the same column and makes behavior predictable.
- // Validate column indices - if (columnIndices.some((idx) => idx < 0 || idx >= width)) { + // Validate column indices + if (!columnIndices.length || columnIndices.some((idx) => idx < 0 || idx >= width)) { console.warn("Invalid column indices for duplication"); return tr; } const mapStart = tr.mapping.maps.length; + // Normalize once to avoid double-duplication + const indices = Array.from(new Set(columnIndices)).sort((a, b) => a - b); + for (let row = 0; row < height; row++) { - const lastColumnPos = map[row * width + columnIndices[columnIndices.length - 1]]; + const lastColumnPos = map[row * width + indices[indices.length - 1]]; const nextColumnStart = lastColumnPos + (table.node.nodeAt(lastColumnPos)?.nodeSize ?? 0); const insertPos = tr.mapping.slice(mapStart).map(table.start + nextColumnStart); - for (let i = columnIndices.length - 1; i >= 0; i--) { - const copiedNode = rows[row][columnIndices[i]]; + for (let i = indices.length - 1; i >= 0; i--) { + const copiedNode = rows[row][indices[i]]; if (copiedNode !== null) { tr.insert(insertPos, copiedNode); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/color-selector.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx(1 hunks)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts(1 hunks)packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts(4 hunks)packages/editor/src/core/extensions/table/table/utilities/helpers.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts
- packages/editor/src/core/extensions/table/plugins/insert-handlers/plugin.ts
- packages/editor/src/core/extensions/table/plugins/drag-handles/color-selector.tsx
- packages/editor/src/core/extensions/table/table/utilities/helpers.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (6)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (5)
findTable(51-52)selectColumn(164-171)getTableWidthPx(216-219)isCellSelection(13-13)getTableHeightPx(205-208)packages/editor/src/core/extensions/table/plugins/drag-handles/column/utils.ts (3)
getTableColumnNodesInfo(88-106)calculateColumnDropIndex(21-80)constructColumnDragPreview(115-143)packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (7)
getDropMarker(7-8)getColDragMarker(48-49)hideDropMarker(10-14)hideDragMarker(54-58)updateColDropMarker(16-30)DROP_MARKER_THICKNESS(5-5)updateColDragMarker(60-77)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (1)
updateCellContentVisibility(49-60)packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
moveSelectedColumns(20-51)packages/editor/src/core/extensions/table/plugins/drag-handles/column/dropdown.tsx (1)
ColumnOptionsDropdown(62-100)
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (1)
TableNodeLocation(40-44)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (6)
packages/editor/src/core/extensions/table/table/utilities/helpers.ts (5)
findTable(51-52)selectRow(180-187)getTableHeightPx(205-208)isCellSelection(13-13)getTableWidthPx(216-219)packages/editor/src/core/extensions/table/plugins/drag-handles/row/utils.ts (3)
getTableRowNodesInfo(88-105)calculateRowDropIndex(21-80)constructRowDragPreview(114-143)packages/editor/src/core/extensions/table/plugins/drag-handles/marker-utils.ts (7)
getDropMarker(7-8)getRowDragMarker(51-52)hideDropMarker(10-14)hideDragMarker(54-58)updateRowDropMarker(32-46)DROP_MARKER_THICKNESS(5-5)updateRowDragMarker(79-96)packages/editor/src/core/extensions/table/plugins/drag-handles/utils.ts (1)
updateCellContentVisibility(49-60)packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
moveSelectedRows(62-91)packages/editor/src/core/extensions/table/plugins/drag-handles/row/dropdown.tsx (1)
RowOptionsDropdown(62-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/editor/src/core/extensions/table/plugins/drag-handles/row/drag-handle.tsx (1)
160-201: Floating UI integration and dropdown wiring look good.The portal/overlay usage, z-index layering, and interaction hooks are set up cleanly and match ColumnHandle behavior.
packages/editor/src/core/extensions/table/plugins/drag-handles/column/drag-handle.tsx (1)
160-201: Nice parity with Row handle; UI/overlay plumbing LGTM.Consistent Floating UI setup, styling, and dropdown anchor behavior.
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts (1)
169-201: Table to/from cells approach is clean and consistent for moves.The
tableToCells/tableFromCellshelpers are thoughtfully implemented, handle visited cells, and keep the move operations straightforward.
packages/editor/src/core/extensions/table/plugins/drag-handles/actions.ts
Show resolved
Hide resolved
…entity-rearrrange
4 participants
Description
This PR brings the ability to rearrange rows and columns in a table in the editor. Along with this, users will now witness a more elegant row and column options dropdown with more options that before, including-
Type of Change
Media
Screen.Recording.2025-08-22.at.14.06.00.mov
Summary by CodeRabbit
New Features
Style
Chores