From 01561438966f9e3b45e66e40a3c9da732d221286 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 9 Feb 2026 19:12:21 -0300 Subject: [PATCH 1/6] fix(super-converter): resolve table style conditional shading on cell import --- .../helpers/legacy-handle-table-cell-node.js | 68 +++++- .../legacy-handle-table-cell-node.test.js | 223 ++++++++++++++++++ 2 files changed, 288 insertions(+), 3 deletions(-) diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js index 6ad889fbbc..45508b22a7 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js @@ -99,11 +99,28 @@ export function handleTableCellNode({ } // Background - const backgroundColor = - resolveShadingFillColor(tableCellProperties.shading) ?? resolveShadingFillColor(tableProperties?.shading); + // Priority cascade: inline cell shading > conditional style shading > table-level shading + let backgroundColor = resolveShadingFillColor(tableCellProperties.shading); + + if (!backgroundColor) { + backgroundColor = resolveConditionalShading({ + tableLook, + rowCnfStyle, + tableCellProperties, + isFirstRow, + isLastRow, + isFirstColumn, + isLastColumn, + referencedStyles, + }); + } + + if (!backgroundColor) { + backgroundColor = resolveShadingFillColor(tableProperties?.shading); + } + const background = { color: backgroundColor }; - // TODO: Do we need other background attrs? if (background.color) attributes['background'] = background; // Vertical Align @@ -267,6 +284,51 @@ function normalizeTableCellContent(content, editor) { return normalized; } +/** + * Resolve cell background color from conditional table style shading. + * Uses rowCnfStyle for row flags (firstRow/lastRow) and cellCnfStyle for column flags, + * since cell cnfStyle only carries column-level annotations. + */ +const resolveConditionalShading = ({ + tableLook, + rowCnfStyle, + tableCellProperties, + isFirstRow, + isLastRow, + isFirstColumn, + isLastColumn, + referencedStyles, +}) => { + const cellCnfStyle = tableCellProperties?.cnfStyle; + const getFlag = (source, flag) => + source && Object.prototype.hasOwnProperty.call(source, flag) ? source[flag] : undefined; + const isRowStyleEnabled = (flag) => getFlag(rowCnfStyle, flag) ?? getFlag(tableLook, flag) ?? true; + const isColStyleEnabled = (flag) => + getFlag(cellCnfStyle, flag) ?? getFlag(rowCnfStyle, flag) ?? getFlag(tableLook, flag) ?? true; + + const getShading = (styleVariant) => resolveShadingFillColor(styleVariant?.tableCellProperties?.shading); + + if (isFirstRow && isRowStyleEnabled('firstRow')) { + const color = getShading(referencedStyles?.firstRow); + if (color) return color; + } + if (isLastRow && isRowStyleEnabled('lastRow')) { + const color = getShading(referencedStyles?.lastRow); + if (color) return color; + } + if (isFirstColumn && isColStyleEnabled('firstColumn')) { + const color = getShading(referencedStyles?.firstCol); + if (color) return color; + } + if (isLastColumn && isColStyleEnabled('lastColumn')) { + const color = getShading(referencedStyles?.lastCol); + if (color) return color; + } + + // wholeTable fallback + return getShading(referencedStyles?.wholeTable) ?? null; +}; + const processInlineCellBorders = (borders, rowBorders) => { if (!borders) return null; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js index c1a48cd91a..829f2feeed 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js @@ -191,6 +191,229 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.background).toEqual({ color: '808080' }); }); + it('applies firstRow conditional shading from referenced styles', () => { + const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; + const row1 = { name: 'w:tr', elements: [cellNode] }; + const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }] }; + const table = { name: 'w:tbl', elements: [row1, row2] }; + + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => []) }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row: row1, + rowBorders: {}, + baseTableBorders: null, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90, 100], + rowIndex: 0, + totalRows: 2, + totalColumns: 2, + _referencedStyles: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '4472C4' }, + }, + }, + }, + }); + + expect(out.attrs.background).toEqual({ color: '4472C4' }); + }); + + it('prefers inline cell shading over conditional style shading', () => { + const cellNode = { + name: 'w:tc', + elements: [ + { + name: 'w:tcPr', + elements: [{ name: 'w:shd', attributes: { 'w:fill': 'FF0000' } }], + }, + { name: 'w:p' }, + ], + }; + const row = { name: 'w:tr', elements: [cellNode] }; + const table = { name: 'w:tbl', elements: [row] }; + + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => []) }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row, + rowBorders: {}, + baseTableBorders: null, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90], + rowIndex: 0, + totalRows: 1, + totalColumns: 1, + _referencedStyles: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '4472C4' }, + }, + }, + }, + }); + + // Inline shading (FF0000) wins over conditional style shading (4472C4) + expect(out.attrs.background).toEqual({ color: 'FF0000' }); + }); + + it('applies wholeTable shading as fallback when no positional style matches', () => { + const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; + const row = { name: 'w:tr', elements: [cellNode] }; + const table = { name: 'w:tbl', elements: [row] }; + + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => []) }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row, + rowBorders: {}, + baseTableBorders: null, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90], + rowIndex: 0, + totalRows: 1, + totalColumns: 1, + _referencedStyles: { + wholeTable: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: 'CCCCCC' }, + }, + }, + }, + }); + + expect(out.attrs.background).toEqual({ color: 'CCCCCC' }); + }); + + it('applies firstRow shading even when cell cnfStyle sets firstRow=false (column-only flag)', () => { + // Reproduces the real DOCX scenario: cell (0,0) has cnfStyle with firstRow=0, firstColumn=1 + // but the row's cnfStyle has firstRow=1. Row-level flags should determine row shading. + const cellNode = { + name: 'w:tc', + elements: [ + { + name: 'w:tcPr', + elements: [ + { + name: 'w:cnfStyle', + attributes: { + 'w:val': '001000000000', + 'w:firstRow': '0', + 'w:lastRow': '0', + 'w:firstColumn': '1', + 'w:lastColumn': '0', + }, + }, + ], + }, + { name: 'w:p' }, + ], + }; + const row1 = { name: 'w:tr', elements: [cellNode] }; + const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }] }; + const table = { name: 'w:tbl', elements: [row1, row2] }; + + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => []) }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row: row1, + rowBorders: {}, + baseTableBorders: null, + rowCnfStyle: { firstRow: true }, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90, 100], + rowIndex: 0, + totalRows: 2, + totalColumns: 2, + _referencedStyles: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '156082' }, + }, + }, + }, + }); + + // firstRow shading should apply despite cell cnfStyle having firstRow=0 + expect(out.attrs.background).toEqual({ color: '156082' }); + }); + + it('skips firstRow conditional shading when tableLook disables it', () => { + const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; + const row = { name: 'w:tr', elements: [cellNode] }; + const table = { name: 'w:tbl', elements: [row] }; + + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => []) }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row, + rowBorders: {}, + baseTableBorders: null, + tableLook: { firstRow: false }, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90], + rowIndex: 0, + totalRows: 1, + totalColumns: 1, + _referencedStyles: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '4472C4' }, + }, + }, + }, + }); + + // firstRow shading should be skipped; no other shading source available + expect(out.attrs.background).toBeUndefined(); + }); + it('applies firstRow/firstCol conditional borders from referenced styles', () => { const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; const row1 = { name: 'w:tr', elements: [cellNode] }; From fd59a8ab9a9b8492738c2261f4038c4b5c92fab4 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 9 Feb 2026 20:05:21 -0300 Subject: [PATCH 2/6] ci(fix): ecma spec review --- .github/workflows/spec-review.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/spec-review.yml b/.github/workflows/spec-review.yml index ced5d2d330..9e08b4955f 100644 --- a/.github/workflows/spec-review.yml +++ b/.github/workflows/spec-review.yml @@ -62,7 +62,9 @@ jobs: gh pr diff ${{ github.event.pull_request.number }} > /tmp/pr-diff.txt # Run Claude Code review - REVIEW=$(claude --print "Review this PR for OOXML spec compliance. + REVIEW=$(claude --print \ + --allowedTools "mcp__ecma-spec__search_ecma_spec,mcp__ecma-spec__get_section,mcp__ecma-spec__list_parts" \ + "Review this PR for OOXML spec compliance. Context: These are OOXML element handlers (XML ↔ ProseMirror JSON converters). From 7ea8796a298ba221c8a6902656404839d67282f2 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 10 Feb 2026 10:51:08 -0300 Subject: [PATCH 3/6] fix(style-engine): resolve table cell shading from style cascade instead of baking into import --- .../pm-adapter/src/converters/table.ts | 22 +- .../layout-engine/style-engine/src/cascade.ts | 3 +- .../style-engine/src/ooxml/index.test.ts | 129 ++++++++++ .../style-engine/src/ooxml/index.ts | 56 ++++- .../helpers/legacy-handle-table-cell-node.js | 68 +----- .../legacy-handle-table-cell-node.test.js | 223 ------------------ 6 files changed, 206 insertions(+), 295 deletions(-) diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 1534d15c1d..00e944e4fe 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -41,7 +41,7 @@ import { applySdtMetadataToParagraphBlocks, applySdtMetadataToTableBlock, } from '../sdt/index.js'; -import { TableProperties } from '@superdoc/style-engine/ooxml'; +import { TableProperties, resolveTableCellProperties } from '@superdoc/style-engine/ooxml'; type TableParserDependencies = { nextBlockId: BlockIdGenerator; @@ -185,7 +185,17 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { // Table cells can contain paragraphs, images/drawings, structured content blocks, and nested tables. const blocks: (ParagraphBlock | ImageBlock | DrawingBlock | TableBlock)[] = []; + // Resolve table cell properties from the style cascade (wholeTable → bands → conditional → inline) + const inlineTcProps = cellNode.attrs?.tableCellProperties as Record | undefined; + const tableInfo = tableProperties ? { tableProperties, rowIndex, cellIndex, numCells, numRows } : undefined; + const resolvedTcProps = resolveTableCellProperties( + inlineTcProps as Parameters[0], + tableInfo, + context.converterContext?.translatedLinkedStyles, + ); + // Extract cell background color for auto text color resolution + // Priority: inline background attr > resolved style shading const cellBackground = cellNode.attrs?.background as { color?: string } | undefined; let cellBackgroundColor: string | undefined; if (cellBackground && typeof cellBackground.color === 'string') { @@ -198,6 +208,13 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { } } } + // Fall back to resolved style shading if no inline background + if (!cellBackgroundColor && resolvedTcProps?.shading?.fill) { + const fill = resolvedTcProps.shading.fill; + if (fill !== 'auto') { + cellBackgroundColor = fill.startsWith('#') ? fill : `#${fill}`; + } + } // Create enhanced converter context with table style paragraph props for the style cascade // This allows paragraphs inside table cells to inherit table style's pPr @@ -416,6 +433,9 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { if (background && typeof background.color === 'string') { const bgColor = background.color; cellAttrs.background = bgColor.startsWith('#') ? bgColor : `#${bgColor}`; + } else if (cellBackgroundColor) { + // Use resolved style background when no inline background is set + cellAttrs.background = cellBackgroundColor; } const tableCellProperties = cellNode.attrs?.tableCellProperties; diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index bc4c1fe77b..293dde176c 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -11,8 +11,9 @@ */ import { ParagraphProperties, RunFontFamilyProperties, RunProperties } from './ooxml/types'; +import type { TableCellProperties } from './ooxml/styles-types'; -export type PropertyObject = ParagraphProperties | RunProperties; +export type PropertyObject = ParagraphProperties | RunProperties | TableCellProperties; /** * Performs a deep merge on an ordered list of property objects. diff --git a/packages/layout-engine/style-engine/src/ooxml/index.test.ts b/packages/layout-engine/style-engine/src/ooxml/index.test.ts index d47944ae13..1fd091625d 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.test.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.test.ts @@ -6,6 +6,7 @@ import { resolveRunProperties, resolveParagraphProperties, resolveCellStyles, + resolveTableCellProperties, type OoxmlResolverParams, } from './index.js'; @@ -414,3 +415,131 @@ describe('ooxml - resolveCellStyles', () => { expect(result).toEqual([{ fontSize: 10 }, { fontSize: 50 }]); }); }); + +describe('ooxml - resolveTableCellProperties', () => { + const gridTable4Styles = { + ...emptyStyles, + styles: { + 'GridTable4-Accent1': { + type: 'table', + tableProperties: { tableStyleRowBandSize: 1, tableStyleColBandSize: 1 }, + tableStyleProperties: { + firstRow: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: '156082' }, + borders: { top: { val: 'single', color: '156082', size: 4 } }, + }, + }, + band1Horz: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: 'C1E4F5' }, + }, + }, + wholeTable: { + tableCellProperties: { + shading: { val: 'clear', color: 'auto', fill: 'EEEEEE' }, + }, + }, + }, + }, + }, + }; + + it('resolves firstRow shading from table style', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 1, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: '156082' }); + }); + + it('resolves band1Horz shading for data rows', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 1, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + // band1Horz overrides wholeTable + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'C1E4F5' }); + }); + + it('falls back to wholeTable when no band matches', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: true, noVBand: true }, + }, + rowIndex: 1, + cellIndex: 1, + numRows: 3, + numCells: 4, + }; + const result = resolveTableCellProperties(null, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'EEEEEE' }); + }); + + it('inline cell shading overrides style shading', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const inlineProps = { shading: { val: 'clear', color: 'auto', fill: 'FF0000' } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', color: 'auto', fill: 'FF0000' }); + }); + + it('returns inline props when no table style exists', () => { + const tableInfo = { + tableProperties: {}, + rowIndex: 0, + cellIndex: 0, + numRows: 1, + numCells: 1, + }; + const inlineProps = { shading: { val: 'clear', fill: 'AABBCC' } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + expect(result.shading).toEqual({ val: 'clear', fill: 'AABBCC' }); + }); + + it('returns empty object when no props available', () => { + const result = resolveTableCellProperties(null, null, null); + expect(result).toEqual({}); + }); + + it('merges borders from style and inline', () => { + const tableInfo = { + tableProperties: { + tableStyleId: 'GridTable4-Accent1', + tblLook: { firstRow: true, noHBand: false, noVBand: true }, + }, + rowIndex: 0, + cellIndex: 0, + numRows: 3, + numCells: 4, + }; + const inlineProps = { borders: { bottom: { val: 'double', color: '000000', size: 8 } } }; + const result = resolveTableCellProperties(inlineProps, tableInfo, gridTable4Styles); + // firstRow style provides top border, inline provides bottom border - both should be present + expect(result.borders?.top).toEqual({ val: 'single', color: '156082', size: 4 }); + expect(result.borders?.bottom).toEqual({ val: 'double', color: '000000', size: 8 }); + }); +}); diff --git a/packages/layout-engine/style-engine/src/ooxml/index.ts b/packages/layout-engine/style-engine/src/ooxml/index.ts index 16c6c97247..44dafd8bc6 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.ts @@ -9,7 +9,13 @@ import { combineIndentProperties, combineProperties, combineRunProperties } from import type { PropertyObject } from '../cascade.js'; import type { ParagraphProperties, RunProperties } from './types.ts'; import type { NumberingProperties } from './numbering-types.ts'; -import type { StylesDocumentProperties, TableStyleType, TableProperties, TableLookProperties } from './styles-types.ts'; +import type { + StylesDocumentProperties, + TableStyleType, + TableProperties, + TableLookProperties, + TableCellProperties, +} from './styles-types.ts'; export { combineIndentProperties, combineProperties, combineRunProperties }; export type { PropertyObject }; @@ -259,7 +265,7 @@ export function resolveStyleChain( return combineProperties(styleChain); } -export function getNumberingProperties( +export function getNumberingProperties( propertyType: 'paragraphProperties' | 'runProperties', params: OoxmlResolverParams, ilvl: number, @@ -356,7 +362,7 @@ export function resolveDocxFontFamily( } export function resolveCellStyles( - propertyType: 'paragraphProperties' | 'runProperties', + propertyType: 'paragraphProperties' | 'runProperties' | 'tableCellProperties', tableInfo: TableInfo | null | undefined, translatedLinkedStyles: StylesDocumentProperties, ): T[] { @@ -388,6 +394,41 @@ export function resolveCellStyles( return cellStyleProps; } +/** + * Resolve table cell properties (shading, borders, margins) by cascading + * conditional table style properties with inline cell properties. + * + * Cascade order (low → high priority): + * wholeTable → bands → firstRow/lastRow/firstCol/lastCol → corner cells → inline + */ +export function resolveTableCellProperties( + inlineProps: TableCellProperties | null | undefined, + tableInfo: TableInfo | null | undefined, + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): TableCellProperties { + if (!translatedLinkedStyles) { + return (inlineProps ?? {}) as TableCellProperties; + } + + const cellStyleProps = resolveCellStyles( + 'tableCellProperties', + tableInfo, + translatedLinkedStyles, + ); + + if (cellStyleProps.length === 0) { + return (inlineProps ?? {}) as TableCellProperties; + } + + // Cascade: style properties (low→high) then inline wins last + const chain: TableCellProperties[] = [...cellStyleProps]; + if (inlineProps && Object.keys(inlineProps).length > 0) { + chain.push(inlineProps); + } + + return combineProperties(chain, { fullOverrideProps: ['shading'] }); +} + function determineCellStyleTypes( tblLook: TableLookProperties | null | undefined, rowIndex: number, @@ -401,8 +442,13 @@ function determineCellStyleTypes( const normalizedRowBandSize = rowBandSize > 0 ? rowBandSize : 1; const normalizedColBandSize = colBandSize > 0 ? colBandSize : 1; - const rowGroup = Math.floor(rowIndex / normalizedRowBandSize); - const colGroup = Math.floor(cellIndex / normalizedColBandSize); + + // Per ECMA-376, banding excludes header/footer rows and first/last columns. + // Offset the index so the first data row/column starts at band1. + const bandRowIndex = Math.max(0, rowIndex - (tblLook?.firstRow ? 1 : 0)); + const bandColIndex = Math.max(0, cellIndex - (tblLook?.firstColumn ? 1 : 0)); + const rowGroup = Math.floor(bandRowIndex / normalizedRowBandSize); + const colGroup = Math.floor(bandColIndex / normalizedColBandSize); if (!tblLook?.noHBand) { if (rowGroup % 2 === 0) { diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js index 45508b22a7..6ad889fbbc 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js @@ -99,28 +99,11 @@ export function handleTableCellNode({ } // Background - // Priority cascade: inline cell shading > conditional style shading > table-level shading - let backgroundColor = resolveShadingFillColor(tableCellProperties.shading); - - if (!backgroundColor) { - backgroundColor = resolveConditionalShading({ - tableLook, - rowCnfStyle, - tableCellProperties, - isFirstRow, - isLastRow, - isFirstColumn, - isLastColumn, - referencedStyles, - }); - } - - if (!backgroundColor) { - backgroundColor = resolveShadingFillColor(tableProperties?.shading); - } - + const backgroundColor = + resolveShadingFillColor(tableCellProperties.shading) ?? resolveShadingFillColor(tableProperties?.shading); const background = { color: backgroundColor }; + // TODO: Do we need other background attrs? if (background.color) attributes['background'] = background; // Vertical Align @@ -284,51 +267,6 @@ function normalizeTableCellContent(content, editor) { return normalized; } -/** - * Resolve cell background color from conditional table style shading. - * Uses rowCnfStyle for row flags (firstRow/lastRow) and cellCnfStyle for column flags, - * since cell cnfStyle only carries column-level annotations. - */ -const resolveConditionalShading = ({ - tableLook, - rowCnfStyle, - tableCellProperties, - isFirstRow, - isLastRow, - isFirstColumn, - isLastColumn, - referencedStyles, -}) => { - const cellCnfStyle = tableCellProperties?.cnfStyle; - const getFlag = (source, flag) => - source && Object.prototype.hasOwnProperty.call(source, flag) ? source[flag] : undefined; - const isRowStyleEnabled = (flag) => getFlag(rowCnfStyle, flag) ?? getFlag(tableLook, flag) ?? true; - const isColStyleEnabled = (flag) => - getFlag(cellCnfStyle, flag) ?? getFlag(rowCnfStyle, flag) ?? getFlag(tableLook, flag) ?? true; - - const getShading = (styleVariant) => resolveShadingFillColor(styleVariant?.tableCellProperties?.shading); - - if (isFirstRow && isRowStyleEnabled('firstRow')) { - const color = getShading(referencedStyles?.firstRow); - if (color) return color; - } - if (isLastRow && isRowStyleEnabled('lastRow')) { - const color = getShading(referencedStyles?.lastRow); - if (color) return color; - } - if (isFirstColumn && isColStyleEnabled('firstColumn')) { - const color = getShading(referencedStyles?.firstCol); - if (color) return color; - } - if (isLastColumn && isColStyleEnabled('lastColumn')) { - const color = getShading(referencedStyles?.lastCol); - if (color) return color; - } - - // wholeTable fallback - return getShading(referencedStyles?.wholeTable) ?? null; -}; - const processInlineCellBorders = (borders, rowBorders) => { if (!borders) return null; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js index 829f2feeed..c1a48cd91a 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js @@ -191,229 +191,6 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.background).toEqual({ color: '808080' }); }); - it('applies firstRow conditional shading from referenced styles', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row1 = { name: 'w:tr', elements: [cellNode] }; - const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }] }; - const table = { name: 'w:tbl', elements: [row1, row2] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row: row1, - rowBorders: {}, - baseTableBorders: null, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 0, - totalRows: 2, - totalColumns: 2, - _referencedStyles: { - firstRow: { - tableCellProperties: { - shading: { val: 'clear', color: 'auto', fill: '4472C4' }, - }, - }, - }, - }); - - expect(out.attrs.background).toEqual({ color: '4472C4' }); - }); - - it('prefers inline cell shading over conditional style shading', () => { - const cellNode = { - name: 'w:tc', - elements: [ - { - name: 'w:tcPr', - elements: [{ name: 'w:shd', attributes: { 'w:fill': 'FF0000' } }], - }, - { name: 'w:p' }, - ], - }; - const row = { name: 'w:tr', elements: [cellNode] }; - const table = { name: 'w:tbl', elements: [row] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row, - rowBorders: {}, - baseTableBorders: null, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90], - rowIndex: 0, - totalRows: 1, - totalColumns: 1, - _referencedStyles: { - firstRow: { - tableCellProperties: { - shading: { val: 'clear', color: 'auto', fill: '4472C4' }, - }, - }, - }, - }); - - // Inline shading (FF0000) wins over conditional style shading (4472C4) - expect(out.attrs.background).toEqual({ color: 'FF0000' }); - }); - - it('applies wholeTable shading as fallback when no positional style matches', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row = { name: 'w:tr', elements: [cellNode] }; - const table = { name: 'w:tbl', elements: [row] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row, - rowBorders: {}, - baseTableBorders: null, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90], - rowIndex: 0, - totalRows: 1, - totalColumns: 1, - _referencedStyles: { - wholeTable: { - tableCellProperties: { - shading: { val: 'clear', color: 'auto', fill: 'CCCCCC' }, - }, - }, - }, - }); - - expect(out.attrs.background).toEqual({ color: 'CCCCCC' }); - }); - - it('applies firstRow shading even when cell cnfStyle sets firstRow=false (column-only flag)', () => { - // Reproduces the real DOCX scenario: cell (0,0) has cnfStyle with firstRow=0, firstColumn=1 - // but the row's cnfStyle has firstRow=1. Row-level flags should determine row shading. - const cellNode = { - name: 'w:tc', - elements: [ - { - name: 'w:tcPr', - elements: [ - { - name: 'w:cnfStyle', - attributes: { - 'w:val': '001000000000', - 'w:firstRow': '0', - 'w:lastRow': '0', - 'w:firstColumn': '1', - 'w:lastColumn': '0', - }, - }, - ], - }, - { name: 'w:p' }, - ], - }; - const row1 = { name: 'w:tr', elements: [cellNode] }; - const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }] }; - const table = { name: 'w:tbl', elements: [row1, row2] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row: row1, - rowBorders: {}, - baseTableBorders: null, - rowCnfStyle: { firstRow: true }, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 0, - totalRows: 2, - totalColumns: 2, - _referencedStyles: { - firstRow: { - tableCellProperties: { - shading: { val: 'clear', color: 'auto', fill: '156082' }, - }, - }, - }, - }); - - // firstRow shading should apply despite cell cnfStyle having firstRow=0 - expect(out.attrs.background).toEqual({ color: '156082' }); - }); - - it('skips firstRow conditional shading when tableLook disables it', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row = { name: 'w:tr', elements: [cellNode] }; - const table = { name: 'w:tbl', elements: [row] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row, - rowBorders: {}, - baseTableBorders: null, - tableLook: { firstRow: false }, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90], - rowIndex: 0, - totalRows: 1, - totalColumns: 1, - _referencedStyles: { - firstRow: { - tableCellProperties: { - shading: { val: 'clear', color: 'auto', fill: '4472C4' }, - }, - }, - }, - }); - - // firstRow shading should be skipped; no other shading source available - expect(out.attrs.background).toBeUndefined(); - }); - it('applies firstRow/firstCol conditional borders from referenced styles', () => { const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; const row1 = { name: 'w:tr', elements: [cellNode] }; From 50c2bade92a9f85d3527a6fe53f3607d75a47ef1 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 10 Feb 2026 15:59:14 -0300 Subject: [PATCH 4/6] chore(visual-testing): add table cell shading story and R2 wrangler scripts --- .gitignore | 2 +- devtools/visual-testing/package.json | 3 + devtools/visual-testing/pnpm-lock.yaml | 4 +- .../scripts/r2-sync-registry.sh | 68 +++++++++++++++++++ .../importing/table-style-cell-shading.ts | 24 +++++++ 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100755 devtools/visual-testing/scripts/r2-sync-registry.sh create mode 100644 devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts diff --git a/.gitignore b/.gitignore index f51268092b..c80a1729e6 100644 --- a/.gitignore +++ b/.gitignore @@ -64,7 +64,7 @@ reports/ # Planning and progress tracking documents **/plan/progress.md - +wrangler/ codex.config.json perf-baseline-results.json .claude diff --git a/devtools/visual-testing/package.json b/devtools/visual-testing/package.json index aaae51e506..26aad745f3 100644 --- a/devtools/visual-testing/package.json +++ b/devtools/visual-testing/package.json @@ -5,6 +5,7 @@ "type": "module", "packageManager": "pnpm@10.25.0", "scripts": { + "postinstall": "pnpm exec playwright install", "dev": "pnpm --filter @superdoc-testing/harness dev", "build": "pnpm --filter @superdoc-testing/harness build", "preview": "pnpm --filter @superdoc-testing/harness preview", @@ -24,6 +25,8 @@ "get-corpus": "tsx scripts/get-corpus.ts", "get-docx": "tsx scripts/get-docx.ts", "upload": "tsx scripts/upload-corpus-doc.ts", + "r2:upload": "wrangler r2 object put", + "r2:sync-registry": "bash scripts/r2-sync-registry.sh", "filters": "tsx scripts/list-filters.ts", "clear:all": "read -p 'This will delete all baselines, screenshots, and results. Are you sure? (y/N) ' confirm && [ \"$confirm\" = 'y' ] && rm -rf baselines baselines-interactions screenshots results && echo 'Cleared!' || echo 'Aborted.'", "test": "vitest", diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index 7635a575e6..0f59692d0e 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2062,8 +2062,8 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-ujI4h2Ru+d9t8xIZmdJazSVhaDA6l2QS3IK2MuThzl0MqSCGjxAKDlfvTeyWA/7WLxQkQzxvY+V8/lCo0+ysqg==, tarball: file:../../packages/superdoc/superdoc.tgz} - version: 1.10.1-next.4 + resolution: {integrity: sha512-riLJi3/uPgpUeuHBaisa67AfG0u/KtpSqzIx5TuJbMarSCmwDsasUgIWzE8+wGE2Wg/TwjICrK+ZEQwVlDAjXQ==, tarball: file:../../packages/superdoc/superdoc.tgz} + version: 1.11.0 peerDependencies: '@hocuspocus/provider': ^2.13.6 pdfjs-dist: '>=4.3.136 <=4.6.82' diff --git a/devtools/visual-testing/scripts/r2-sync-registry.sh b/devtools/visual-testing/scripts/r2-sync-registry.sh new file mode 100755 index 0000000000..9fd4914b41 --- /dev/null +++ b/devtools/visual-testing/scripts/r2-sync-registry.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Add an uploaded doc to the R2 corpus registry. +# +# Usage: +# pnpm r2:sync-registry tables/cell-shading.docx +# +# Prerequisites: +# wrangler login (one-time browser auth) +# jq (brew install jq) + +set -euo pipefail + +BUCKET="docx-test-corpus" +TMP_DIR="$(mktemp -d)" +REGISTRY_FILE="${TMP_DIR}/registry.json" + +cleanup() { rm -rf "$TMP_DIR"; } +trap cleanup EXIT + +KEY="${1:?Usage: pnpm r2:sync-registry (e.g. tables/cell-shading.docx)}" + +# Download current registry +echo "Fetching registry.json..." +if wrangler r2 object get "${BUCKET}/registry.json" --remote --file "$REGISTRY_FILE" 2>/dev/null; then + echo "Found existing registry." +else + echo "No existing registry — starting fresh." + echo '{"updated_at":"","docs":[]}' > "$REGISTRY_FILE" +fi + +# Derive fields from key +FILENAME="$(basename "$KEY")" +GROUP="$(dirname "$KEY")" +if [ "$GROUP" = "." ]; then + GROUP="" +fi +DOC_ID="$(echo "$KEY" | sed 's/\.docx$//' | tr '/' '-' | tr -c 'A-Za-z0-9._-' '-' | tr '[:upper:]' '[:lower:]' | sed 's/^-//;s/-$//')" + +# Update registry: add or replace entry matching this relative_path +UPDATED="$(jq \ + --arg key "$KEY" \ + --arg doc_id "$DOC_ID" \ + --arg filename "$FILENAME" \ + --arg group "$GROUP" \ + ' + .docs |= ( + [.[] | select(.relative_path != $key)] + + [{ + doc_id: $doc_id, + doc_rev: "manual", + filename: $filename, + group: (if $group == "" then null else $group end), + relative_path: $key + }] + | sort_by(.relative_path) + ) + | .updated_at = (now | todate) + ' "$REGISTRY_FILE")" + +echo "$UPDATED" | jq '.' > "$REGISTRY_FILE" + +DOC_COUNT="$(echo "$UPDATED" | jq '.docs | length')" +echo "Registry: ${DOC_COUNT} doc(s). Added: ${KEY} (doc_id=${DOC_ID})" + +# Upload updated registry +echo "Uploading registry.json..." +wrangler r2 object put "${BUCKET}/registry.json" --remote --file "$REGISTRY_FILE" --content-type "application/json" +echo "Done." diff --git a/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts b/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts new file mode 100644 index 0000000000..554103c5c1 --- /dev/null +++ b/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts @@ -0,0 +1,24 @@ +import { defineStory } from '@superdoc-testing/helpers'; + +const WAIT_MS = 400; + +export default defineStory({ + name: 'table-style-cell-shading', + description: + 'Test that table cell shading from conditional table styles (GridTable4-Accent1) renders correctly via the style-engine cascade.', + tickets: ['SD-1833'], + startDocument: 'tables/cell-shading.docx', + layout: true, + hideCaret: true, + hideSelection: true, + + async run(page, helpers): Promise { + const { step, waitForStable, milestone } = helpers; + + await step('Verify document loads with table style shading', async () => { + await page.waitForSelector('.ProseMirror', { timeout: 30_000 }); + await waitForStable(WAIT_MS); + await milestone('loaded', 'Table cells should show conditional shading (firstRow #156082, band1Horz #C1E4F5)'); + }); + }, +}); From 86ff313df0e2d195f21d6786134df21f9163d3ab Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 10 Feb 2026 18:06:29 -0300 Subject: [PATCH 5/6] chore(visual-testing): remove interaction story for cell shading (visual test is the right fit) --- .../importing/table-style-cell-shading.ts | 24 ------------------- 1 file changed, 24 deletions(-) delete mode 100644 devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts diff --git a/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts b/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts deleted file mode 100644 index 554103c5c1..0000000000 --- a/devtools/visual-testing/tests/interactions/stories/importing/table-style-cell-shading.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { defineStory } from '@superdoc-testing/helpers'; - -const WAIT_MS = 400; - -export default defineStory({ - name: 'table-style-cell-shading', - description: - 'Test that table cell shading from conditional table styles (GridTable4-Accent1) renders correctly via the style-engine cascade.', - tickets: ['SD-1833'], - startDocument: 'tables/cell-shading.docx', - layout: true, - hideCaret: true, - hideSelection: true, - - async run(page, helpers): Promise { - const { step, waitForStable, milestone } = helpers; - - await step('Verify document loads with table style shading', async () => { - await page.waitForSelector('.ProseMirror', { timeout: 30_000 }); - await waitForStable(WAIT_MS); - await milestone('loaded', 'Table cells should show conditional shading (firstRow #156082, band1Horz #C1E4F5)'); - }); - }, -}); From 4cc0127bd0977d7fe6d1ee8d68533e491ba04d39 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Tue, 10 Feb 2026 18:09:38 -0300 Subject: [PATCH 6/6] chore: update lock --- devtools/visual-testing/pnpm-lock.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index 0f59692d0e..3a5de97f75 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2062,8 +2062,8 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-riLJi3/uPgpUeuHBaisa67AfG0u/KtpSqzIx5TuJbMarSCmwDsasUgIWzE8+wGE2Wg/TwjICrK+ZEQwVlDAjXQ==, tarball: file:../../packages/superdoc/superdoc.tgz} - version: 1.11.0 + resolution: {integrity: sha512-uv32JEnNC45h5pivFHQsOOP1rUnUucwHy9fGnWVwr483QNYDJkAXqbkKZVZPizj8+VBxX/ETiLP0tSpG4kXUfw==, tarball: file:../../packages/superdoc/superdoc.tgz} + version: 1.0.2 peerDependencies: '@hocuspocus/provider': ^2.13.6 pdfjs-dist: '>=4.3.136 <=4.6.82'