Skip to content

Fix table context menu actions and dialog-backed cell splitting#243

Closed
alokwhitewolf wants to merge 1 commit intoeigenpal:mainfrom
alokwhitewolf:fix/table-context-menu-split-cell
Closed

Fix table context menu actions and dialog-backed cell splitting#243
alokwhitewolf wants to merge 1 commit intoeigenpal:mainfrom
alokwhitewolf:fix/table-context-menu-split-cell

Conversation

@alokwhitewolf
Copy link
Copy Markdown
Contributor

@alokwhitewolf alokwhitewolf commented Mar 31, 2026

Summary

  • fix right-click table actions so row/column insert/delete commands actually execute
  • add Merge cells and Split cell to the right-click table menu
  • make Split cell dialog-backed with explicit row/column input and unify that behavior across the ProseMirror and legacy table-selection paths
  • fix a follow-up column insertion bug where adding a row and then a column from the middle of a table could corrupt the table shape
  • update and extend E2E coverage for the context menu, split flow, and add-column regression

Fixes #242.

Before

  • right-click table row/column actions did nothing
  • Merge cells / Split cell were missing from the right-click menu
  • existing splitCell behavior was closer to unmerge than true row/column split
    section3_table_ops_before_1774984634472

After

  • right-click table actions execute correctly
  • Merge cells and Split cell are available in the right-click menu
  • Split cell opens a row/column dialog and performs a real structural split
|

section3_table_ops_after_1774984513736
table_split_after_1774982844021

Verification

  • bunx tsc -p packages/react/tsconfig.json --noEmit
  • npx playwright test e2e/tests/table-context-menu.spec.ts --project=chromium --workers=1
  • npx playwright test e2e/tests/table-merge-split.spec.ts --project=chromium --workers=1
  • npx playwright test e2e/tests/table-add-column-regression.spec.ts --project=chromium --workers=1

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 31, 2026

@alokwhitewolf is attempting to deploy a commit to the EigenPal Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docx-editor Ready Ready Preview, Comment Apr 1, 2026 4:37am

Request Review

Copy link
Copy Markdown
Contributor

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @alokwhitewolf, good stuff! Thank you for contributing, I reviewed and left a couple of comments, mostly about code duplication

@@ -0,0 +1,352 @@
import type { Node as PMNode } from 'prosemirror-model';
Copy link
Copy Markdown
Contributor

@jedrazb jedrazb Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things about this file:

  1. Wrong package — this is pure ProseMirror logic with zero React. It should live in packages/core (e.g. alongside TableExtension or in a table utils module), not in packages/react/src/components/.

  2. ~350 lines duplicated with TableToolbar.tsx — the anchor collection, column-width splitting, coordinate adjustment, and table rebuild algorithm here is nearly identical to collectDocumentTableAnchors / splitTableColumnWidths / splitTableCell in TableToolbar.tsx. The only difference is the data types (PMNode vs Document-model Table).

Right now any change one path has to be manually mirrored in the other.

}

function collectPMTableAnchors(
node: PMNode,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the third copy of the same anchor-collection algorithm in this PR (also in tableSplit.ts:collectTableAnchors and TableToolbar.tsx:collectDocumentTableAnchors). All three walk the table, build an occupied grid, and produce { row, col, rowspan, colspan } anchors.

The types differ (PMNode vs TableCell) but the grid logic is identical. Worth extracting into a generic helper that takes a row-iterator and a cell-span-getter.

const attrs = node.attrs as TableAttrs;
const { anchors, totalCols } = collectPMTableAnchors(node, documentCounts);
const anchorByStart = new Map<string, PMTableCellAnchor>();
const anchorByCoveredSlot = new Map<string, PMTableCellAnchor>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This anchorByStart / anchorByCoveredSlot + rebuild loop is the same pattern used in tableSplit.ts:splitActiveTableCell and TableToolbar.tsx:splitTableCell. Here it's used for the save round-trip (PM → DOCX), which means it now runs on every table save — not just split operations.

The old convertPMTableRow was simpler but didn't handle post-split merged cells correctly, so the rewrite is justified. Just flagging that this is a hot path — any bug here silently corrupts saved DOCX files. Consider adding a unit test that round-trips a table with mixed merged/unmerged cells through toProseDocfromProseDoc and asserts the XML is preserved.

const handleEditorContextMenu = useCallback((e: React.MouseEvent) => {
const target = e.target as HTMLElement | null;
if (target?.closest('.paged-editor__pages')) {
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This early return means right-clicks on the visible pages now silently skip this handler and rely on PagedEditor's handleContextMenu callback instead. That works, but it's an implicit coupling — if someone removes or changes the PagedEditor handler, the context menu disappears with no error.

A short comment explaining the delegation would help:

// Visible pages handle their own context menu via PagedEditor.handleContextMenu


return (
<div style={overlayStyle} onClick={onClose} onKeyDown={handleKeyDown}>
<div
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onKeyDown is on the overlay div, which isn't focusable by default. This works because clicks inside the dialog give focus to the inputs, and key events bubble up. But if the dialog opens and the user hasn't clicked anything yet, Escape/Enter won't fire.

Minor — could add tabIndex={-1} and autoFocus on the dialog container, or move the keydown listener to a useEffect with document.addEventListener.

}

export function splitActiveTableCell(
state: EditorState,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the core split algorithm. The same logic (anchor adjustment loop with deltaRows/deltaCols, band intersection checks, and full table rebuild) is duplicated in TableToolbar.tsx:splitTableCell for the document-model path.

Both are needed because the editor has two table representations (PM nodes vs Document model), but the algorithm is identical — only the data access differs. A bug fix in one needs to be mirrored in the other.

At minimum, add a comment at the top of this function pointing to its counterpart:

// Mirror: TableToolbar.splitTableCell() — same algorithm for the Document-model path

}

function collectDocumentTableAnchors(table: Table): {
anchors: DocumentTableAnchor[];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the document-model counterpart of tableSplit.ts:collectTableAnchors. Same grid-walk algorithm, but reads gridSpan / vMerge (OOXML attributes) instead of PM's colspan / rowspan.

The two functions will need to stay in sync. Add a cross-reference comment so the next person editing one knows to check the other.

yash-giantanalytics added a commit to giantanalyticsai/docx-editor that referenced this pull request Apr 5, 2026
…ll splitting

Fixes right-click table row/column actions that were non-functional.
Adds merge cells and split cell to the context menu with a dialog-backed
split cell that accepts explicit row/column counts. Fixes column
insertion regression when adding a row then column from mid-table.
yash-giantanalytics added a commit to giantanalyticsai/docx-editor that referenced this pull request Apr 5, 2026
Lock file was stale after merging upstream PRs eigenpal#201, eigenpal#243, eigenpal#246, eigenpal#247.
Regenerated to include new dependencies (e.g. @happy-dom/global-registrator).
jedrazb added a commit that referenced this pull request Apr 10, 2026
Fixes from code review of PR #243:

1. Restore right-click cursor precision in table cells — the PR snapped
   the cursor to cell start on right-click, losing the user's click
   position for cut/copy/paste operations.

2. Capture cell coordinates at dialog-open time and pass them through to
   the split function — prevents operating on the wrong cell if the user
   clicks elsewhere while the dialog is open.

3. Add onMouseDown stopPropagation to SplitCellDialog overlay — prevents
   ProseMirror focus stealing when interacting with dialog inputs.

4. Fix fragile TextSelection offset (+ 2 → + 1) — let TextSelection.near()
   find the correct position instead of hardcoding an assumption about
   cell content structure.

5. Add bounds validation (rows/cols >= 1) to both splitActiveTableCell and
   splitTableCell to guard against zero/negative inputs from the public API.

6. Fix fragile grid column count heuristic in test helper — read from CSS
   grid-template-columns instead of guessing via Math.sqrt().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jedrazb added a commit that referenced this pull request Apr 10, 2026
…+ review fixes) (#253)

* Fix table context menu actions and cell splitting

* fix: address stability and correctness issues in table context menu PR

Fixes from code review of PR #243:

1. Restore right-click cursor precision in table cells — the PR snapped
   the cursor to cell start on right-click, losing the user's click
   position for cut/copy/paste operations.

2. Capture cell coordinates at dialog-open time and pass them through to
   the split function — prevents operating on the wrong cell if the user
   clicks elsewhere while the dialog is open.

3. Add onMouseDown stopPropagation to SplitCellDialog overlay — prevents
   ProseMirror focus stealing when interacting with dialog inputs.

4. Fix fragile TextSelection offset (+ 2 → + 1) — let TextSelection.near()
   find the correct position instead of hardcoding an assumption about
   cell content structure.

5. Add bounds validation (rows/cols >= 1) to both splitActiveTableCell and
   splitTableCell to guard against zero/negative inputs from the public API.

6. Fix fragile grid column count heuristic in test helper — read from CSS
   grid-template-columns instead of guessing via Math.sqrt().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: extract shared table split algorithm, move ProseMirror split to core

The split-cell logic was duplicated between tableSplit.ts (ProseMirror
path, 387 lines) and TableToolbar.tsx (Document model path, ~280 lines).
Both implemented identical algorithms for anchor collection, column
width redistribution, and neighbor span adjustment.

Changes:
- Extract shared algorithm to packages/core/src/utils/tableSplitAlgorithm.ts
  (computeSplitLayout, redistributeColumnWidths, buildAnchorMaps,
  computeSplitDialogDefaults) — generic over cell type via CellAnchor<T>
- Move ProseMirror split to packages/core/src/prosemirror/commands/tableSplit.ts
  where it belongs alongside other PM table commands
- Refactor TableToolbar.tsx to use shared algorithm (-120 lines)
- Replace tableSplit.ts in react package with thin re-export
- Re-export from commands/table.ts for discoverability

Net: -442 lines, zero duplication in the core split algorithm.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Alok Kumar Bishoyi <kurdleak.alok@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jedrazb
Copy link
Copy Markdown
Contributor

jedrazb commented Apr 10, 2026

Fixed in #253

@jedrazb jedrazb closed this Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants