Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/spec-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ reports/

# Planning and progress tracking documents
**/plan/progress.md

wrangler/
codex.config.json
perf-baseline-results.json
.claude
Expand Down
3 changes: 3 additions & 0 deletions devtools/visual-testing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions devtools/visual-testing/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

68 changes: 68 additions & 0 deletions devtools/visual-testing/scripts/r2-sync-registry.sh
Original file line number Diff line number Diff line change
@@ -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 <relative-path> (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."
22 changes: 21 additions & 1 deletion packages/layout-engine/pm-adapter/src/converters/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<string, unknown> | undefined;
const tableInfo = tableProperties ? { tableProperties, rowIndex, cellIndex, numCells, numRows } : undefined;
const resolvedTcProps = resolveTableCellProperties(
inlineTcProps as Parameters<typeof resolveTableCellProperties>[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;
Comment thread
luccas-harbour marked this conversation as resolved.
let cellBackgroundColor: string | undefined;
if (cellBackground && typeof cellBackground.color === 'string') {
Expand All @@ -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}`;
Comment thread
caio-pizzol marked this conversation as resolved.
}
}

// 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
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/layout-engine/style-engine/src/cascade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
129 changes: 129 additions & 0 deletions packages/layout-engine/style-engine/src/ooxml/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
resolveRunProperties,
resolveParagraphProperties,
resolveCellStyles,
resolveTableCellProperties,
type OoxmlResolverParams,
} from './index.js';

Expand Down Expand Up @@ -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 });
});
});
Loading
Loading