Merge branch 'main' into nick/sd-2045-bug-tablessplitcell-removes-neighboring-cell-content-instead

Author: harbournick

packages/super-editor/src/document-api-adapters/tables-adapter.integration.test.ts

@@ -0,0 +1,74 @@
+/* @vitest-environment jsdom */
+
+import { afterEach, beforeAll, describe, expect, it } from 'vitest';
+import { initTestEditor, loadTestDataForEditorTests } from '@tests/helpers/helpers.js';
+import DocxZipper from '@core/DocxZipper.js';
+import type { Editor } from '../core/Editor.js';
+import { createTableAdapter, tablesSplitAdapter } from './tables-adapter.js';
+
+type LoadedDocData = Awaited<ReturnType<typeof loadTestDataForEditorTests>>;
+
+const DIRECT_MUTATION_OPTIONS = { changeMode: 'direct' } as const;
+
+function mapExportedFiles(files: Array<{ name: string; content: string }>): Record<string, string> {
+  const byName: Record<string, string> = {};
+  for (const file of files) {
+    byName[file.name] = file.content;
+  }
+  return byName;
+}
+
+async function exportDocxFiles(editor: Editor): Promise<Record<string, string>> {
+  const zipper = new DocxZipper();
+  const exportedBuffer = await editor.exportDocx();
+  const exportedFiles = await zipper.getDocxData(exportedBuffer, true);
+  return mapExportedFiles(exportedFiles);
+}
+
+function resolveTableNodeId(result: ReturnType<typeof createTableAdapter>): string {
+  if (!result.success || result.table?.kind !== 'block' || result.table.nodeType !== 'table' || !result.table.nodeId) {
+    throw new Error('Expected create.table to return a table nodeId.');
+  }
+  return result.table.nodeId;
+}
+
+describe('tables adapter DOCX integration', () => {
+  let docData: LoadedDocData;
+  let editor: Editor | undefined;
+
+  beforeAll(async () => {
+    docData = await loadTestDataForEditorTests('blank-doc.docx');
+  });
+
+  afterEach(() => {
+    editor?.destroy();
+    editor = undefined;
+  });
+
+  it('exports a paragraph separator between split tables', async () => {
+    ({ editor } = initTestEditor({
+      content: docData.docx,
+      media: docData.media,
+      mediaFiles: docData.mediaFiles,
+      fonts: docData.fonts,
+      useImmediateSetTimeout: false,
+    }));
+
+    const createResult = createTableAdapter(
+      editor,
+      { rows: 3, columns: 3, at: { kind: 'documentEnd' } },
+      DIRECT_MUTATION_OPTIONS,
+    );
+    const tableNodeId = resolveTableNodeId(createResult);
+
+    const splitResult = tablesSplitAdapter(editor, { nodeId: tableNodeId, atRowIndex: 1 }, DIRECT_MUTATION_OPTIONS);
+    expect(splitResult.success).toBe(true);
+
+    const exportedFiles = await exportDocxFiles(editor);
+    const documentXml = exportedFiles['word/document.xml'];
+
+    expect(documentXml).toBeTruthy();
+    expect(documentXml).not.toMatch(/<\/w:tbl>\s*<w:tbl>/);
+    expect(documentXml).toMatch(/<\/w:tbl>\s*<w:p\b[^>]*(?:\/>|>[\s\S]*?<\/w:p>)\s*<w:tbl>/);
+  });
+});

packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts

@@ -11,6 +11,7 @@ import {
   tablesSetBorderAdapter,
   tablesSetShadingAdapter,
   tablesSplitCellAdapter,
+  tablesSplitAdapter,
 } from './tables-adapter.js';
 
 vi.mock('prosemirror-tables', () => ({
@@ -238,7 +239,7 @@ function makeTableEditor(options: TableEditorOptions = {}): Editor {
                   ? ([content] as ProseMirrorNode[])
                   : [];
               return createNode('paragraph', children, {
-                attrs,
+                attrs: { paragraphProperties: {}, ...attrs },
                 isBlock: true,
                 inlineContent: true,
               });
@@ -361,6 +362,27 @@ describe('tables-adapter regressions', () => {
     expect(tr.insert).toHaveBeenCalledWith(expectedInsertPos, expect.anything());
   });
 
+  it('inserts a separator paragraph before the split-off table', () => {
+    const editor = makeTableEditor();
+    const tr = editor.state.tr as unknown as { insert: ReturnType<typeof vi.fn> };
+    const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode;
+    const expectedInsertPos = tableNode.nodeSize;
+
+    const result = tablesSplitAdapter(editor, { nodeId: 'table-1', atRowIndex: 1 });
+    expect(result.success).toBe(true);
+    expect(tr.insert).toHaveBeenCalledTimes(2);
+
+    const firstInsertCall = tr.insert.mock.calls[0] as [number, ProseMirrorNode];
+    const secondInsertCall = tr.insert.mock.calls[1] as [number, ProseMirrorNode];
+    const insertedSeparator = firstInsertCall[1];
+    const insertedTable = secondInsertCall[1];
+
+    expect(firstInsertCall[0]).toBe(expectedInsertPos);
+    expect(insertedSeparator.type.name).toBe('paragraph');
+    expect(secondInsertCall[0]).toBe(expectedInsertPos + insertedSeparator.nodeSize);
+    expect(insertedTable.type.name).toBe('table');
+  });
+
   it('deletes shiftLeft cells without appending a trailing replacement cell', () => {
     const editor = makeTableEditor();
     const tr = editor.state.tr as unknown as {
@@ -634,6 +656,56 @@ describe('tables-adapter regressions', () => {
     });
   });
 
+  it('applies table shading to all cells when target is a table', () => {
+    const editor = makeTableEditor();
+    const tr = editor.state.tr as unknown as { setNodeMarkup: ReturnType<typeof vi.fn> };
+
+    const result = tablesSetShadingAdapter(editor, {
+      nodeId: 'table-1',
+      color: 'FFFF00',
+    });
+
+    expect(result.success).toBe(true);
+
+    const cellUpdates = tr.setNodeMarkup.mock.calls.filter(
+      (call) =>
+        typeof call[2] === 'object' &&
+        call[2] != null &&
+        (call[2] as { tableCellProperties?: { shading?: { fill?: string } } }).tableCellProperties?.shading?.fill ===
+          'FFFF00',
+    );
+
+    expect(cellUpdates).toHaveLength(4);
+    for (const call of cellUpdates) {
+      expect((call[2] as { background?: { color?: string } }).background).toEqual({ color: 'FFFF00' });
+    }
+  });
+
+  it('does not write cell background when table shading color is auto', () => {
+    const editor = makeTableEditor();
+    const tr = editor.state.tr as unknown as { setNodeMarkup: ReturnType<typeof vi.fn> };
+
+    const result = tablesSetShadingAdapter(editor, {
+      nodeId: 'table-1',
+      color: 'auto',
+    });
+
+    expect(result.success).toBe(true);
+
+    const cellUpdates = tr.setNodeMarkup.mock.calls.filter(
+      (call) =>
+        typeof call[2] === 'object' &&
+        call[2] != null &&
+        (call[2] as { tableCellProperties?: { shading?: { fill?: string } } }).tableCellProperties?.shading?.fill ===
+          'auto',
+    );
+
+    expect(cellUpdates).toHaveLength(4);
+    for (const call of cellUpdates) {
+      expect((call[2] as { background?: unknown }).background).toBeUndefined();
+    }
+  });
+
   it.each([
     {
       name: 'tables.setBorder',

packages/super-editor/src/document-api-adapters/tables-adapter.ts

@@ -84,6 +84,19 @@ function generateParaId(): string {
     .toUpperCase();
 }
 
+function createSeparatorParagraph(schema: Editor['state']['schema']): import('prosemirror-model').Node | null {
+  const paragraphType = schema.nodes.paragraph;
+  if (!paragraphType) return null;
+
+  // Keep separator paragraphs addressable/stable for downstream DOCX roundtrip.
+  const separatorAttrs = {
+    sdBlockId: uuidv4(),
+    paraId: generateParaId(),
+  };
+
+  return paragraphType.createAndFill(separatorAttrs) ?? paragraphType.createAndFill();
+}
+
 function notYetImplemented(operationName: string): never {
   throw new DocumentApiAdapterError('CAPABILITY_UNAVAILABLE', `${operationName} is not yet implemented.`, {
     reason: 'not_implemented',
@@ -1811,10 +1824,16 @@ export function tablesSplitAdapter(
     delete newTableAttrs.paraId; // Avoid duplicate w14:paraId after split.
     delete newTableAttrs.textId; // Avoid duplicate w14:textId after split.
     const newTable = schema.nodes.table.create(newTableAttrs, secondTableRows);
+    const separatorParagraph = createSeparatorParagraph(schema);
+    if (!separatorParagraph) {
+      return toTableFailure('INVALID_TARGET', 'Table split could not create a separator paragraph.');
+    }
 
-    // Insert the new table after the original.
+    // Insert an empty paragraph between tables. Without this block separator,
+    // Word merges adjacent <w:tbl> nodes into one visual table.
     const insertPos = tr.mapping.slice(mapFrom).map(tablePos + tableNode.nodeSize);
-    tr.insert(insertPos, newTable);
+    tr.insert(insertPos, separatorParagraph);
+    tr.insert(insertPos + separatorParagraph.nodeSize, newTable);
 
     applyDirectMutationMeta(tr);
     editor.dispatch(tr);
@@ -3013,6 +3032,38 @@ export function tablesSetShadingAdapter(
     currentProps.shading = { fill: input.color, val: 'clear', color: 'auto' };
     const syncAttrs = resolved.scope === 'table' ? syncExtractedTableAttrs(currentProps) : {};
     tr.setNodeMarkup(resolved.pos, null, { ...currentAttrs, [propsKey]: currentProps, ...syncAttrs });
+
+    if (resolved.scope === 'table') {
+      const tableNode = resolved.node;
+      const tableStart = resolved.pos + 1;
+      const map = TableMap.get(tableNode);
+      const seen = new Set<number>();
+      const mapFrom = tr.mapping.maps.length;
+
+      for (let i = 0; i < map.map.length; i++) {
+        const relPos = map.map[i]!;
+        if (seen.has(relPos)) continue;
+        seen.add(relPos);
+
+        const cellNode = tableNode.nodeAt(relPos);
+        if (!cellNode) continue;
+
+        const cellAttrs = cellNode.attrs as Record<string, unknown>;
+        const cellProps = { ...((cellAttrs.tableCellProperties ?? {}) as Record<string, unknown>) };
+        cellProps.shading = { fill: input.color, val: 'clear', color: 'auto' };
+
+        const nextCellAttrs: Record<string, unknown> = {
+          ...cellAttrs,
+          tableCellProperties: cellProps,
+        };
+
+        if (input.color === 'auto') delete nextCellAttrs.background;
+        else nextCellAttrs.background = { color: input.color };
+
+        tr.setNodeMarkup(tr.mapping.slice(mapFrom).map(tableStart + relPos), null, nextCellAttrs);
+      }
+    }
+
     applyDirectMutationMeta(tr);
     editor.dispatch(tr);
     clearIndexCache(editor);