From 38f23f966383fead5e31dd48a690e0bddb0d0350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Tue, 22 Sep 2026 05:06:46 +0200 Subject: [PATCH 1/5] fix(mobile): stop Latin tracking from splitting Arabic text https://github.com/Kilo-Org/cloud/pull/6446 --- apps/mobile/plugins/branded-splash.test.ts | 8 +- .../agents/session-filter-button.tsx | 14 ++- ...ssion-list-header-actions.mounted.test.tsx | 99 ++++++++------- .../agents/session-list-header-actions.tsx | 26 ++-- .../manual-review-screen.mounted.test.tsx | 10 ++ ...estructive-confirm-dialog.mounted.test.tsx | 10 +- .../components/destructive-confirm-dialog.tsx | 8 +- .../profile-screen.signout.mounted.test.tsx | 94 +++++++------- apps/mobile/src/components/profile-screen.tsx | 10 +- .../src/components/ui/segmented-control.tsx | 4 - .../src/components/ui/text.mounted.test.tsx | 115 ++++++++++++++++-- .../ui/text.rtl-tracking.mounted.test.tsx | 4 +- apps/mobile/src/components/ui/text.tsx | 22 ++-- .../components/use-sign-out-confirmation.ts | 38 ++---- .../src/lib/destructive-confirm-platform.ts | 19 --- apps/mobile/src/lib/rtl-text.test.ts | 49 ++++++++ apps/mobile/src/lib/rtl-text.ts | 41 +++++++ 17 files changed, 384 insertions(+), 187 deletions(-) delete mode 100644 apps/mobile/src/lib/destructive-confirm-platform.ts create mode 100644 apps/mobile/src/lib/rtl-text.test.ts diff --git a/apps/mobile/plugins/branded-splash.test.ts b/apps/mobile/plugins/branded-splash.test.ts index d3f381d452..bdf95fe785 100644 --- a/apps/mobile/plugins/branded-splash.test.ts +++ b/apps/mobile/plugins/branded-splash.test.ts @@ -124,6 +124,12 @@ describe('shared branded splash', () => { }); it('generates both native splash surfaces from the same options', async () => { + // Introspect against a temporary project, not the repository root: the base + // mods merge the resources already on disk there, so a developer's + // `android/` prebuild (gitignored) would add its own colors to the + // assertion below. Only `config._internal.projectRoot` names the real + // project, where the splash image resolves. + const { root } = createAndroidProject(); const config: ExportedConfig = withBrandedSplash( { name: 'Kilo', slug: 'kilo-app', _internal: { projectRoot } }, { image: './assets/images/logo-mark.png', backgroundColor: '#FAF74F', imageWidth: 100 } @@ -136,7 +142,7 @@ describe('shared branded splash', () => { expect(config.mods?.android?.styles).toBeTypeOf('function'); const evaluated = await compileModsAsync(config, { - projectRoot, + projectRoot: root, platforms: ['ios', 'android'], introspect: true, }); diff --git a/apps/mobile/src/components/agents/session-filter-button.tsx b/apps/mobile/src/components/agents/session-filter-button.tsx index f42cdc39e1..dacb04318d 100644 --- a/apps/mobile/src/components/agents/session-filter-button.tsx +++ b/apps/mobile/src/components/agents/session-filter-button.tsx @@ -12,6 +12,14 @@ type SessionFilterButtonProps = { activeCount: number; onPress: () => void; testID?: string; + /** + * Overrides the control's own per-side slop. The agents header row narrows + * this control's two horizontal sides to fit its `gap-4` row gap: the row + * mirrors under RTL while `hitSlop` does not, so the cap cannot sit on one + * physical side. Callers pass an explicit per-side slop instead of the + * control's default. + */ + hitSlop?: React.ComponentProps['hitSlop']; }; /** @@ -23,6 +31,7 @@ export function SessionFilterButton({ activeCount, onPress, testID, + hitSlop, }: Readonly) { const colors = useThemeColors(); const { t } = useTranslation(); @@ -34,8 +43,9 @@ export function SessionFilterButton({ // The frame is the tap target the size audit measures, not the 20pt // glyph: `h-11 w-11` is 38.5pt on device, and the 3pt slop carries it to // the 44pt minimum. It fits the header's own `min-h-11` row, so the - // header keeps its height. - hitSlop={COMPACT_CONTROL_HIT_SLOP_DP} + // header keeps its height. A caller that lays this control beside another + // in a tight row overrides the slop to fit the row's gap. + hitSlop={hitSlop ?? COMPACT_CONTROL_HIT_SLOP_DP} accessibilityRole="button" // The count is spoken as part of the name, so no new translated string is // needed to announce "Filter sessions, 2". diff --git a/apps/mobile/src/components/agents/session-list-header-actions.mounted.test.tsx b/apps/mobile/src/components/agents/session-list-header-actions.mounted.test.tsx index f0efbace94..e4b7111119 100644 --- a/apps/mobile/src/components/agents/session-list-header-actions.mounted.test.tsx +++ b/apps/mobile/src/components/agents/session-list-header-actions.mounted.test.tsx @@ -8,6 +8,7 @@ import { act, TestRenderer } from '@/test/renderer'; import { describe, expect, it, vi } from 'vitest'; import { MIN_TAP_TARGET_DP, TOUCH_TARGET_DP } from '@/lib/a11y/tap-target'; +import { COMPACT_CONTROL_FRAME_DP } from '@/lib/a11y/touch-target'; import { SessionListHeaderActions } from './session-list-header-actions'; import '@/i18n'; @@ -223,49 +224,65 @@ describe('SessionListHeaderActions new-session control', () => { }); }); - it('meets the filter control at the row gap instead of overlapping its touch region', async () => { - const renderer = await mountHeader(true, noop); + // React Native mirrors the row's `flex-row` order under RTL but does not + // mirror `hitSlop` (`screen-header.tsx:165` spells its own slop per direction + // for the same reason), so the gap-facing pair follows the physical order the + // direction gives the row: the new-session control's right with the filter's + // left in LTR, and its left with the filter's right under RTL. + it.each([false, true])( + 'meets the filter control at the row gap instead of overlapping its touch region (RTL=%s)', + async isRTL => { + const renderer = await mountHeader(true, noop); + + const newSession = pressesWithLabel(renderer.root, 'New session')[0]; + const filter = pressesWithLabel(renderer.root, 'Filter sessions')[0]; + if (!newSession || !filter) { + throw new Error('header controls not found'); + } - const newSession = pressesWithLabel(renderer.root, 'New session')[0]; - const filter = pressesWithLabel(renderer.root, 'Filter sessions')[0]; - if (!newSession || !filter) { - throw new Error('header controls not found'); + const row = renderer.root.find( + node => + typeof node.type === 'string' && + (node.type as string) === 'View' && + typeof node.props.className === 'string' && + node.props.className.split(/\s+/).includes('gap-4') + ); + const gapDp = await compiledGapDp(row.props.className as string); + // NativeWind v5 fixes 1rem at 14pt, so the row's `gap-4` is 14pt, not 16pt. + expect(gapDp).toBe(14); + + // `hitSlopInsets` validates and normalizes either shape; `slopSideDp` then + // reads the facing side, because the filter expresses its slop as one dp + // value for every side while the new-session control caps its right side. + const newSessionSlop = hitSlopInsets(newSession.props.hitSlop); + const filterSlop = hitSlopInsets(filter.props.hitSlop); + // The gap has to fit the pair's two facing slops in either direction; + // more than the gap means the two touch regions overlap, and the later + // sibling (the filter) claims the taps inside the overlap. Either control + // may express hitSlop as one number or as per-side insets: the filter + // writes its slop as one number for every side, the new-session control + // as a per-side object. + const facingDp = isRTL + ? slopSideDp(newSessionSlop, 'left') + slopSideDp(filterSlop, 'right') + : slopSideDp(newSessionSlop, 'right') + slopSideDp(filterSlop, 'left'); + expect(facingDp).toBeLessThanOrEqual(gapDp); + // Capping the facing sides must not drop either control below the design + // target: the compact new-session box plus its symmetric slop, and the + // filter's `h-11` frame plus the capped slop. + const box = boxDp(newSession.props.className as string); + expect( + box.width + slopSideDp(newSessionSlop, 'left') + slopSideDp(newSessionSlop, 'right') + ).toBeGreaterThanOrEqual(TOUCH_TARGET_DP); + expect(String(filter.props.className)).toContain('h-11 w-11'); + expect( + COMPACT_CONTROL_FRAME_DP + slopSideDp(filterSlop, 'left') + slopSideDp(filterSlop, 'right') + ).toBeGreaterThanOrEqual(TOUCH_TARGET_DP); + + act(() => { + renderer.unmount(); + }); } - - const row = renderer.root.find( - node => - typeof node.type === 'string' && - (node.type as string) === 'View' && - typeof node.props.className === 'string' && - node.props.className.split(/\s+/).includes('gap-4') - ); - const gapDp = await compiledGapDp(row.props.className as string); - // NativeWind v5 fixes 1rem at 14pt, so the row's `gap-4` is 14pt, not 16pt. - expect(gapDp).toBe(14); - - // `hitSlopInsets` validates and normalizes either shape; `slopSideDp` then - // reads the facing side, because the filter expresses its slop as one dp - // value for every side while the new-session control caps its right side. - const newSessionSlop = hitSlopInsets(newSession.props.hitSlop); - const filterSlop = hitSlopInsets(filter.props.hitSlop); - // The new-session control sits left of the filter, so the gap has to fit - // both facing slops; more than the gap means the two regions overlap. Either - // control may express hitSlop as one number or as per-side insets: the - // filter writes its slop as one number for every side, the new-session - // control as a per-side object. - expect( - slopSideDp(newSessionSlop, 'right') + slopSideDp(filterSlop, 'left') - ).toBeLessThanOrEqual(gapDp); - // Capping the right side must not drop the control below the design target. - const box = boxDp(newSession.props.className as string); - expect( - box.width + slopSideDp(newSessionSlop, 'left') + slopSideDp(newSessionSlop, 'right') - ).toBeGreaterThanOrEqual(TOUCH_TARGET_DP); - - act(() => { - renderer.unmount(); - }); - }); + ); it('renders no new-session control when showNewSession is false', async () => { const renderer = await mountHeader(false, noop); diff --git a/apps/mobile/src/components/agents/session-list-header-actions.tsx b/apps/mobile/src/components/agents/session-list-header-actions.tsx index 9dcc4660b9..4ad2fecf11 100644 --- a/apps/mobile/src/components/agents/session-list-header-actions.tsx +++ b/apps/mobile/src/components/agents/session-list-header-actions.tsx @@ -8,14 +8,19 @@ import { COMPACT_CONTROL_HIT_SLOP_DP } from '@/lib/a11y/tap-target'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; // The row's `gap-4` compiles to 14pt, not 16pt: NativeWind v5 fixes 1rem at -// 14pt, so `gap-4` (1rem) is 14pt. The filter control's own left slop is 8pt, -// so the shared 8pt right slop would overlap its touch region by 2pt; capping -// the new-session control's right side at 14 - 8 leaves the two regions meeting -// at the gap's boundary. 32 + 8 + 6 = 46pt still clears `DESIGN.md:364`'s 44pt. -const NEW_SESSION_HIT_SLOP = { +// 14pt. React Native mirrors the row's flex order under RTL but does not mirror +// `hitSlop` (`screen-header.tsx:165`), so a cap spelled on one physical side +// would meet the gap in one direction and overlap it in the other: the +// new-session control keeps the shared symmetric 8pt compact slop, and the +// wider filter frame absorbs the row's 2pt shortfall on both horizontal sides +// (14 - 8 = 6). Whichever way the row mirrors, the facing pair sums to exactly +// the 14pt gap, so the two touch regions meet at its boundary instead of one +// claiming the later sibling's taps inside an overlap. +// 38.5 + 6 + 6 = 50.5pt still clears `DESIGN.md:364`'s 44pt. +const FILTER_HIT_SLOP = { top: COMPACT_CONTROL_HIT_SLOP_DP, bottom: COMPACT_CONTROL_HIT_SLOP_DP, - left: COMPACT_CONTROL_HIT_SLOP_DP, + left: 6, right: 6, }; @@ -41,11 +46,9 @@ export function SessionListHeaderActions({ return ( {showNewSession ? ( - + // No `hitSlop` here: `IconButton`'s default is the symmetric 8pt + // compact slop, the pair's larger half (see `FILTER_HIT_SLOP` above). + ) : null} @@ -53,6 +56,7 @@ export function SessionListHeaderActions({ activeCount={activeFilterCount} onPress={onOpenFilters} testID="agents-open-filters" + hitSlop={FILTER_HIT_SLOP} /> ); diff --git a/apps/mobile/src/components/code-reviewer/manual-review-screen.mounted.test.tsx b/apps/mobile/src/components/code-reviewer/manual-review-screen.mounted.test.tsx index 8682f46244..6ac9f9a071 100644 --- a/apps/mobile/src/components/code-reviewer/manual-review-screen.mounted.test.tsx +++ b/apps/mobile/src/components/code-reviewer/manual-review-screen.mounted.test.tsx @@ -38,6 +38,16 @@ vi.mock('react-native', () => ({ })); vi.mock('@/components/agents/model-selector', () => ({ ModelSelector: 'ModelSelector' })); vi.mock('@/components/empty-state', () => ({ EmptyState: 'EmptyState' })); +// The keyboard padding wraps the form for the scroll-reveal path and loads +// `react-native-safe-area-context`, whose untransformed source the mounted +// project cannot parse; this test only asserts the connect CTA. +vi.mock('@/components/kilo-chat/app-aware-keyboard-padding', () => ({ + AppAwareKeyboardPaddingView: 'View', + useAppAwareKeyboardPadding: () => 0, +})); +vi.mock('@/components/kilo-chat/use-reveal-end-on-keyboard', () => ({ + useRevealEndOnKeyboard: () => ({ current: null }), +})); vi.mock('@/components/query-error', () => ({ QueryError: 'QueryError' })); vi.mock('@/components/screen-header', () => ({ ScreenHeader: 'ScreenHeader' })); vi.mock('@/components/ui/button', () => ({ Button: 'Button' })); diff --git a/apps/mobile/src/components/destructive-confirm-dialog.mounted.test.tsx b/apps/mobile/src/components/destructive-confirm-dialog.mounted.test.tsx index 208c1ec01f..85cb278059 100644 --- a/apps/mobile/src/components/destructive-confirm-dialog.mounted.test.tsx +++ b/apps/mobile/src/components/destructive-confirm-dialog.mounted.test.tsx @@ -83,9 +83,11 @@ afterEach(() => { }); describe('DestructiveConfirmDialog', () => { - // The finding's defect is that the destructive sign-out action had the same - // affordance as the neutral cancel on Android. The confirm control must carry - // the destructive (red) fill while cancel stays a neutral outline. + // The defect this dialog exists for: a destructive action whose confirmation + // control carries no distinct affordance (Android's native alert paints every + // button with the theme accent). The confirm control must carry the + // destructive (red) fill while cancel stays a neutral outline, on both + // platforms — this component is the one implementation for both. it('gives the confirm action the destructive fill and cancel a neutral one', () => { const root = mount(); @@ -132,7 +134,7 @@ describe('DestructiveConfirmDialog', () => { expect(onConfirm).toHaveBeenCalledTimes(1); }); - it('dismisses without confirming on the Android back request', () => { + it('dismisses without confirming on the back request', () => { const root = mount(); act(() => { diff --git a/apps/mobile/src/components/destructive-confirm-dialog.tsx b/apps/mobile/src/components/destructive-confirm-dialog.tsx index f658a8e3cd..8c73e02337 100644 --- a/apps/mobile/src/components/destructive-confirm-dialog.tsx +++ b/apps/mobile/src/components/destructive-confirm-dialog.tsx @@ -16,10 +16,10 @@ type DestructiveConfirmDialogProps = { * In-app confirmation for a destructive action, rendered with the destructive * (red) button variant. * - * Android's native `AlertDialog` paints every button with the theme accent, so - * `Alert.alert`'s `style: 'destructive'` never reaches the screen there (iOS - * honors it and keeps the native alert). Android renders this surface instead, - * so the destructive choice still carries the red affordance. + * One implementation for both platforms: Android's native `AlertDialog` paints + * every button with the theme accent, so `Alert.alert`'s `style: 'destructive'` + * never reaches the screen there, and the confirmation must behave the same on + * iOS and Android. This surface carries the red affordance on both. * * Mount it only while it should be open (e.g. `{confirming && }`), * the same lifecycle `RenameModal` uses. diff --git a/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx index 1c192aedd5..bb880c2312 100644 --- a/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx +++ b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx @@ -168,58 +168,46 @@ describe('ProfileScreen sign-out confirmation', () => { }); } - // The finding: the native Android alert painted sign-out and cancel the same - // teal, so the destructive choice had no distinct affordance. Android renders - // the in-app dialog whose sign-out control carries the destructive (red) - // variant. - it('opens the in-app dialog whose destructive control signs out on android', async () => { - platform.os = 'android'; - const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen)); - - pressSignOutTile(renderer); - - // Android never falls back to the native alert. - expect(alertFn).not.toHaveBeenCalled(); - // Opening the confirmation signs nobody out; only its destructive control does. - expect(signOutFn).not.toHaveBeenCalled(); - const confirm = renderer.root.find( - node => isType(node, 'Button') && node.props.variant === 'destructive' - ); - act(() => { - (confirm.props as { onPress?: () => void }).onPress?.(); - }); - expect(signOutFn).toHaveBeenCalledTimes(1); - - unmount(); - }); - - // `apps/mobile/AGENTS.md`: "Prefer native sheets, alerts, pickers, gestures, - // and keyboard behavior. Confirm destructive actions with `Alert.alert()`." - // iOS keeps the native alert, whose `style: 'destructive'` already renders the - // sign-out choice in red. - it('keeps the native alert whose destructive button signs out on ios', async () => { - platform.os = 'ios'; - const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen)); - - pressSignOutTile(renderer); - - // No in-app dialog on iOS: the confirmation is the native alert. - expect( - renderer.root.findAll(node => isType(node, 'Button') && node.props.variant === 'destructive') - ).toHaveLength(0); - expect(alertFn).toHaveBeenCalledTimes(1); - // Opening the confirmation signs nobody out; only the destructive alert - // button does. - expect(signOutFn).not.toHaveBeenCalled(); - const buttons = alertFn.mock.calls[0]?.[2] as - | { style?: string; onPress?: () => void }[] - | undefined; - const destructive = buttons?.find(button => button.style === 'destructive'); - act(() => { - destructive?.onPress?.(); - }); - expect(signOutFn).toHaveBeenCalledTimes(1); + function dialogButton( + renderer: TestRenderer.ReactTestRenderer, + variant: 'outline' | 'destructive' + ) { + return renderer.root.find(node => isType(node, 'Button') && node.props.variant === variant); + } - unmount(); - }); + // The request names no platform, so one destructive confirm serves both: the + // in-app dialog renders on iOS and Android alike, and its sign-out control + // carries the destructive (red) variant. The native alert cannot be the + // shared implementation — Android's `AlertDialog` paints every button with + // the theme accent, ignoring `style: 'destructive'`. + it.each(['android', 'ios'] as const)( + 'opens the in-app dialog whose destructive control signs out on %s', + async os => { + platform.os = os; + const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen)); + + pressSignOutTile(renderer); + + // One implementation for both platforms: the confirmation is the in-app + // dialog everywhere; no native alert replaces it on either side. + expect(alertFn).not.toHaveBeenCalled(); + // Opening the confirmation signs nobody out. + expect(signOutFn).not.toHaveBeenCalled(); + + // Cancel keeps the user signed in. + act(() => { + (dialogButton(renderer, 'outline').props as { onPress?: () => void }).onPress?.(); + }); + expect(signOutFn).not.toHaveBeenCalled(); + + // Reopen: only the destructive control signs out. + pressSignOutTile(renderer); + act(() => { + (dialogButton(renderer, 'destructive').props as { onPress?: () => void }).onPress?.(); + }); + expect(signOutFn).toHaveBeenCalledTimes(1); + + unmount(); + } + ); }); diff --git a/apps/mobile/src/components/profile-screen.tsx b/apps/mobile/src/components/profile-screen.tsx index 985d694d39..51449eb322 100644 --- a/apps/mobile/src/components/profile-screen.tsx +++ b/apps/mobile/src/components/profile-screen.tsx @@ -85,12 +85,10 @@ export function ProfileScreen() { // the only place the signed-in address renders. const afterInteractions = useAfterInteractions(); const prReviewEnabled = useFeatureFlag(FEATURE_FLAG_PR_REVIEW, true); - // Android's native alert paints every button with the theme accent, so - // `Alert.alert`'s destructive style never shows the red affordance there. - // Android opens the in-app confirmation instead; iOS keeps the native alert, - // which already renders the destructive sign-out choice in red. - // The confirmation's platform split lives in the hook, keeping this screen's - // shared layout path free of platform forks (`screen-insets.test.ts`). + // One destructive confirm for both platforms: the in-app dialog carries the + // destructive (red) affordance on iOS and Android alike, so the sign-out + // path never branches on the platform. The confirmation itself, and its + // rationale, live in `useSignOutConfirmation`. const { confirmVisible, requestSignOut, dismissConfirm, confirmSignOut } = useSignOutConfirmation( () => void signOut() ); diff --git a/apps/mobile/src/components/ui/segmented-control.tsx b/apps/mobile/src/components/ui/segmented-control.tsx index 7f547e6c42..a9d69e02fd 100644 --- a/apps/mobile/src/components/ui/segmented-control.tsx +++ b/apps/mobile/src/components/ui/segmented-control.tsx @@ -62,10 +62,6 @@ export function SegmentedControl({ 'text-center text-sm', selected ? 'font-medium text-foreground' : 'text-muted-foreground' )} - // One line per option: a wrapped label makes the two choices - // uneven. Longer locales ellipsize instead of growing a second - // line; the radio's accessibilityLabel still carries the full text. - numberOfLines={1} > {option.label} diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index 39b117e005..e733f3bb30 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -1,22 +1,71 @@ +/* eslint-disable typescript-eslint/no-deprecated -- DOM-free mounted React Native style regression tests. */ +// eslint-disable-next-line import/no-nodejs-modules -- Use the compiler's compatible CommonJS export. +import { createRequire } from 'node:module'; +import tailwindcss from '@tailwindcss/postcss'; +import postcss from 'postcss'; import { createElement, type ReactElement } from 'react'; +import { type TextStyle } from 'react-native'; +import type * as NativeCSSCompiler from 'react-native-css/compiler'; import { act, TestRenderer } from '@/test/renderer'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { Eyebrow } from '@/components/ui/eyebrow'; -import { Text } from '@/components/ui/text'; +import { Eyebrow } from './eyebrow'; +import { Text } from './text'; const i18nManager = vi.hoisted(() => ({ isRTL: false })); -// `Text` reads the native direction at render time, so the mutable flag drives -// each render; the host text element is the assertion target. vi.mock('react-native', () => ({ I18nManager: i18nManager, Text: 'Text', })); -// `@rn-primitives/slot` ships untranspiled JSX and is only reached by `asChild`. vi.mock('@rn-primitives/slot', () => ({ Text: 'Slot.Text' })); +const { compile } = createRequire(import.meta.url)( + 'react-native-css/compiler' +) as typeof NativeCSSCompiler; + let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; -function mount(element: ReactElement) { + +function mount(children: string, style?: TextStyle, className = 'tracking-wide') { + act(() => { + renderer = TestRenderer.create(createElement(Text, { className, style }, children)); + }); + if (!renderer) { + throw new Error('Missing Text renderer'); + } + return renderer.root.find(node => Object.is(node.type, 'Text')); +} + +/** The component's own inline styles, flattened the way React Native merges them. */ +function ownStyles(node: TestRenderer.ReactTestInstance): TextStyle[] { + const { style } = node.props; + const entries = Array.isArray(style) ? (style as unknown[]).flat(Infinity) : [style]; + return entries.filter((entry): entry is TextStyle => typeof entry === 'object' && entry !== null); +} + +/** The letter spacing a React Native style array resolves to: the last entry wins. */ +function resolvedLetterSpacing(styles: TextStyle[]): TextStyle['letterSpacing'] { + return styles.findLast(style => 'letterSpacing' in style)?.letterSpacing; +} + +/** What the app's own Tailwind + react-native-css compile a tracking class to. */ +async function compiledLetterSpacing(className: string): Promise { + const { css } = await postcss([tailwindcss()]).process( + `@reference "../../global.css"; .target { @apply ${className}; }`, + { from: import.meta.filename } + ); + const rules = compile(css, { inlineVariables: false }).stylesheet().s; + const declarations = + rules?.find(([name]) => name === 'target')?.[1].flatMap(rule => rule.d ?? []) ?? []; + const value = declarations + .map(declaration => declaration as { letterSpacing?: unknown }) + .findLast(declaration => typeof declaration.letterSpacing === 'number')?.letterSpacing; + if (typeof value !== 'number') { + throw new TypeError(`${className} did not compile to a letter spacing`); + } + return value; +} + +function mountElement(element: ReactElement) { act(() => { renderer = TestRenderer.create(element); }); @@ -35,17 +84,65 @@ beforeEach(() => { (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; i18nManager.isRTL = false; }); + afterEach(() => { act(() => renderer?.unmount()); renderer = undefined; }); +describe('Text joined-script letter spacing', () => { + it('keeps Latin tracking in a left-to-right interface', () => { + const node = mount('Preferences'); + expect(node.props.className).toContain('tracking-wide'); + expect(ownStyles(node)).toEqual([]); + }); + + it('resets the tracking on an Arabic run in a left-to-right interface', () => { + expect(ownStyles(mount('التفصيلات'))).toEqual([{ letterSpacing: 0 }]); + }); + + it('adds the RTL paragraph direction beside the reset on an Arabic run', () => { + i18nManager.isRTL = true; + expect(ownStyles(mount('أعلام المميزات'))).toEqual([ + { writingDirection: 'rtl' }, + { letterSpacing: 0 }, + ]); + }); + + it('resets the tracking in a right-to-left interface whatever the script', () => { + i18nManager.isRTL = true; + const styles = ownStyles(mount('Preferences')); + expect(styles).toContainEqual({ writingDirection: 'rtl' }); + expect(styles).toContainEqual({ letterSpacing: 0 }); + }); + + it('lets an explicit caller letterSpacing win over the reset', () => { + const styles = ownStyles(mount('المظهر', { letterSpacing: 2 })); + expect(styles.at(-1)).toEqual({ letterSpacing: 2 }); + }); + + it('overrides the tracking the app compiles, on a joined run only', async () => { + const tracking = await compiledLetterSpacing('tracking-[1.5px]'); + expect(tracking).toBeGreaterThan(0); + + const latin = mount('Settings', undefined, 'tracking-[1.5px]'); + const arabic = mount('التفصيلات', undefined, 'tracking-[1.5px]'); + + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...ownStyles(latin)])).toBe( + tracking + ); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...ownStyles(arabic)])).toBe(0); + }); +}); + describe('Text eyebrow letterspacing', () => { // Finding home-ar-loading: an Arabic section label carried the Latin // uppercase letter-spacing and broke apart mid-word ('ال جلسا ت'). it.each([false, true])('keeps the eyebrow display treatment in LTR only (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; - const classes = hostClasses(mount(createElement(Text, { variant: 'eyebrow' }, 'Live now'))); + const classes = hostClasses( + mountElement(createElement(Text, { variant: 'eyebrow' }, 'Live now')) + ); expect(classes).toEqual( expect.arrayContaining(['font-mono-medium', 'text-[10px]', 'text-muted-foreground']) ); @@ -59,7 +156,7 @@ describe('Text eyebrow letterspacing', () => { it('leaves a non-eyebrow variant untouched in either direction', () => { i18nManager.isRTL = true; - const classes = hostClasses(mount(createElement(Text, null, 'Live now'))); + const classes = hostClasses(mountElement(createElement(Text, null, 'Live now'))); expect(classes).not.toContain('uppercase'); expect(classes.some(name => name.startsWith('tracking'))).toBe(false); }); @@ -67,7 +164,7 @@ describe('Text eyebrow letterspacing', () => { it.each([false, true])('applies the same rule to the Eyebrow component (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; const classes = hostClasses( - mount(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) + mountElement(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) ); expect(classes).toEqual(expect.arrayContaining(['font-mono-medium', 'text-[10px]'])); if (isRTL) { diff --git a/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx b/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx index abe31c35e6..70d726e561 100644 --- a/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx @@ -102,7 +102,9 @@ describe('Text tracked labels in RTL', () => { i18nManager.isRTL = true; const root = mount(createElement(Eyebrow, null, 'استكشف')); - expect(hostText(root).props.className as string).toContain('tracking-[1.5px]'); + // The RTL eyebrow drops the Latin display treatment — the tracked class is + // an LTR-only addition to the variant — while the reset still applies. + expect(hostText(root).props.className as string).not.toContain('tracking-[1.5px]'); expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); }); }); diff --git a/apps/mobile/src/components/ui/text.tsx b/apps/mobile/src/components/ui/text.tsx index 2b273ed3dc..2546b1a3fb 100644 --- a/apps/mobile/src/components/ui/text.tsx +++ b/apps/mobile/src/components/ui/text.tsx @@ -1,9 +1,9 @@ import * as Slot from '@rn-primitives/slot'; import { cva, type VariantProps } from 'class-variance-authority'; import * as React from 'react'; -import { I18nManager, Text as RNText, type Role } from 'react-native'; +import { I18nManager, Text as RNText, type Role, type TextStyle } from 'react-native'; -import { RTL_NO_LETTER_SPACING, RTL_WRITING_DIRECTION } from '@/lib/rtl-text'; +import { RTL_NO_LETTER_SPACING, RTL_WRITING_DIRECTION, textLetterSpacing } from '@/lib/rtl-text'; import { cn } from '@/lib/utils'; const textVariants = cva('text-foreground text-base font-medium', { @@ -74,6 +74,18 @@ function Text({ }) { const textClass = React.useContext(TextClassContext); const Component = asChild ? Slot.Text : RNText; + // Letter spacing — Tailwind's `tracking-*` — is a Latin typographic device: + // it opens every glyph from its neighbour. A joined-script run (Arabic, + // Farsi, Urdu, Kurdish, Pashto) is one connected shape, so any tracking class + // on it would pull apart letters the script joins; the app's RTL catalogs do + // not take tracking either. The reset therefore follows the script whatever + // the interface direction is, and an RTL interface whatever the script is. + // Latin runs in LTR keep the style's tracking. The caller's own style stays + // last, so an explicit `letterSpacing` still wins. + const ownStyles = [ + I18nManager.isRTL ? RTL_WRITING_DIRECTION : undefined, + textLetterSpacing(props.children) ?? (I18nManager.isRTL ? RTL_NO_LETTER_SPACING : undefined), + ].filter((style): style is TextStyle => style !== undefined); return ( 0 ? [...ownStyles, props.style] : props.style} /> ); } diff --git a/apps/mobile/src/components/use-sign-out-confirmation.ts b/apps/mobile/src/components/use-sign-out-confirmation.ts index dd96d55271..52e747cf69 100644 --- a/apps/mobile/src/components/use-sign-out-confirmation.ts +++ b/apps/mobile/src/components/use-sign-out-confirmation.ts @@ -1,38 +1,26 @@ import { useCallback, useState } from 'react'; -import { Alert } from 'react-native'; -import { useTranslation } from 'react-i18next'; - -import { needsInAppDestructiveConfirm } from '@/lib/destructive-confirm-platform'; /** - * Sign-out confirmation, including its platform split. + * Sign-out confirmation. * - * Kept in its own module so the Profile screen carries no platform fork: that - * screen is a caller of the shared side-inset entry point, and - * `screen-insets.test.ts` pins every such caller to one cross-platform - * implementation. + * One destructive confirm for both platforms: the in-app + * `DestructiveConfirmDialog` carries the red affordance on iOS and Android + * alike. The native alert cannot be the shared implementation — Android's + * `AlertDialog` paints every button with the theme accent, so + * `Alert.alert`'s `style: 'destructive'` never reaches the screen there — so + * this hook never branches on the platform. * - * Android's native `AlertDialog` paints every button with the theme accent, so - * `Alert.alert`'s `style: 'destructive'` never reaches the screen there. - * Android opens the in-app `DestructiveConfirmDialog` instead; iOS keeps the - * native alert, whose destructive choice already renders red. The platform read - * itself lives in `needsInAppDestructiveConfirm`, shared with the other - * destructive confirmations. + * Kept in its own module so the Profile screen holds no confirmation state: + * that screen reads its side insets from the shared entry point, and + * `screen-insets.test.ts` pins every caller of it to one cross-platform + * implementation. */ export function useSignOutConfirmation(onSignOut: () => void) { - const { t } = useTranslation(); const [confirmVisible, setConfirmVisible] = useState(false); const requestSignOut = useCallback(() => { - if (needsInAppDestructiveConfirm()) { - setConfirmVisible(true); - return; - } - Alert.alert(t('profile.signOutTitle'), t('profile.signOutMessage'), [ - { text: t('common.cancel'), style: 'cancel' }, - { text: t('common.signOut'), style: 'destructive', onPress: onSignOut }, - ]); - }, [onSignOut, t]); + setConfirmVisible(true); + }, []); const dismissConfirm = useCallback(() => { setConfirmVisible(false); diff --git a/apps/mobile/src/lib/destructive-confirm-platform.ts b/apps/mobile/src/lib/destructive-confirm-platform.ts deleted file mode 100644 index 1e7b995efa..0000000000 --- a/apps/mobile/src/lib/destructive-confirm-platform.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Platform } from 'react-native'; - -/** - * Whether a destructive confirmation needs the in-app `DestructiveConfirmDialog` - * instead of `Alert.alert`. - * - * Android's native dialog paints every button with the theme accent, so - * `style: 'destructive'` never reaches the screen there and the destructive - * choice has no distinct affordance; the in-app dialog carries the red variant. - * iOS honors `style: 'destructive'` and keeps the native alert. - * - * The platform read lives in this module rather than in the screen because - * `src/lib/screen-insets.test.ts` holds the Profile screen to one - * platform-agnostic implementation (the same shape as - * `src/lib/pr-review/connect-gate-platform.ts`). - */ -export function needsInAppDestructiveConfirm(): boolean { - return Platform.OS === 'android'; -} diff --git a/apps/mobile/src/lib/rtl-text.test.ts b/apps/mobile/src/lib/rtl-text.test.ts new file mode 100644 index 0000000000..66822e570b --- /dev/null +++ b/apps/mobile/src/lib/rtl-text.test.ts @@ -0,0 +1,49 @@ +import { createElement } from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +import { containsJoinedScript, NATURAL_LETTER_SPACING, textLetterSpacing } from './rtl-text'; + +// `rtl-text` reads I18nManager at call time; the native module itself is not +// loadable under Node, so keep it out of the pure project's module graph. +vi.mock('react-native', () => ({ I18nManager: { isRTL: false } })); + +describe('containsJoinedScript', () => { + it.each([ + ['Arabic heading', 'التفصيلات'], + ['Arabic section label', 'أعلام المميزات'], + ['Farsi', 'تنظیمات'], + ['Urdu', 'ترجیحات'], + ['Kurdish (Sorani)', 'ڕێکخستنەکان'], + ['Pashto', 'تنظیمات'], + ['Arabic presentation form', '\uFB50\uFB51'], + ['a Latin run quoting an Arabic word', 'Saved · محفوظ'], + ])('reads a joined script in %s', (_name, text) => { + expect(containsJoinedScript(text)).toBe(true); + }); + + it.each([ + ['English', 'Preferences'], + ['Hebrew (right-to-left, not joined)', 'הגדרות'], + ['Greek', 'Ρυθμίσεις'], + ['Cyrillic', 'Настройки'], + ['a number', '1.0.12'], + ])('does not read a joined script in %s', (_name, text) => { + expect(containsJoinedScript(text)).toBe(false); + }); + + it('reads every string in a child array and ignores non-text children', () => { + expect(containsJoinedScript(['المظهر', undefined, 3, null])).toBe(true); + expect(containsJoinedScript(['Appearance', 3])).toBe(false); + expect(containsJoinedScript(3)).toBe(false); + }); + + it('leaves a nested element to its own run', () => { + // A nested Text is a separate run and applies its own letter spacing. + expect(containsJoinedScript(createElement('Text', null, 'التفصيلات'))).toBe(false); + }); + + it('returns the natural-spacing override only for a joined run', () => { + expect(textLetterSpacing('المظهر')).toEqual(NATURAL_LETTER_SPACING); + expect(textLetterSpacing('Appearance')).toBeUndefined(); + }); +}); diff --git a/apps/mobile/src/lib/rtl-text.ts b/apps/mobile/src/lib/rtl-text.ts index 5fe7e86723..45d98f7da8 100644 --- a/apps/mobile/src/lib/rtl-text.ts +++ b/apps/mobile/src/lib/rtl-text.ts @@ -1,3 +1,4 @@ +import { type ReactNode } from 'react'; import { I18nManager, type TextStyle } from 'react-native'; /** @@ -35,6 +36,46 @@ export function withRtlWritingDirection(style: TextStyle | undefined): TextStyle return style ? { ...RTL_WRITING_DIRECTION, ...style } : RTL_WRITING_DIRECTION; } +/** + * Arabic, Arabic Supplement, Arabic Extended-A, and the two Arabic + * Presentation Forms blocks. The interface's joined-script languages (Arabic, + * Farsi, Urdu, Kurdish, Pashto) all write in this script, and Arabic text + * quoted inside any other catalog does too. + */ +const JOINED_SCRIPT = /[\u0600-\u06FF\u0750-\u077F\u08A0-\u08FF\uFB50-\uFDFF\uFE70-\uFEFF]/; + +/** + * Letter spacing stays at its natural width on a joined script: tracking is an + * uppercase-Latin affordance, and on Arabic it inserts space between letters + * that must stay joined, tearing the word apart (seen on the Preferences + * section labels). The script, not the interface direction, decides — Hebrew + * is right-to-left but does not join, and an Arabic run in an English + * interface joins the same way. + * + * Only this `Text` node's own string children are read: a nested `Text` is its + * own run and applies this on its own. + */ +export const NATURAL_LETTER_SPACING: TextStyle = { letterSpacing: 0 }; + +/** True when any direct string child of a text node holds a joined-script glyph. */ +export function containsJoinedScript(children: ReactNode): boolean { + if ( + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- ReactNode has no non-typeof way to reach its plain-string leaf + typeof children === 'string' + ) { + return JOINED_SCRIPT.test(children); + } + if (Array.isArray(children)) { + return children.some(child => containsJoinedScript(child as ReactNode)); + } + return false; +} + +/** The letter-spacing override a text run needs, or `undefined` when Latin tracking is safe. */ +export function textLetterSpacing(children: ReactNode): TextStyle | undefined { + return containsJoinedScript(children) ? NATURAL_LETTER_SPACING : undefined; +} + /** * 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 From 765816ffa427f9c31d4865d36efd28b104852365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Fri, 25 Sep 2026 15:16:07 +0200 Subject: [PATCH 2/5] test(mobile): unmount the Text renderer each mount replaces The joined-script test mounts a Latin tree and then an Arabic one through the module-level renderer, so the first tree stayed mounted: `afterEach` only unmounts the last one. The shared helper now unmounts the tracked tree before it replaces it, and the test reads the Latin styles before the Arabic mount takes its place. --- .../src/components/ui/text.mounted.test.tsx | 54 +++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index 9af0f3bce2..16a6c7594b 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -25,14 +25,22 @@ const { compile } = createRequire(import.meta.url)( let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; -function mount(children: string, style?: TextStyle, className = 'tracking-wide') { +/** The one live renderer's root: any previous tree is unmounted before it is replaced. */ +function renderRoot(element: ReactElement): TestRenderer.ReactTestInstance { + act(() => renderer?.unmount()); + renderer = undefined; act(() => { - renderer = TestRenderer.create(createElement(Text, { className, style }, children)); + renderer = TestRenderer.create(element); }); if (!renderer) { throw new Error('Missing Text renderer'); } - return renderer.root.find(node => Object.is(node.type, 'Text')); + return renderer.root; +} + +function mount(children: string, style?: TextStyle, className = 'tracking-wide') { + const root = renderRoot(createElement(Text, { className, style }, children)); + return root.find(node => Object.is(node.type, 'Text')); } /** The component's own inline styles, flattened the way React Native merges them. */ @@ -65,16 +73,6 @@ async function compiledLetterSpacing(className: string): Promise { return value; } -function mountElement(element: ReactElement) { - act(() => { - renderer = TestRenderer.create(element); - }); - if (!renderer) { - throw new Error('Missing Text renderer'); - } - return renderer.root; -} - // Both assertions are kept: `hostText` reaches the host node to read its inline // style, `hostClasses` reads the resolved class list. function hostText(root: TestRenderer.ReactTestInstance) { @@ -125,13 +123,13 @@ describe('Text joined-script letter spacing', () => { expect(tracking).toBeGreaterThan(0); i18nManager.isRTL = true; - const latin = mount('Settings', undefined, 'tracking-[1.5px]'); - const arabic = mount('التفصيلات', undefined, 'tracking-[1.5px]'); + // Each mount replaces the tracked renderer, so read the Latin tree before + // the Arabic one takes its place. + const latin = ownStyles(mount('Settings', undefined, 'tracking-[1.5px]')); + const arabic = ownStyles(mount('التفصيلات', undefined, 'tracking-[1.5px]')); - expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...ownStyles(latin)])).toBe( - tracking - ); - expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...ownStyles(arabic)])).toBe(0); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...latin])).toBe(tracking); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...arabic])).toBe(0); }); }); @@ -144,7 +142,7 @@ describe('Text mounted letter spacing', () => { isRTL => { i18nManager.isRTL = isRTL; const text = hostText( - mountElement(createElement(Text, { className: 'tracking-[1.5px]' }, 'الجلسات الجارية الآن')) + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'الجلسات الجارية الآن')) ); if (isRTL) { @@ -157,7 +155,7 @@ describe('Text mounted letter spacing', () => { it('leaves Latin children untouched and keeps LTR style undefined', () => { const text = hostText( - mountElement(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) ); expect(text.props.style).toBeUndefined(); @@ -165,7 +163,7 @@ describe('Text mounted letter spacing', () => { it('resets the eyebrow variant tracking for Arabic children in RTL', () => { i18nManager.isRTL = true; - const text = hostText(mountElement(createElement(Text, { variant: 'eyebrow' }, 'عرض الكل'))); + const text = hostText(renderRoot(createElement(Text, { variant: 'eyebrow' }, 'عرض الكل'))); expect(text.props.style).toContainEqual({ letterSpacing: 0 }); }); @@ -173,7 +171,7 @@ describe('Text mounted letter spacing', () => { it('resets the tab label tracking for Arabic children in RTL', () => { i18nManager.isRTL = true; const text = hostText( - mountElement(createElement(Text, { className: 'tracking-[0.2px]' }, 'الرئيسية')) + renderRoot(createElement(Text, { className: 'tracking-[0.2px]' }, 'الرئيسية')) ); expect(text.props.className as string).toContain('tracking-[0.2px]'); @@ -185,7 +183,7 @@ describe('Text mounted letter spacing', () => { it('keeps the RTL paragraph direction and no reset for Latin children', () => { i18nManager.isRTL = true; const text = hostText( - mountElement(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) ); expect(text.props.style).toContainEqual({ writingDirection: 'rtl' }); @@ -201,7 +199,7 @@ describe('Text eyebrow letterspacing', () => { it.each([false, true])('keeps the eyebrow display treatment for Latin copy (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; const classes = hostClasses( - mountElement(createElement(Text, { variant: 'eyebrow' }, 'Live now')) + renderRoot(createElement(Text, { variant: 'eyebrow' }, 'Live now')) ); expect(classes).toEqual( expect.arrayContaining([ @@ -217,7 +215,7 @@ describe('Text eyebrow letterspacing', () => { it.each([false, true])('drops the treatment from Arabic copy in RTL (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; const classes = hostClasses( - mountElement(createElement(Text, { variant: 'eyebrow' }, 'الجلسات الجارية الآن')) + renderRoot(createElement(Text, { variant: 'eyebrow' }, 'الجلسات الجارية الآن')) ); if (isRTL) { expect(classes).not.toContain('uppercase'); @@ -229,7 +227,7 @@ describe('Text eyebrow letterspacing', () => { it('leaves a non-eyebrow variant untouched in either direction', () => { i18nManager.isRTL = true; - const classes = hostClasses(mountElement(createElement(Text, null, 'Live now'))); + const classes = hostClasses(renderRoot(createElement(Text, null, 'Live now'))); expect(classes).not.toContain('uppercase'); expect(classes.some(name => name.startsWith('tracking'))).toBe(false); }); @@ -237,7 +235,7 @@ describe('Text eyebrow letterspacing', () => { it.each([false, true])('applies the same rule to the Eyebrow component (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; const classes = hostClasses( - mountElement(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) + renderRoot(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) ); expect(classes).toEqual(expect.arrayContaining(['text-[10px]'])); if (isRTL) { From 94e1090fa26ae22b2105775c83fd3dd8da84e292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Fri, 25 Sep 2026 15:23:46 +0200 Subject: [PATCH 3/5] test(mobile): keep the merged Text test type-checking The main merge brought in the mono-variant tests, whose `mount(element)` helper this branch replaced with `renderRoot(element)` plus a `mount(children, style, className)` wrapper, so those calls no longer matched the signature. The mono tests now call `renderRoot`. `renderRoot` also unmounts inside the same `act` as the next `create`: assigning `renderer = undefined` between them narrowed it to `never` for the `if (!renderer)` guard and the following `renderer.root` read. --- apps/mobile/src/components/ui/text.mounted.test.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index dcc43a73ff..b6eefc1fd0 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -27,9 +27,8 @@ let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; /** The one live renderer's root: any previous tree is unmounted before it is replaced. */ function renderRoot(element: ReactElement): TestRenderer.ReactTestInstance { - act(() => renderer?.unmount()); - renderer = undefined; act(() => { + renderer?.unmount(); renderer = TestRenderer.create(element); }); if (!renderer) { @@ -258,7 +257,7 @@ describe('Text mono variant in an RTL interface', () => { it('drops the mono family for an Arabic mono variant in RTL', () => { i18nManager.isRTL = true; const classes = hostClasses( - mount(createElement(Text, { variant: 'mono' }, 'الجلسات الجارية الآن')) + renderRoot(createElement(Text, { variant: 'mono' }, 'الجلسات الجارية الآن')) ); expect(classes.some(name => name.startsWith('font-mono'))).toBe(false); @@ -266,7 +265,9 @@ describe('Text mono variant in an RTL interface', () => { it('keeps the mono family for a session id in RTL', () => { i18nManager.isRTL = true; - const classes = hostClasses(mount(createElement(Text, { variant: 'mono' }, 'ses_9f2c1a7b'))); + const classes = hostClasses( + renderRoot(createElement(Text, { variant: 'mono' }, 'ses_9f2c1a7b')) + ); expect(classes).toContain('font-mono-medium'); }); From 60f3398484fe370bf53c980b375e230cc1e29a90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 28 Sep 2026 15:57:24 +0200 Subject: [PATCH 4/5] fix(mobile): key the letter-spacing reset on the script, not the interface The reset followed `I18nManager.isRTL`, so an Arabic label on an English screen kept the Latin tracking and split its joins. Key both script rules on the copy instead: - The letter-spacing reset follows `containsJoinedScript` (the joined Arabic blocks) in either direction. An RTL interface keeps resetting its RTL-script copy, because the RTL catalogs carry no tracked design the Hebrew block could take. - `withoutMonoFamily` follows `hasRtlScript` (Hebrew and Arabic) in either direction: JetBrains Mono ships no glyph of either script. `NATURAL_LETTER_SPACING` replaces the direction-named `RTL_NO_LETTER_SPACING`. `JOINED_SCRIPT` now covers Arabic Extended-B, the block `hasRtlScript` already read for the font rule. Delete `textLetterSpacing`, which no production code called since the reset became script-keyed. --- .../home/section-header.mounted.test.tsx | 22 +++-- .../src/components/ui/text.mounted.test.tsx | 82 ++++++++++--------- .../ui/text.rtl-labels.mounted.test.tsx | 10 ++- .../ui/text.rtl-tracking.mounted.test.tsx | 48 +++++++---- apps/mobile/src/components/ui/text.tsx | 30 +++++-- apps/mobile/src/lib/rtl-text.test.ts | 35 ++------ apps/mobile/src/lib/rtl-text.ts | 67 +++++++-------- 7 files changed, 161 insertions(+), 133 deletions(-) diff --git a/apps/mobile/src/components/home/section-header.mounted.test.tsx b/apps/mobile/src/components/home/section-header.mounted.test.tsx index 22b7388bbc..123da2cedc 100644 --- a/apps/mobile/src/components/home/section-header.mounted.test.tsx +++ b/apps/mobile/src/components/home/section-header.mounted.test.tsx @@ -76,9 +76,10 @@ describe('SectionHeader mounted layout', () => { expect(label.props.maxFontSizeMultiplier).toBeUndefined(); expect(label.props.adjustsFontSizeToFit).not.toBe(true); expect(label.children).toEqual(['Live now']); - // The tracked class stays for the Latin design; the RTL letter-spacing - // reset applies to RTL-script copy only, so this Latin label keeps its - // tracking (see lib/rtl-text.ts and text.rtl-labels.mounted.test.tsx). + // The tracked class stays for the Latin design; the reset lands on a joined + // script in either direction and on RTL-script copy inside an RTL + // interface, so this Latin label keeps its tracking (see lib/rtl-text.ts and + // text.rtl-labels.mounted.test.tsx). if (isRTL) { expect(label.props.style).toContainEqual({ writingDirection: 'rtl' }); expect(label.props.style).not.toContainEqual({ letterSpacing: 0 }); @@ -116,10 +117,12 @@ describe('SectionHeader mounted layout', () => { expect(text.children).toEqual(['See all']); }); - // The letter-spacing reset belongs to an RTL interface: in this LTR screen - // the eyebrow label and the action keep `tracking-[1.5px]`, and no inline - // style overrides the class (the RTL case below pins the reset). - it('keeps the tracked letter-spacing on the Arabic label and action in LTR', () => { + // The letter-spacing reset belongs to the script: a joined script takes it in + // either direction, so this LTR screen keeps the tracked class on the element + // and the inline reset draws it inert. The mono family goes with it, because + // JetBrains Mono ships no Arabic glyph (the RTL case below pins the same + // treatment). + it('keeps the tracked class and resets the joined-script label and action in LTR', () => { const root = mount( createElement(SectionHeader, { label: 'الجلسات الجارية الآن', @@ -134,8 +137,9 @@ describe('SectionHeader mounted layout', () => { const actionText = action.find(node => Object.is(node.type, 'Text')); expect((label.props.className as string).split(' ')).toContain('tracking-[1.5px]'); - expect(label.props.style).toBeUndefined(); - expect(actionText.props.style).toBeUndefined(); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); + expect(actionText.props.style).toContainEqual({ letterSpacing: 0 }); + expect((actionText.props.className as string).split(' ')).not.toContain('font-mono-medium'); }); it('renders Arabic labels without the mono family or letter spacing in RTL', () => { diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index b6eefc1fd0..83f5c4aa4d 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -93,9 +93,9 @@ afterEach(() => { renderer = undefined; }); -// The reset belongs to the RTL interface: RTL-script copy takes it there, while -// a joined script in an LTR screen keeps the tracking its class asks for (the -// `hasRtlScript` rule in `Text`; `text.rtl-labels` pins that LTR case). +// The reset belongs to the script, not the interface: a joined script takes it +// in either direction, and an RTL interface resets its RTL-script copy as well +// (the `NATURAL_LETTER_SPACING` rule in `Text`). describe('Text joined-script letter spacing', () => { it('keeps Latin tracking in a left-to-right interface', () => { const node = mount('Preferences'); @@ -117,40 +117,35 @@ describe('Text joined-script letter spacing', () => { expect(styles.at(-1)).toEqual({ letterSpacing: 2 }); }); - it('overrides the tracking the app compiles, on a joined run in RTL only', async () => { - const tracking = await compiledLetterSpacing('tracking-[1.5px]'); - expect(tracking).toBeGreaterThan(0); + it.each([false, true])( + 'overrides the tracking the app compiles, on a joined run (isRTL=%s)', + async isRTL => { + const tracking = await compiledLetterSpacing('tracking-[1.5px]'); + expect(tracking).toBeGreaterThan(0); - i18nManager.isRTL = true; - // Each mount replaces the tracked renderer, so read the Latin tree before - // the Arabic one takes its place. - const latin = ownStyles(mount('Settings', undefined, 'tracking-[1.5px]')); - const arabic = ownStyles(mount('التفصيلات', undefined, 'tracking-[1.5px]')); + i18nManager.isRTL = isRTL; + // Each mount replaces the tracked renderer, so read the Latin tree before + // the Arabic one takes its place. + const latin = ownStyles(mount('Settings', undefined, 'tracking-[1.5px]')); + const arabic = ownStyles(mount('التفصيلات', undefined, 'tracking-[1.5px]')); - expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...latin])).toBe(tracking); - expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...arabic])).toBe(0); - }); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...latin])).toBe(tracking); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...arabic])).toBe(0); + } + ); }); describe('Text mounted letter spacing', () => { - // The reset belongs to the RTL interface: RTL-script copy takes it there, - // while a joined script in an LTR screen keeps the tracking its class asks - // for (`text.rtl-labels` pins that LTR case). - it.each([false, true])( - 'resets the tracking for Arabic children in RTL only (isRTL=%s)', - isRTL => { - i18nManager.isRTL = isRTL; - const text = hostText( - renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'الجلسات الجارية الآن')) - ); + // The reset belongs to the script: a joined script takes it in either + // direction, and an RTL interface resets its RTL-script copy as well. + it.each([false, true])('resets the tracking for Arabic children (isRTL=%s)', isRTL => { + i18nManager.isRTL = isRTL; + const text = hostText( + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'الجلسات الجارية الآن')) + ); - if (isRTL) { - expect(text.props.style).toContainEqual({ letterSpacing: 0 }); - } else { - expect(text.props.style).toBeUndefined(); - } - } - ); + expect(text.props.style).toContainEqual({ letterSpacing: 0 }); + }); it('leaves Latin children untouched and keeps LTR style undefined', () => { const text = hostText( @@ -211,15 +206,24 @@ describe('Text eyebrow letterspacing', () => { ); }); - it.each([false, true])('drops the treatment from Arabic copy in RTL (RTL=%s)', isRTL => { + it.each([false, true])('drops the mono family from Arabic copy (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; - const classes = hostClasses( + const node = hostText( renderRoot(createElement(Text, { variant: 'eyebrow' }, 'الجلسات الجارية الآن')) ); + const classes = String(node.props.className).split(' '); + + // The mono family ships no Arabic glyph and the reset is the script's, so + // both hold in either direction. + expect(classes.some(name => name.startsWith('font-mono'))).toBe(false); + expect(ownStyles(node)).toContainEqual({ letterSpacing: 0 }); + if (isRTL) { expect(classes).not.toContain('uppercase'); expect(classes.some(name => name.startsWith('tracking'))).toBe(false); } else { + // The variant's own class stays on the element in an LTR interface and + // the reset draws its tracking inert (NativeWind merges the class first). expect(classes).toEqual(expect.arrayContaining(['uppercase', 'tracking-[1.5px]'])); } }); @@ -250,12 +254,12 @@ describe('Text eyebrow letterspacing', () => { }); }); -describe('Text mono variant in an RTL interface', () => { - // The mono variant carries a font that ships no RTL-script glyphs, so RTL - // copy loses the family in RTL while a Latin run such as a session id keeps - // it. - it('drops the mono family for an Arabic mono variant in RTL', () => { - i18nManager.isRTL = true; +describe('Text mono variant in either interface', () => { + // The mono variant carries a font that ships no RTL-script glyphs, so + // RTL-script copy loses the family in either direction while a Latin run such + // as a session id keeps it. + it.each([false, true])('drops the mono family for an Arabic mono variant (RTL=%s)', isRTL => { + i18nManager.isRTL = isRTL; const classes = hostClasses( renderRoot(createElement(Text, { variant: 'mono' }, 'الجلسات الجارية الآن')) ); diff --git a/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx b/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx index cc231b4c4e..065f8416af 100644 --- a/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx @@ -88,16 +88,18 @@ describe('Text eyebrow in an RTL interface', () => { expect(classes).toContain('font-mono-medium'); expect(classes).toContain('tracking-[1.5px]'); - expect(label.props.style).toEqual([{ writingDirection: 'rtl' }, undefined, undefined]); + expect(label.props.style).toEqual([{ writingDirection: 'rtl' }, undefined]); }); - it('keeps the mono family and adds no letter spacing for Arabic in an LTR interface', () => { + it('drops the mono family and adds the reset for Arabic in an LTR interface', () => { i18nManager.isRTL = false; const label = hostText(mount(createElement(Text, { variant: 'eyebrow' }, ARABIC))); const classes = (label.props.className as string).split(' '); - expect(classes).toContain('font-mono-medium'); - expect(label.props.style).toBeUndefined(); + // Neither rule is the interface's: the script cannot take the tracking and + // the mono family ships no glyph of it, in either direction. + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); }); it('applies the same rule to the Eyebrow wrapper', () => { diff --git a/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx b/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx index de5f6a84bc..0aef4dbb5d 100644 --- a/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx @@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Eyebrow } from '@/components/ui/eyebrow'; import { Text } from '@/components/ui/text'; -import { RTL_NO_LETTER_SPACING, RTL_WRITING_DIRECTION } from '@/lib/rtl-text'; +import { NATURAL_LETTER_SPACING, RTL_WRITING_DIRECTION } from '@/lib/rtl-text'; const i18nManager = vi.hoisted(() => ({ isRTL: false })); vi.mock('react-native', () => ({ @@ -44,17 +44,20 @@ afterEach(() => { }); // The tracked classes a caller puts on a label: the bottom tab labels and any -// other `className`-supplied `tracking-*`. They stay on the element in RTL and -// the reset below zeroes their spacing. The eyebrow variant and the -// section-header action own their Latin display treatment and drop it in RTL -// instead (see `Text`'s eyebrow variant). +// other `className`-supplied `tracking-*`. They stay on the element and the +// reset below zeroes their spacing wherever the script asks for it — a joined +// script in either direction, and any RTL script inside an RTL interface. The +// eyebrow variant and the section-header action own their Latin display +// treatment and drop it in RTL instead (see `Text`'s eyebrow variant). const TRACKED_CLASSES = ['tracking-[1.5px]', 'tracking-[0.2px]'] as const; // The Hebrew eyebrow copy from `he.json` (`home.agentSessions`). Hebrew is a -// shipped RTL locale and not Arabic script, so the reset has to reach it too. +// shipped RTL locale and not Arabic script, so an RTL interface resets it while +// an LTR interface keeps its tracking (`containsJoinedScript` reads the joined +// scripts alone; `hasRtlScript` reads Hebrew too, for the mono family). const HEBREW = 'פעילים עכשיו'; -describe('Text tracked labels in RTL', () => { +describe('Text tracked labels', () => { it.each(TRACKED_CLASSES)( 'draws %s with no letter spacing while a tracked class stays on the element', trackedClass => { @@ -64,7 +67,7 @@ describe('Text tracked labels in RTL', () => { expect(hostText(root).props.className as string).toContain(trackedClass); expect(root.findAll(node => Object.is(node.type, 'Text'))).toHaveLength(1); expect(hostStyle(root)).toContainEqual(RTL_WRITING_DIRECTION); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); } ); @@ -73,7 +76,7 @@ describe('Text tracked labels in RTL', () => { const root = mount(createElement(Text, { className: 'tracking-[0.2px]' }, HEBREW)); expect(hostText(root).props.className as string).toContain('tracking-[0.2px]'); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); }); it('drops the eyebrow Latin display treatment from Hebrew copy', () => { @@ -83,7 +86,7 @@ describe('Text tracked labels in RTL', () => { const className = hostText(root).props.className as string; expect(className.split(' ')).not.toContain('uppercase'); expect(className).not.toContain('tracking'); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); }); it('leaves no non-zero letter spacing on a tracked label in any class order', () => { @@ -113,10 +116,10 @@ describe('Text tracked labels in RTL', () => { ); expect(hostStyle(root)).toContainEqual(callerStyle); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); }); - it('does not touch a tracked label in an LTR interface', () => { + it('does not touch a tracked Latin label in an LTR interface', () => { i18nManager.isRTL = false; const root = mount(createElement(Text, { className: 'tracking-[1.5px]' }, 'Explore')); @@ -124,6 +127,23 @@ describe('Text tracked labels in RTL', () => { expect(hostText(root).props.style).toBeUndefined(); }); + it('resets a joined script in an LTR interface too', () => { + i18nManager.isRTL = false; + const root = mount(createElement(Text, { className: 'tracking-[1.5px]' }, 'استكشف')); + + expect(hostText(root).props.className as string).toContain('tracking-[1.5px]'); + expect(hostStyle(root)).not.toContainEqual(RTL_WRITING_DIRECTION); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); + }); + + it('keeps the tracking of a Hebrew label in an LTR interface, whose script does not join', () => { + i18nManager.isRTL = false; + const root = mount(createElement(Text, { className: 'tracking-[0.2px]' }, HEBREW)); + + expect(hostText(root).props.className as string).toContain('tracking-[0.2px]'); + expect(hostText(root).props.style).toBeUndefined(); + }); + it('applies the same reset to the shared Eyebrow label, whose own display class is dropped in RTL', () => { // The eyebrow's tracking class is LTR-only (text.tsx EYEBROW_LATIN_DISPLAY): // an RTL eyebrow drops it and relies on the RTL letter-spacing reset. @@ -159,7 +179,7 @@ describe('Text tracked labels in RTL', () => { // the RTL eyebrow drops `font-mono-medium` as well (`withoutMonoFamily`), // matching the LTR-only display treatment it just lost. expect(classes.some(name => name.startsWith('font-mono'))).toBe(false); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); }); it('neutralizes a caller-supplied tracked class on the shared Eyebrow label', () => { @@ -177,6 +197,6 @@ describe('Text tracked labels in RTL', () => { // The variant's own letter-spaced class is dropped, so the caller's is the // only tracking class the label carries. expect(classes.filter(name => name.startsWith('tracking'))).toHaveLength(1); - expect(hostStyle(root)).toContainEqual(RTL_NO_LETTER_SPACING); + expect(hostStyle(root)).toContainEqual(NATURAL_LETTER_SPACING); }); }); diff --git a/apps/mobile/src/components/ui/text.tsx b/apps/mobile/src/components/ui/text.tsx index f80589b9ab..a49e8a6bfd 100644 --- a/apps/mobile/src/components/ui/text.tsx +++ b/apps/mobile/src/components/ui/text.tsx @@ -4,8 +4,9 @@ import * as React from 'react'; import { I18nManager, Text as RNText, type Role } from 'react-native'; import { + containsJoinedScript, hasRtlScript, - RTL_NO_LETTER_SPACING, + NATURAL_LETTER_SPACING, RTL_WRITING_DIRECTION, withoutMonoFamily, } from '@/lib/rtl-text'; @@ -59,7 +60,9 @@ const ARIA_LEVEL = { * ships Arabic-script languages and Hebrew): `letter-spacing` pulls a * cursive script apart — an Arabic eyebrow renders 'الجلسات' as 'ال جلسا ت' * — and that copy also drops the mono family (see `withoutMonoFamily`). - * Latin copy, and RTL-script copy in an LTR interface, keep the treatment. + * Latin copy keeps the treatment in either direction, and in an LTR interface + * the class stays on a joined-script label while the letter-spacing reset + * draws it inert (`NATURAL_LETTER_SPACING`, `text.mounted.test.tsx`). * * Exported so the eyebrow-scale labels rendered outside the variant — the * `SectionHeader` action link — carry the identical treatment instead of a @@ -82,7 +85,22 @@ function Text({ const textClass = React.useContext(TextClassContext); const Component = asChild ? Slot.Text : RNText; const isRTL = I18nManager.isRTL; + // The font family is the script's, not the interface's: JetBrains Mono ships + // no Arabic or Hebrew glyphs, so the copy that uses either drops the family + // in both directions (`withoutMonoFamily`). const isRtlScript = hasRtlScript(props.children); + // Letter-spacing is the script's too. A joined script cannot take it in + // either direction: an Arabic label on an LTR screen keeps its joins. An RTL + // interface resets its RTL-script copy as well — a Hebrew label there would + // take a spacing no Hebrew reader asked for — while Latin copy, in either + // direction, keeps the tracking its class asks for (`NATURAL_LETTER_SPACING`). + const resetsTracking = containsJoinedScript(props.children) || (isRTL && isRtlScript); + // The caller's own style stays last, so an explicit `letterSpacing` outranks + // the reset. + const ownStyles = [ + isRTL ? RTL_WRITING_DIRECTION : undefined, + resetsTracking ? NATURAL_LETTER_SPACING : undefined, + ].filter(style => style !== undefined); const classes = cn( textVariants({ variant }), variant === 'eyebrow' && !(isRTL && isRtlScript) && EYEBROW_LATIN_DISPLAY, @@ -91,15 +109,11 @@ function Text({ ); return ( 0 ? [...ownStyles, props.style] : props.style} /> ); } diff --git a/apps/mobile/src/lib/rtl-text.test.ts b/apps/mobile/src/lib/rtl-text.test.ts index 86ef51b77f..fab2f7e429 100644 --- a/apps/mobile/src/lib/rtl-text.test.ts +++ b/apps/mobile/src/lib/rtl-text.test.ts @@ -1,14 +1,7 @@ import { createElement } from 'react'; import { describe, expect, it, vi } from 'vitest'; -import { - containsJoinedScript, - hasRtlScript, - JOINED_SCRIPT, - NATURAL_LETTER_SPACING, - textLetterSpacing, - withoutMonoFamily, -} from './rtl-text'; +import { containsJoinedScript, hasRtlScript, JOINED_SCRIPT, withoutMonoFamily } from './rtl-text'; // `rtl-text` imports `I18nManager` for its direction helpers; the real module // is Flow-syntax source this node project cannot load. @@ -83,7 +76,7 @@ describe('hasRtlScript', () => { }); describe('JOINED_SCRIPT', () => { - it.each(['\u0600', '\u0750', '\u08A0', '\uFB50', '\uFE70'])( + it.each(['\u0600', '\u0750', '\u0870', '\u08A0', '\uFB50', '\uFE70'])( 'covers the block that starts at %j', value => { expect(JOINED_SCRIPT.test(value)).toBe(true); @@ -108,6 +101,7 @@ describe('containsJoinedScript', () => { it.each([ ['Arabic base', '\u0600'], ['Arabic Supplement', '\u0750'], + ['Arabic Extended-B', '\u0870'], ['Arabic Extended-A', '\u08A0'], ['Arabic Presentation Forms-A', '\uFB50'], ['Arabic Presentation Forms-B', '\uFE70'], @@ -115,6 +109,12 @@ describe('containsJoinedScript', () => { expect(containsJoinedScript(value)).toBe(true); }); + // Arabic Extended-B sits between Arabic Supplement and Arabic Extended-A; + // `hasRtlScript` reads it for the mono family, so the reset reads it too. + it('detects a label written only in Arabic Extended-B', () => { + expect(containsJoinedScript(FIRST_EXTENDED_B + LAST_EXTENDED_B)).toBe(true); + }); + it.each(['الجلسات الجارية الآن', 'الرئيسية', 'الوكلاء', 'الملف الشخصي', 'عرض الكل'])( 'detects Arabic in %j', value => { @@ -185,20 +185,3 @@ describe('withoutMonoFamily', () => { expect(withoutMonoFamily('font-mono-bold text-sm')).toBe('font-mono-bold text-sm'); }); }); - -describe('textLetterSpacing', () => { - it('returns the natural-spacing override only for a joined run', () => { - expect(textLetterSpacing('المظهر')).toEqual(NATURAL_LETTER_SPACING); - expect(textLetterSpacing('Appearance')).toBeUndefined(); - }); - - it('returns the natural spacing for a joined script', () => { - expect(textLetterSpacing('الرئيسية')).toBe(NATURAL_LETTER_SPACING); - expect(textLetterSpacing(['عرض', ' الكل'])).toBe(NATURAL_LETTER_SPACING); - }); - - it('returns undefined without a joined script', () => { - expect(textLetterSpacing('Live now')).toBeUndefined(); - expect(textLetterSpacing(4)).toBeUndefined(); - }); -}); diff --git a/apps/mobile/src/lib/rtl-text.ts b/apps/mobile/src/lib/rtl-text.ts index f9851f746a..56ebb837ca 100644 --- a/apps/mobile/src/lib/rtl-text.ts +++ b/apps/mobile/src/lib/rtl-text.ts @@ -44,31 +44,16 @@ export function withRtlInputAlignment( return [RTL_INPUT_ALIGNMENT, style]; } -/** - * Letter-spacing — Tailwind's `tracking-*` — is a Latin typographic device: - * it opens every glyph from its neighbour, and the RTL scripts the app ships - * do not take it. An Arabic-script word is one connected shape, so a tracked - * label renders its letters as isolated forms ('استكشف'), and a Hebrew word - * takes a spacing no Hebrew reader asked for; a letter-spacing of 0 keeps the - * paragraph's natural spacing. An explicit style outranks a `className` rule, - * so applying this leaves the tracked class the LTR design owns inert in RTL. - * - * `@/components/ui/text` applies this to the RTL-script copy that needs it, - * in the same RTL style array as `RTL_WRITING_DIRECTION` (see - * `hasRtlScript`): Latin copy in an RTL interface keeps its tracking. - */ -export const RTL_NO_LETTER_SPACING: TextStyle = { letterSpacing: 0 }; - /** The right-to-left script blocks the app ships: the Hebrew block and its * presentation forms, then Arabic, Arabic Supplement, Arabic Extended-B, - * Arabic Extended-A, and the Arabic Presentation Forms-A and -B. Any - * character in them means the copy is not Latin script: it needs the no-track - * reset, and a joining script needs a font with its glyphs (see - * `withoutMonoFamily`). */ + * Arabic Extended-A, and the Arabic Presentation Forms-A and -B. The mono + * family ships no glyph of either script, so any character in them means the + * copy needs a font with its glyphs (see `withoutMonoFamily`). */ const RTL_SCRIPT = /[\u0590-\u05FF\uFB1D-\uFB4F\u0600-\u06FF\u0750-\u077F\u0870-\u089F\u08A0-\u08FF\uFB50-\uFDFF\uFE70-\uFEFF]/; -/** Whether a React child tree contains copy in a right-to-left script. */ +/** Whether a React child tree contains copy in a right-to-left script: the copy + * the mono family cannot draw, whatever the interface direction is. */ export function hasRtlScript(node: ReactNode): boolean { if (Array.isArray(node)) { return node.some((child: ReactNode) => hasRtlScript(child)); @@ -85,9 +70,10 @@ export function hasRtlScript(node: ReactNode): boolean { /** * JetBrains Mono ships no Arabic or Hebrew glyphs, so a fallback renders the - * word one character at a time (`ا س ت ك ش ف`); the system font the rest of an - * RTL screen uses keeps the joins. Drop the `font-mono*` utility, keeping the - * size, color and weight classes around it. + * word one character at a time (`ا س ت ك ش ف`); the system font the app draws + * its other copy with keeps the joins. The font ships the glyphs of neither + * script, so the rule is the script and not the interface direction. Drop the + * `font-mono*` utility, keeping the size, color and weight classes around it. */ export function withoutMonoFamily(className: string): string { return className @@ -124,13 +110,29 @@ export const LTR_TEXT_DIRECTION: TextStyle = { direction: 'ltr', writingDirectio * Latin display device — opens every glyph from its neighbour and splits a word * mid-shape («الوكلاء» draws as «الوكلا ء»). This is the script, not the * interface direction: a Latin run inside an Arabic interface (`KiloClaw`, - * `PR`) joins nothing and keeps its tracking. Arabic, Arabic Supplement, Arabic - * Extended-A and the two Arabic Presentation Forms blocks are the ranges a - * joined Arabic run arrives in. + * `PR`) joins nothing and keeps its tracking. Arabic, Arabic Supplement, + * Arabic Extended-B, Arabic Extended-A and the two Arabic Presentation Forms + * blocks are the ranges a joined Arabic run arrives in. Hebrew is right to + * left and joins nothing, so it stays out of this range. */ -export const JOINED_SCRIPT = /[\u0600-\u06FF\u0750-\u077F\u08A0-\u08FF\uFB50-\uFDFF\uFE70-\uFEFF]/; +export const JOINED_SCRIPT = + /[\u0600-\u06FF\u0750-\u077F\u0870-\u089F\u08A0-\u08FF\uFB50-\uFDFF\uFE70-\uFEFF]/; -/** No added advance between glyphs, what a joined script's shaping expects. */ +/** + * Letter-spacing reset: no added advance between glyphs, what a joined script's + * shaping expects. `letter-spacing` — Tailwind's `tracking-*` — is a Latin + * typographic device, so the copy that cannot take it gets this instead. Two + * rules reach for it in `@/components/ui/text`: a joined script in either + * direction (see `containsJoinedScript`), and any RTL script the app ships + * inside an RTL interface, where a Hebrew word would take a spacing no Hebrew + * reader asked for (see `hasRtlScript`). Latin copy in an RTL interface keeps + * its tracking. + * + * The reset lands in the style array behind the caller's own style, so an + * explicit `letterSpacing` still wins, and ahead of the class a `className` + * rule compiles to, so a tracked class stays on the element but draws inert + * (`text.rtl-tracking.mounted.test.tsx`). + */ export const NATURAL_LETTER_SPACING: TextStyle = { letterSpacing: 0 }; /** A `ReactNode` string member, the only child a `Text` lays out as one run. */ @@ -148,6 +150,10 @@ function isChildArray(children: ReactNode): children is ReactNode[] { * Whether a direct string child holds a glyph of a joined script. Array * children recurse; a nested `Text` is its own run and applies the rule itself, * and numbers, functions and elements are not strings. + * + * `@/components/ui/text` reads this to decide the letter-spacing reset, in + * either interface direction: the joined shape is what the tracking pulls + * apart. */ export function containsJoinedScript(children: ReactNode): boolean { if (isStringChild(children)) { @@ -158,8 +164,3 @@ export function containsJoinedScript(children: ReactNode): boolean { } return false; } - -/** The natural spacing for a joined script, or nothing to override. */ -export function textLetterSpacing(children: ReactNode): TextStyle | undefined { - return containsJoinedScript(children) ? NATURAL_LETTER_SPACING : undefined; -} From c7a6e9c744bd2293fa9c0527c89d93db55c85633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 28 Sep 2026 16:07:11 +0200 Subject: [PATCH 5/5] fix(mobile): keep the three-entry RTL style array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two RTL assertions outside this change — `chat-composer.test.ts` and `new-session-prompt-initial-prompt.test.ts` — pin the array `[{ writingDirection: 'rtl' }, undefined, undefined]`. A filtered array dropped the trailing entry and failed both. Restore the array `main` ships: the RTL run keeps its three entries, and a joined-script run in an LTR interface carries a leading `undefined`. --- .../ui/text.rtl-labels.mounted.test.tsx | 2 +- apps/mobile/src/components/ui/text.tsx | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx b/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx index 065f8416af..c44a647aa3 100644 --- a/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx @@ -88,7 +88,7 @@ describe('Text eyebrow in an RTL interface', () => { expect(classes).toContain('font-mono-medium'); expect(classes).toContain('tracking-[1.5px]'); - expect(label.props.style).toEqual([{ writingDirection: 'rtl' }, undefined]); + expect(label.props.style).toEqual([{ writingDirection: 'rtl' }, undefined, undefined]); }); it('drops the mono family and adds the reset for Arabic in an LTR interface', () => { diff --git a/apps/mobile/src/components/ui/text.tsx b/apps/mobile/src/components/ui/text.tsx index a49e8a6bfd..883375221a 100644 --- a/apps/mobile/src/components/ui/text.tsx +++ b/apps/mobile/src/components/ui/text.tsx @@ -95,12 +95,6 @@ function Text({ // take a spacing no Hebrew reader asked for — while Latin copy, in either // direction, keeps the tracking its class asks for (`NATURAL_LETTER_SPACING`). const resetsTracking = containsJoinedScript(props.children) || (isRTL && isRtlScript); - // The caller's own style stays last, so an explicit `letterSpacing` outranks - // the reset. - const ownStyles = [ - isRTL ? RTL_WRITING_DIRECTION : undefined, - resetsTracking ? NATURAL_LETTER_SPACING : undefined, - ].filter(style => style !== undefined); const classes = cn( textVariants({ variant }), variant === 'eyebrow' && !(isRTL && isRtlScript) && EYEBROW_LATIN_DISPLAY, @@ -113,7 +107,15 @@ function Text({ role={variant ? ROLE[variant as keyof typeof ROLE] : undefined} aria-level={variant ? ARIA_LEVEL[variant as keyof typeof ARIA_LEVEL] : undefined} {...props} - style={ownStyles.length > 0 ? [...ownStyles, props.style] : props.style} + style={ + isRTL || resetsTracking + ? [ + isRTL ? RTL_WRITING_DIRECTION : undefined, + resetsTracking ? NATURAL_LETTER_SPACING : undefined, + props.style, + ] + : props.style + } /> ); }