Skip to content

docs(direction): document visual mirror rule + drop dead isRtl args#3273

Merged
caio-pizzol merged 1 commit into
mainfrom
caio-pizzol/SD-3134-visual-mirror-cleanup
May 14, 2026
Merged

docs(direction): document visual mirror rule + drop dead isRtl args#3273
caio-pizzol merged 1 commit into
mainfrom
caio-pizzol/SD-3134-visual-mirror-cleanup

Conversation

@caio-pizzol
Copy link
Copy Markdown
Contributor

Follow-up to #3272 (SD-3134). Docs + dead-arg cleanup, no behavior change.

  • Add a "Visual mirror rule (table-visual RTL axis)" section to pm-adapter/src/direction/README.md. Spells out the three-condition gate (table-scoped + logical-side language + painter mirrors), the OOXML properties it covers (tblBorders/tcBorders/tcMar start/end, bidiVisual cell order, gridBefore/gridAfter), and the properties it does NOT cover (paragraph w:bidi, run w:rtl/w:dir/w:bdo, w:textDirection, numbering w:start, editing-side visual-to-logical mapping).
  • Drop dead isRtl args at three call sites in pm-adapter/src/converters/table.ts. The receivers already ignored them; the call-site args were a trap where a future agent might assume isRtl flowed through.
  • Tighten the existing "ignored" comments on extractCellBorders, extractCellPadding, and BorderConversionOptions to be definite and link to the new README section.

Why: SD-3134 risked being re-broken on the next bidiVisual feature someone adds. Without the rule documented somewhere persistent, future agents see the dead isRtl plumbing and assume the receiver reads it.

Extractor signatures (_options? params) intentionally kept to avoid churning ~6 test invocations across three "regardless of isRtl flag" tests. A broader API trim can happen later.

Tests: pm-adapter 213 pass (table-styles + borders + table).

@caio-pizzol caio-pizzol requested a review from a team as a code owner May 13, 2026 22:08
@linear
Copy link
Copy Markdown

linear Bot commented May 13, 2026

SD-3134

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@caio-pizzol caio-pizzol self-assigned this May 14, 2026
@caio-pizzol caio-pizzol force-pushed the caio-pizzol/SD-3134-visual-mirror-cleanup branch 2 times, most recently from a419207 to 5eb84cd Compare May 14, 2026 09:48
…SD-3134)

Cleanup PR stacked on the SD-3134 fix. No behavior change.

- Add a "Visual mirror rule (table-visual RTL axis)" section to
  pm-adapter/src/direction/README.md. Spells out the three-condition
  gate (table-scoped + logical-side language + painter mirrors),
  enumerates the OOXML properties it covers (tblBorders/tcBorders/
  tcMar start/end + bidiVisual cell order + gridBefore/After), and
  the properties it does NOT cover (paragraph w:bidi, run w:rtl/dir/
  bdo, w:textDirection, numbering w:start, editing-side visual-to-
  logical mapping). Future bidiVisual-axis features should follow
  the same pattern instead of re-introducing pre-mirrors.

- Remove dead isRtl args at three call sites in pm-adapter/src/
  converters/table.ts. The receivers in borders.ts already ignore
  the flag (the comments at lines 313 and 369 said so); the call
  sites were misleading. Removing them eliminates the trap where a
  future agent reads the call and assumes isRtl flows through.

- Tighten the existing "ignored" comments on extractCellBorders,
  extractCellPadding, and BorderConversionOptions to be definite
  ("isRtl is IGNORED") and to reference direction/README.md.

Extractor signatures keep the optional _options param for now to
avoid churning ~6 test invocations across the three "regardless of
isRtl flag" tests. A broader API trim can happen later.

Tests: pm-adapter 213 pass (table-styles + borders + table).

Stacked on caio-pizzol/SD-3134; retarget to main after that lands.
@caio-pizzol caio-pizzol force-pushed the caio-pizzol/SD-3134-visual-mirror-cleanup branch from 5eb84cd to ebf90b0 Compare May 14, 2026 09:56
caio-pizzol added a commit that referenced this pull request May 14, 2026
…umers (SD-3138)

Centralize the read of table visual direction (w:bidiVisual, ECMA-376
section 17.4.1) through one helper, mirroring the paragraph-axis pattern
already established for getParagraphInlineDirection.

- Add getTableVisualDirection(attrs) to @superdoc/contracts. Reads
  attrs.tableDirectionContext.visualDirection first (future-ready for
  when pm-adapter writes the resolved context onto TableAttrs), falls
  back to attrs.tableProperties.rightToLeft (or bidiVisual alias) for
  compatibility.
- Migrate three consumers off raw reads:
  - layout-engine/src/layout-table.ts (table-frame X positioning)
  - painters/dom/src/table/renderTableFragment.ts (visual mirror gate)
  - super-editor/.../tableBoundaryNavigation.js (RTL cursor nav)

Out of scope for this PR:
- pm-adapter wiring that populates TableAttrs.tableDirectionContext.
  Phase 1B - the helper is future-ready but the context isn't written
  yet, so the fallback path is exercised today.
- packages/super-editor/.../TableResizeOverlay.vue reads a different
  shape (tableMetadata.value.rtl); migration deferred to keep this PR
  scoped.

Companion to SD-3134 and PRs #3272/#3273/#3278; under SD-2771 Wave 3.

Tests:
- 14 unit tests for getTableVisualDirection (precedence, alias, no
  signal, null fallback).
- 174 painter table tests, 652 layout-engine tests, 189 super-editor
  table extension tests all pass.
@caio-pizzol caio-pizzol merged commit a81be59 into main May 14, 2026
65 checks passed
@caio-pizzol caio-pizzol deleted the caio-pizzol/SD-3134-visual-mirror-cleanup branch May 14, 2026 10:20
@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.90

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in vscode-ext v2.3.0-next.135

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in @superdoc-dev/react v1.2.0-next.133

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in superdoc-cli v0.8.0-next.105

The release is available on GitHub release

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in superdoc-sdk v1.8.0-next.89

@superdoc-bot
Copy link
Copy Markdown
Contributor

superdoc-bot Bot commented May 14, 2026

🎉 This PR is included in superdoc v1.30.0-next.87

The release is available on GitHub release

caio-pizzol added a commit that referenced this pull request May 14, 2026
…umers (SD-3138) (#3279)

Centralize the read of table visual direction (w:bidiVisual, ECMA-376
section 17.4.1) through one helper, mirroring the paragraph-axis pattern
already established for getParagraphInlineDirection.

- Add getTableVisualDirection(attrs) to @superdoc/contracts. Reads
  attrs.tableDirectionContext.visualDirection first (future-ready for
  when pm-adapter writes the resolved context onto TableAttrs), falls
  back to attrs.tableProperties.rightToLeft (or bidiVisual alias) for
  compatibility.
- Migrate three consumers off raw reads:
  - layout-engine/src/layout-table.ts (table-frame X positioning)
  - painters/dom/src/table/renderTableFragment.ts (visual mirror gate)
  - super-editor/.../tableBoundaryNavigation.js (RTL cursor nav)

Out of scope for this PR:
- pm-adapter wiring that populates TableAttrs.tableDirectionContext.
  Phase 1B - the helper is future-ready but the context isn't written
  yet, so the fallback path is exercised today.
- packages/super-editor/.../TableResizeOverlay.vue reads a different
  shape (tableMetadata.value.rtl); migration deferred to keep this PR
  scoped.

Companion to SD-3134 and PRs #3272/#3273/#3278; under SD-2771 Wave 3.

Tests:
- 14 unit tests for getTableVisualDirection (precedence, alias, no
  signal, null fallback).
- 174 painter table tests, 652 layout-engine tests, 189 super-editor
  table extension tests all pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants