Skip to content

Commit 4b8f2fd

Browse files
authored
refactor(direction): centralize last paragraph isRtl reads on helper (SD-2778) (#3289)
* refactor(direction): centralize last paragraph isRtl reads on helper (SD-2778) Migrate the two remaining direct reads of attrs.direction/.dir on the paragraph inline-direction axis onto getParagraphInlineDirection: - layout-bridge/src/position-hit.ts: isRtlBlock - layout-resolved/src/resolveParagraph.ts: isRtl Behavior is unchanged on the typed directionContext path and strictly broader on fallback (the helper also covers paragraphProperties.rightToLeft). After this, no consumer outside the helper reads the legacy scalar fields; a follow-up can stop pm-adapter from writing them and drop them from ParagraphAttrs. * test(direction): pin SD-2778 migration via typed-path + broader-fallback cases Two coverage gaps caught in review: - layout-resolved/src/resolveLayout.test.ts ("preserves increasing first-line marker anchor for nested RTL list levels") used attrs.direction: 'rtl'. The pre-migration code read attrs.direction directly, so that fixture would have passed against the old implementation. Switch to directionContext.inlineDirection so the test only passes through the new helper-driven typed path. - layout-bridge/test/position-hit.test.ts: switching isRtlBlock to getParagraphInlineDirection is strictly broader on fallback (the helper also picks up paragraphProperties.rightToLeft when no directionContext is present). Pin that case so the broadening is intentional and not a regression vector.
1 parent ef9821e commit 4b8f2fd

4 files changed

Lines changed: 24 additions & 22 deletions

File tree

packages/layout-engine/layout-bridge/src/position-hit.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ import type {
2323
ParagraphBlock,
2424
ParagraphMeasure,
2525
} from '@superdoc/contracts';
26-
import { adjustAvailableWidthForTextIndent, computeLinePmRange, getFirstLineIndentOffset } from '@superdoc/contracts';
26+
import {
27+
adjustAvailableWidthForTextIndent,
28+
computeLinePmRange,
29+
getFirstLineIndentOffset,
30+
getParagraphInlineDirection,
31+
} from '@superdoc/contracts';
2732
import { charOffsetToPm, findCharacterAtX } from './text-measurement.js';
2833
import type { PageGeometryHelper } from './page-geometry-helper.js';
2934

@@ -120,26 +125,9 @@ export const getAtomicPmRange = (fragment: AtomicFragment, block: FlowBlock): {
120125

121126
export const isRtlBlock = (block: FlowBlock): boolean => {
122127
if (block.kind !== 'paragraph') return false;
123-
const attrs = block.attrs as Record<string, unknown> | undefined;
124-
if (!attrs) return false;
125-
// AIDEV-NOTE: The typed directionContext.inlineDirection (SD-2776) is the source of
126-
// truth for paragraph inline direction. Check the value, not the key — `inlineDirection`
127-
// can be `undefined` per the resolver contract (no explicit w:bidi anywhere in the
128-
// cascade), and we should fall through to the legacy field in that case.
129128
// Do NOT consult attrs.textDirection here: that's writing-mode (ECMA §17.18.93,
130129
// values lrTb/tbRl/btLr/lrTbV/tbRlV/tbLrV) which is a separate axis from inline RTL.
131-
const directionContext = attrs.directionContext as { inlineDirection?: string } | undefined;
132-
if (directionContext?.inlineDirection != null) {
133-
return directionContext.inlineDirection === 'rtl';
134-
}
135-
// AIDEV-NOTE: compat-fallback — `attrs.direction` / `attrs.dir` are the legacy scalar
136-
// duplicates of `directionContext.inlineDirection`. Retire once SD-2778 collapses
137-
// them on `ParagraphAttrs`.
138-
const directionAttr = attrs.direction ?? attrs.dir;
139-
if (typeof directionAttr === 'string' && directionAttr.toLowerCase() === 'rtl') {
140-
return true;
141-
}
142-
return false;
130+
return getParagraphInlineDirection(block.attrs) === 'rtl';
143131
};
144132

145133
export const determineColumn = (layout: Layout, fragmentX: number): number => {

packages/layout-engine/layout-bridge/test/position-hit.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,13 @@ describe('isRtlBlock', () => {
6161
),
6262
).toBe(true);
6363
});
64+
65+
// SD-2778: switching to getParagraphInlineDirection is strictly broader on
66+
// fallback than the prior inline read. Specifically, the helper picks up
67+
// paragraphProperties.rightToLeft when neither directionContext nor the legacy
68+
// scalar field is present. Pin that case so the broader fallback is intentional.
69+
it('falls back to paragraphProperties.rightToLeft when no other direction signal is present', () => {
70+
expect(isRtlBlock(paragraph({ paragraphProperties: { rightToLeft: true } }))).toBe(true);
71+
expect(isRtlBlock(paragraph({ paragraphProperties: { rightToLeft: false } }))).toBe(false);
72+
});
6473
});

packages/layout-engine/layout-resolved/src/resolveLayout.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1777,7 +1777,12 @@ describe('resolveLayout', () => {
17771777
id,
17781778
runs: [{ kind: 'text', text: 'RTL list item' }],
17791779
attrs: {
1780-
direction: 'rtl',
1780+
// SD-2778: use directionContext so this test only passes through the
1781+
// new helper-driven typed path. The pre-migration code read
1782+
// attrs.direction directly, so the prior `direction: 'rtl'` fixture
1783+
// would have passed against the old implementation too and didn't
1784+
// actually prove the migration.
1785+
directionContext: { inlineDirection: 'rtl', writingMode: 'horizontal-tb' },
17811786
indent: { right, hanging: -24 },
17821787
wordLayout: {
17831788
marker: {

packages/layout-engine/layout-resolved/src/resolveParagraph.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type {
99
ResolvedDropCapItem,
1010
ResolvedListMarkerItem,
1111
} from '@superdoc/contracts';
12-
import { adjustAvailableWidthForTextIndent } from '@superdoc/contracts';
12+
import { adjustAvailableWidthForTextIndent, getParagraphInlineDirection } from '@superdoc/contracts';
1313
import {
1414
isMinimalWordLayout,
1515
resolveListMarkerGeometry,
@@ -93,7 +93,7 @@ export function resolveParagraphContent(
9393
const paraIndent = (block.attrs as ParagraphAttrs | undefined)?.indent;
9494
const paraIndentLeft = paraIndent?.left ?? 0;
9595
const paraIndentRight = paraIndent?.right ?? 0;
96-
const isRtl = (block.attrs as ParagraphAttrs | undefined)?.direction === 'rtl';
96+
const isRtl = getParagraphInlineDirection(block.attrs) === 'rtl';
9797
const {
9898
anchorIndentPx: paraMarkerAnchorIndent,
9999
firstLinePx: markerFirstLine,

0 commit comments

Comments
 (0)