From 36dac6e8eae054ecc2186c0014a505e92f4de15e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Sun, 20 Sep 2026 01:45:03 +0200 Subject: [PATCH] fix(mobile): keep diff code left-aligned in RTL interfaces https://github.com/Kilo-Org/cloud/pull/6400 --- .../pr-review/diff/diff-line.mounted.test.tsx | 20 ++++++ .../components/pr-review/diff/diff-line.tsx | 7 ++ .../diff/pr-diff-hunk-rows.mounted.test.tsx | 69 +++++++++++++++++++ .../pr-review/diff/pr-diff-hunk-rows.test.tsx | 20 +++++- .../pr-review/diff/pr-diff-hunk-rows.tsx | 10 ++- .../pr-diff-side-by-side-row.mounted.test.tsx | 19 +++++ .../diff/pr-diff-side-by-side-row.tsx | 11 ++- apps/mobile/src/lib/rtl-text.ts | 15 ++++ 8 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.mounted.test.tsx diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx index 1482dc2fb4c..1770a1969db 100644 --- a/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/diff-line.mounted.test.tsx @@ -159,6 +159,26 @@ describe('DiffLine gutter alignment', () => { }); }); +describe('DiffLine code direction', () => { + // Code is written left to right whatever the interface language. The code + // Text must name its own base direction: under the interface's RTL direction + // Android aligns LTR script to the right margin, so the continuation of a + // wrapped line starts mid-row instead of under the first line's start. + it('names the left-to-right base direction on the code text', () => { + const renderer = mountLine({ line: line(), language: null, keyId: 'ltr-code' }); + + const [codeText] = renderer.root.findAll( + node => node.type === ('RNText' as never) && node.props.selectable === true + ); + if (codeText === undefined) { + throw new Error('expected a selectable code Text'); + } + const style = codeText.props.style as { direction?: string; writingDirection?: string }; + expect(style.direction).toBe('ltr'); + expect(style.writingDirection).toBe('ltr'); + }); +}); + describe('DiffLine highlight runs', () => { // The diff line shares the code block's run renderer: untagged tokens stay // raw strings (React Native coalesces them into the code Text's own diff --git a/apps/mobile/src/components/pr-review/diff/diff-line.tsx b/apps/mobile/src/components/pr-review/diff/diff-line.tsx index 8e4b2b5d066..d9996fd7c26 100644 --- a/apps/mobile/src/components/pr-review/diff/diff-line.tsx +++ b/apps/mobile/src/components/pr-review/diff/diff-line.tsx @@ -7,6 +7,11 @@ // 56-point gutter; honouring the raw a11y scale (1.8x at AX5) would // overflow the gutter and break the side-by-side grid. // +// The code `Text` names its own LTR base direction (`LTR_TEXT_DIRECTION`): +// code reads left to right in every interface language, and inheriting an RTL +// interface's direction would right-align it, so a wrapped line's continuation +// would start mid-row instead of under the first line's start. +// // S7a adds two opt-in behaviours, both passed from the diff list: // - `onTap` makes the line tappable; the diff list runs the // selection reducer and updates the bridge / floating action. @@ -49,6 +54,7 @@ import { type ParsedDiffLine, } from '@/lib/pr-review/diff/parse-patch'; import { MUTED_COLOR } from '@/lib/pr-review/diff/syntax-colors'; +import { LTR_TEXT_DIRECTION } from '@/lib/rtl-text'; import { cn } from '@/lib/utils'; import { DIFF_MAX_FONT_SCALE, @@ -138,6 +144,7 @@ function DiffLineImpl({ line, language, onTap, isSelected }: Readonly ({ isRTL: false })); +vi.mock('react-native', () => ({ + I18nManager: i18nManager, + Pressable: 'Pressable', + Text: 'Text', + View: 'View', +})); +vi.mock('@rn-primitives/slot', () => ({ Text: 'Slot.Text' })); +vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' })); +vi.mock('@/components/ui/icons', () => ({ + Check: 'Check', + ChevronDown: 'ChevronDown', + File: 'File', + GitCommit: 'GitCommit', + X: 'X', +})); +vi.mock('@/components/ui/refresh-control', () => ({ RefreshControl: 'RefreshControl' })); +vi.mock('@/lib/format', () => ({ formatNumber: (value: number) => `${value}` })); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ + useThemeColors: () => ({ mutedForeground: '#888888' }), +})); + +let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; + +afterEach(() => { + act(() => renderer?.unmount()); + renderer = undefined; +}); + +/** The style the host `Text` ended up with, flattened the way RN flattens it. */ +function renderedTextStyle(element: ReactElement): Record { + act(() => { + renderer = TestRenderer.create(element); + }); + if (!renderer) { + throw new Error('the text did not render'); + } + const host = renderer.root.find( + node => Object.is(node.type, 'Text') && node.props.style !== undefined + ); + return Object.assign({}, ...([host.props.style] as Record[]).flat()); +} + +describe('HunkHeaderRow in an RTL interface', () => { + // The header is a code literal, so the LTR direction it passes must survive + // the shared Text's own RTL writing direction: the caller's style is the later + // entry in the component's `[rtl, caller]` array, which RN flattens last. + it('keeps the header left to right', () => { + i18nManager.isRTL = true; + const style = renderedTextStyle(createElement(HunkHeaderRow, { header: '@@ -0,0 +1,82 @@' })); + expect(style).toMatchObject({ direction: 'ltr', writingDirection: 'ltr' }); + }); + + // The component does name the interface direction when the caller has none, + // so the header's own direction is the only thing changing it. + it('leaves a text with no direction of its own under the interface direction', () => { + i18nManager.isRTL = true; + const style = renderedTextStyle(createElement(Text, { style: { color: '#000000' } }, 'plain')); + expect(style.writingDirection).toBe('rtl'); + }); +}); diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.test.tsx index 9e960aedb4b..47104cee497 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.test.tsx @@ -4,7 +4,7 @@ import { act, TestRenderer } from '@/test/renderer'; import { describe, expect, it, vi } from 'vitest'; import { i18n } from '@/i18n'; -import { EmptyFilesView, PaginationRow, TabStateMessage } from './pr-diff-hunk-rows'; +import { EmptyFilesView, HunkHeaderRow, PaginationRow, TabStateMessage } from './pr-diff-hunk-rows'; vi.mock('react-native', () => ({ View: 'View', @@ -90,6 +90,24 @@ describe('Files pane full-body states', () => { }); }); +describe('HunkHeaderRow code direction', () => { + // The header is a code literal ("@@ -0,0 +1,82 @@"). Under the interface's + // RTL base direction its runs reorder — the "+"/"-" land on the wrong side of + // their numbers and the ranges swap ends — so the header names its own + // left-to-right direction, like the diff lines below it. + it('names the left-to-right base direction on the header text', () => { + const renderer = mountNode(createElement(HunkHeaderRow, { header: '@@ -0,0 +1,82 @@' })); + + const [headerText] = renderer.root.findAll(node => String(node.type) === 'Text'); + if (headerText === undefined) { + throw new Error('expected the hunk header text'); + } + const style = headerText.props.style as { direction?: string; writingDirection?: string }; + expect(style.direction).toBe('ltr'); + expect(style.writingDirection).toBe('ltr'); + }); +}); + describe('PaginationRow', () => { function renderPaginationRow() { const renderer = mountNode( diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.tsx index baab4ba81fb..0ef188ca4a5 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-hunk-rows.tsx @@ -1,4 +1,9 @@ // Hunk / expand / pagination / empty-state rows for the PR diff FlashList. +// +// A hunk header is a code literal, so it names its own LTR base direction +// (`LTR_TEXT_DIRECTION`): under an RTL interface's base direction its runs +// reorder — `@@ -0,0 +1,82 @@` draws as `@@ 1,82+ 0,0- @@`. Every other row here +// is interface copy and keeps the interface's direction. import { Check, ChevronDown, File, GitCommit, X } from '@/components/ui/icons'; import { useTranslation } from 'react-i18next'; @@ -8,6 +13,7 @@ import { Text } from '@/components/ui/text'; import { i18n } from '@/i18n'; import { formatNumber } from '@/lib/format'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { LTR_TEXT_DIRECTION } from '@/lib/rtl-text'; import { CenteredState } from '@/components/centered-state'; import { type ExpandSeparatorItem } from '@/lib/pr-review/diff/pr-diff-list-items'; @@ -24,8 +30,8 @@ export function HunkHeaderRow({ header }: { header: string }) { > {header} diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.mounted.test.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.mounted.test.tsx index a9f199deab5..5a878b81304 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.mounted.test.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.mounted.test.tsx @@ -95,6 +95,25 @@ describe('SideBySideRow syntax palette follows the color scheme', () => { }); }); +describe('SideBySideRow code direction', () => { + // Same defect class as the unified DiffLine: code reads left to right in + // every interface language, so both column code Texts name their own base + // direction instead of inheriting the interface's RTL one. + it('names the left-to-right base direction on both column code texts', () => { + const renderer = mountRow(row()); + + const codeTexts = renderer.root.findAll( + node => node.type === ('RNText' as never) && node.props.selectable === true + ); + expect(codeTexts).toHaveLength(2); + for (const codeText of codeTexts) { + const style = codeText.props.style as { direction?: string; writingDirection?: string }; + expect(style.direction).toBe('ltr'); + expect(style.writingDirection).toBe('ltr'); + } + }); +}); + describe('SideBySideRow gutter alignment', () => { // Same defect class as the unified DiffLine gutter: a wrapped code line // makes the column several visual lines tall, and a centered number would diff --git a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx index abe91c6ca9d..038080c4e25 100644 --- a/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx +++ b/apps/mobile/src/components/pr-review/diff/pr-diff-side-by-side-row.tsx @@ -5,6 +5,11 @@ // Either column may be empty (left blank with a placeholder) when the // pair is a pure add or pure del. // +// Both column code `Text`s and the hunk header name their own LTR base +// direction (`LTR_TEXT_DIRECTION`), like the unified `DiffLine`: code reads left +// to right whatever the interface language, so a wrapped line's continuation +// starts under the first line's start instead of mid-row. +// // Side-by-side is read-only — commenting is unified-view only — so the // row does not accept tap/selection handlers. // @@ -24,6 +29,7 @@ import { highlightLine, type HighlightToken } from '@/lib/pr-review/diff/highlig import { type ParsedDiffLine, type ParsedHunk } from '@/lib/pr-review/diff/parse-patch'; import { type SideBySideRow as SideBySideRowData } from '@/lib/pr-review/diff/side-by-side'; import { MUTED_COLOR } from '@/lib/pr-review/diff/syntax-colors'; +import { LTR_TEXT_DIRECTION } from '@/lib/rtl-text'; import { cn } from '@/lib/utils'; import { type BoundedFontMetrics, @@ -97,6 +103,7 @@ function SideColumnImpl({ line, side, language, isDark, foreground }: SideColumn }; const codeContainerStyle: ViewStyle = { paddingVertical: VERTICAL_PADDING }; const codeBaseStyle: TextStyle = { + ...LTR_TEXT_DIRECTION, fontFamily: 'JetBrainsMono_500Medium', fontSize: metrics.codeFontSize, lineHeight: metrics.lineHeight, @@ -263,8 +270,8 @@ export function HunkSideBySideHeader({ hunk }: Readonly {hunk.header} diff --git a/apps/mobile/src/lib/rtl-text.ts b/apps/mobile/src/lib/rtl-text.ts index 0f7119bf433..a27079f00c0 100644 --- a/apps/mobile/src/lib/rtl-text.ts +++ b/apps/mobile/src/lib/rtl-text.ts @@ -21,3 +21,18 @@ export function withRtlWritingDirection(style: TextStyle | undefined): TextStyle } return style ? { ...RTL_WRITING_DIRECTION, ...style } : RTL_WRITING_DIRECTION; } + +/** + * Base direction for code content (a diff line, a hunk header): code is written + * left to right whatever the interface language. Inheriting the interface's + * direction instead leaves Android resolving the paragraph's `auto` alignment + * against RTL, which right-aligns LTR script — the continuation of a wrapped + * diff line starts mid-row instead of under the first line's start — and + * reorders a hunk header's runs (`@@ -0,0 +1,82 @@` draws as `@@ 1,82+ 0,0- @@`). + * + * `direction` names the base direction Android's paragraph layout reads, and + * `writingDirection` the same for iOS (see `RTL_WRITING_DIRECTION`). Apply it to + * the code `Text` itself: `direction` on a wrapping `View` would mirror that + * view's children too (it would move the diff gutter to the left). + */ +export const LTR_TEXT_DIRECTION: TextStyle = { direction: 'ltr', writingDirection: 'ltr' };