Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 489b04e27c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js
Outdated
Show resolved
Hide resolved
|
Based on my review of the code and understanding of OOXML specifications, I can now provide a comprehensive review. Let me analyze the key changes: Status: PASS I've reviewed this PR for OOXML spec compliance, focusing on the SummaryThe changes correctly implement table cell spacing according to ECMA-376 specifications. The implementation properly handles the Key Changes Reviewed1. tbl-translator.js:90 - Cell spacing conversion['tableCellSpacing', ({ value, type }) => ({ value: twipsToPixels(value), type })]✅ Correct: The spec defines 2. tbl-translator.test.js:154 - Test expectation updateexpect(result.attrs.tableCellSpacing).toEqual({ value: 0.5, type: 'dxa' });✅ Correct: With input UPDATE: Looking at the diff, the original test returned 3. legacy-handle-table-cell-node.js:58,308 - Border spacing logichasBorderSpacing: !!tableProperties?.tableCellSpacing✅ Correct: According to ECMA-376, when 4. contracts/index.ts - Type definition updatecellSpacing?: CellSpacing;
// where CellSpacing = { type: string; value: number; }✅ Correct: Properly represents OOXML measurement with both value and type (e.g., OOXML Spec ComplianceThe implementation correctly follows ECMA-376 Part 1 §17.4.19 (
For detailed spec information, see: https://ooxml.dev/spec?q=tblCellSpacing No Issues FoundAll attributes, elements, and default values align with the ECMA-376 specification. The conversion logic properly handles the measurement type pattern used consistently across other table properties. |
caio-pizzol
left a comment
There was a problem hiding this comment.
nice work @VladaHarbour!
just a few nit pick comments
packages/layout-engine/painters/dom/src/table/renderTableFragment.ts
Outdated
Show resolved
Hide resolved
packages/layout-engine/painters/dom/src/table/renderTableFragment.ts
Outdated
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js
Show resolved
Hide resolved
packages/layout-engine/painters/dom/src/table/renderTableFragment.ts
Outdated
Show resolved
Hide resolved
1af9e81 to
9702ea5
Compare
|
Hi @harbournick! After rebasing with the latest main unit tests fail (for main branch as well) |
4de4869 to
ac681e7
Compare
caio-pizzol
left a comment
There was a problem hiding this comment.
LGTM - let's just upload file to rendering
ac681e7 to
0c2e2d1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2e2d1704
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Visual diffs detectedPixel differences were found in visual tests. This is not blocking — reproduce locally with |
0036ee6 to
35b3721
Compare
35b3721 to
b0327e4
Compare
|
Hi @caio-pizzol! Added some fixes that should address AI Risk Review comment. Just wondering whether critical risk label can be removed? |
Hey @VladaHarbour! Thanks - critical label is more as an assessment for the code reviewer to be-aware the PR touches important parts of the codebase (not really about risk of your code) |
b0327e4 to
7ed6a7a
Compare
|
🎉 This PR is included in superdoc v1.17.0-next.17 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.2.0-next.12 The release is available on GitHub release |
# [1.17.0](v1.16.0...v1.17.0) (2026-02-28) ### Bug Fixes * active track change ([#2163](#2163)) ([108c14d](108c14d)) * add currentTotalPages getter and pagination-update event ([#2202](#2202)) ([95b4579](95b4579)), closes [#958](#958) * always call resolveComment after custom TC bubble handlers (SD-2049) ([#2204](#2204)) ([34fb4e0](34fb4e0)) * backward replace insert text ([#2172](#2172)) ([66f0849](66f0849)) * **collaboration:** deduplicate updateYdocDocxData during replaceFile (SD-1920) ([#2162](#2162)) ([52962fc](52962fc)) * **comments:** cross-page collision avoidance for floating comment bubbles (SD-1998) ([#2180](#2180)) ([6cfbeca](6cfbeca)) * **comments:** remove synchronous dispatch from plugin apply() (SD-1940) ([#2157](#2157)) ([887175b](887175b)) * **css:** scope ProseMirror CSS to prevent bleeding into host apps (SD-1850) ([#2134](#2134)) ([b9d98fa](b9d98fa)) * document-api improvements, plan mode, query.match, mutations ([6221580](6221580)) * **document-api:** delete table cell fix ([#2209](#2209)) ([5e5c43f](5e5c43f)) * **document-api:** distribute columns command fixes ([#2207](#2207)) ([8f4eaf7](8f4eaf7)) * **document-api:** fix cell shading in document api ([#2215](#2215)) ([456f60e](456f60e)) * **document-api:** insert table cell ([#2210](#2210)) ([357ee90](357ee90)) * **document-api:** plan-engine reliability fixes and error diagnostics ([#2185](#2185)) ([abfd81b](abfd81b)) * **document-api:** split table cell command ([#2217](#2217)) ([0b3e2b4](0b3e2b4)) * **document-api:** split table command ([#2214](#2214)) ([ec31699](ec31699)) * **editor:** render styles applied inside SDT fields (SD-2011) ([#2188](#2188)) ([9c34be3](9c34be3)) * **editor:** selection highlight flickers when dragging across mark boundaries (SD-2024) ([#2205](#2205)) ([ba03e76](ba03e76)) * extract duplicate block identity normalization from docxImporter ([7f7ff93](7f7ff93)) * improve backspace behavior near run boundaries for tracked changes ([#2175](#2175)) ([6c9c7a3](6c9c7a3)) * **layout:** per-section footer constraints for multi-section docs (SD-1837) ([#2022](#2022)) ([e11acc5](e11acc5)) * normalize review namespace into trackChanges, harden input validation ([33e907b](33e907b)) * outside click for toolbar dropdown ([#2174](#2174)) ([5f859c7](5f859c7)) * preserve line spacing and indentation on Google Docs paste ([#2183](#2183)) ([b9a7357](b9a7357)), closes [#2151](#2151) * **shapes:** render grouped DrawingML shapes with custom geometry (SD-1877) ❇️ ([#2105](#2105)) ([14985a5](14985a5)) * support cell spacing ([#1879](#1879)) ([1639967](1639967)) * **tables:** expand auto-width tables to fill available page width ([#2109](#2109)) ([15f36bc](15f36bc)) * text highlight on export ([#2189](#2189)) ([9cbd022](9cbd022)) * track highlight changes ([#2192](#2192)) ([e164625](e164625)) * undo/redo actions ([#2161](#2161)) ([495e92f](495e92f)) ### Features * allow custom accept/reject handlers for TC bubbles ([#1921](#1921)) ([e30abf6](e30abf6)) * **document-api:** add format operations font size alignment color font family ([#2179](#2179)) ([f19c688](f19c688)) * **document-api:** add plan-based mutation engine with query.match and style capture ([#2160](#2160)) ([365293a](365293a)) * **document-api:** doc default initial styles ([#2184](#2184)) ([f25e41f](f25e41f)) * **document-api:** include anchored text in comments list response ([#2177](#2177)) ([b3a2912](b3a2912)) * **document-api:** inline formatting parity core end-to-end ([#2197](#2197)) ([b405b03](b405b03)) * **document-api:** inline formatting rpr parity ([#2198](#2198)) ([41ab771](41ab771)) * **document-api:** section commands ([#2199](#2199)) ([ec4abe3](ec4abe3)) * **document-api:** support deleting entire block nodes not only text ([#2181](#2181)) ([2897246](2897246)) * **document-api:** table of contents commands ([#2200](#2200)) ([baa72c4](baa72c4)) * **document-api:** tables namespace and commands ([#2182](#2182)) ([b80ee31](b80ee31)) * **markdown:** add markdown override to sdk, improve conversion ([#2196](#2196)) ([04a1c71](04a1c71)) * preserve w:view setting through DOCX round-trip ([#2190](#2190)) ([48b4210](48b4210)), closes [#2070](#2070) * **track-changes:** clear comment bubbles when bulk accept or reject TCs ([#2159](#2159)) ([27fbe8e](27fbe8e)) ### Performance Improvements * **comments:** batch tracked change comment creation on load ([#2166](#2166)) ([0c2eca5](0c2eca5)) * **comments:** batch tracked change creation and virtualize floating bubbles (SD-1997) ([#2168](#2168)) ([70fd7d9](70fd7d9))
Adds cell spacing support