diff --git a/apps/mobile/src/components/agents/session-filter-button.tsx b/apps/mobile/src/components/agents/session-filter-button.tsx index bd28112d14..6cae1509ff 100644 --- a/apps/mobile/src/components/agents/session-filter-button.tsx +++ b/apps/mobile/src/components/agents/session-filter-button.tsx @@ -29,10 +29,11 @@ type SessionFilterButtonProps = { * carries `shrink-0`, so the header row cannot squeeze it under the audit's * 28dp floor. * - * The sides are spelled out rather than a single uniform number: the row's - * sibling control (`session-list-header-actions.tsx`) caps its facing right - * slop against this control's left slop at the row's 14pt gap, and the test - * that guards that invariant reads the insets. + * The sides are spelled out rather than a single uniform number: the agents + * header row caps this control's two horizontal sides at its `gap-4` gap (the + * row mirrors under RTL while `hitSlop` does not, so the cap cannot sit on one + * physical side) and passes the capped insets in through the prop below. The + * test that guards that invariant reads the insets. */ const FILTER_HIT_SLOP = { top: COMPACT_CONTROL_HIT_SLOP_DP, 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 5afefc9975..d9f9100bf3 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 @@ -229,49 +229,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: the header caps the filter's two horizontal + // sides, while the new-session control keeps the shared symmetric slop. + 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: `IconButton` + // and this control both pass a per-side object today. + 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 own 36pt box 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); + const filterBox = boxDp(filter.props.className as string); + expect(filterBox.width).toBeGreaterThanOrEqual(MIN_TAP_TARGET_DP); + expect( + filterBox.width + 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` (inside `slopSideDp`) validates and normalizes either - // shape of React Native's `number | Rect` hitSlop: a control may express one - // dp value for every side or spell out per-side insets. Both header controls - // spell out insets today — the filter writes four equal sides, while the - // new-session control caps its facing (right) side — so the helper has to - // accept either shape rather than assume one. 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. - expect( - slopSideDp(newSession.props.hitSlop, 'right') + slopSideDp(filter.props.hitSlop, '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(newSession.props.hitSlop, 'left') + - slopSideDp(newSession.props.hitSlop, '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 a7bfbc614a..bf017cd5aa 100644 --- a/apps/mobile/src/components/agents/session-list-header-actions.tsx +++ b/apps/mobile/src/components/agents/session-list-header-actions.tsx @@ -8,24 +8,20 @@ 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 -// (`@/lib/a11y/tap-target`), so the new-session control's right side is capped -// at 14 - 8 = 6 and the two facing slops meet at the row's gap without -// overlapping (6 + 8 = 14). 32 + 8 + 6 = 46pt still clears `DESIGN.md:364`'s -// 44pt. The filter's slop is spelled per side too, so that meeting can be -// checked instead of only the smallest of its four sides. -const NEW_SESSION_HIT_SLOP = { - top: COMPACT_CONTROL_HIT_SLOP_DP, - bottom: COMPACT_CONTROL_HIT_SLOP_DP, - left: COMPACT_CONTROL_HIT_SLOP_DP, - right: 6, -}; - +// 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. +// 36 + 6 + 6 = 48pt 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, - right: COMPACT_CONTROL_HIT_SLOP_DP, + left: 6, + right: 6, }; type SessionListHeaderActionsProps = { @@ -50,19 +46,17 @@ 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} ); 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/profile-screen.signout.mounted.test.tsx b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx index f225024a2b..f05b563c7a 100644 --- a/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx +++ b/apps/mobile/src/components/profile-screen.signout.mounted.test.tsx @@ -15,6 +15,7 @@ const insets = vi.hoisted(() => ({ top: 0, bottom: 0, left: 0, right: 0 })); vi.mock('react-native', () => ({ Alert: { alert: alertFn }, + Modal: 'Modal', Platform: { get OS() { return platform.os; @@ -155,11 +156,6 @@ function isType(node: TestRenderer.ReactTestInstance, type: string): boolean { return typeof node.type === 'string' && node.type === type; } -function alertButtons(call: unknown): { style?: string; onPress?: () => void }[] { - const buttons = (call as unknown[] | undefined)?.[2]; - return Array.isArray(buttons) ? (buttons as { style?: string; onPress?: () => void }[]) : []; -} - describe('ProfileScreen sign-out confirmation', () => { beforeEach(() => { signOutFn.mockReset(); @@ -175,49 +171,46 @@ describe('ProfileScreen sign-out confirmation', () => { }); } - // The confirmation is the one shared native alert on both platforms: - // plugins/withAndroidAlertDialogTheme repaints Android's AppCompat dialog with - // the app tokens, and iOS's `UIAlertController` already follows the device - // appearance. No platform mounts an in-app confirmation of its own, so the - // same tap takes the same path whichever value the device reports. - for (const os of ['android', 'ios'] as const) { - it(`opens the shared native alert on ${os}`, async () => { + function dialogButton( + renderer: TestRenderer.ReactTestRenderer, + variant: 'outline' | 'destructive' + ) { + return renderer.root.find(node => isType(node, 'Button') && node.props.variant === variant); + } + + // 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); - expect(alertFn).toHaveBeenCalledTimes(1); - expect(alertFn.mock.calls[0]?.[0]).toBe('Sign out?'); - // Opening the confirmation signs nobody out; only its own choices act. + // 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(); - const buttons = alertButtons(alertFn.mock.calls[0]); - expect(buttons.find(button => button.style === 'cancel')?.onPress).toBeUndefined(); - expect(buttons.some(button => button.style === 'destructive')).toBe(true); - // The alert is the whole confirmation: no in-app dialog renders beside it. - expect( - renderer.root.findAll( - node => isType(node, 'Button') && node.props.variant === 'destructive' - ) - ).toHaveLength(0); - - unmount(); - }); - } - it("signs out only when the alert's destructive choice is pressed", async () => { - const { renderer, unmount } = await renderWithProviders(createElement(ProfileScreen)); - - pressSignOutTile(renderer); + // Cancel keeps the user signed in. + act(() => { + (dialogButton(renderer, 'outline').props as { onPress?: () => void }).onPress?.(); + }); + expect(signOutFn).not.toHaveBeenCalled(); - const destructive = alertButtons(alertFn.mock.calls[0]).find( - button => button.style === 'destructive' - ); - act(() => { - destructive?.onPress?.(); - }); - expect(signOutFn).toHaveBeenCalledTimes(1); + // Reopen: only the destructive control signs out. + pressSignOutTile(renderer); + act(() => { + (dialogButton(renderer, 'destructive').props as { onPress?: () => void }).onPress?.(); + }); + expect(signOutFn).toHaveBeenCalledTimes(1); - unmount(); - }); + unmount(); + } + ); }); diff --git a/apps/mobile/src/components/profile-screen.tsx b/apps/mobile/src/components/profile-screen.tsx index 51bedacf66..125c878aed 100644 --- a/apps/mobile/src/components/profile-screen.tsx +++ b/apps/mobile/src/components/profile-screen.tsx @@ -19,6 +19,7 @@ import { import { Alert, View } from 'react-native'; import Animated, { FadeOut } from 'react-native-reanimated'; +import { DestructiveConfirmDialog } from '@/components/destructive-confirm-dialog'; import { ActionTile } from '@/components/profile-action-tile'; import { CreditsCard } from '@/components/profile-credits-card'; import { QueryError } from '@/components/query-error'; @@ -31,6 +32,7 @@ import { Skeleton } from '@/components/ui/skeleton'; import { Text } from '@/components/ui/text'; import { useDeleteAccount } from '@/components/use-delete-account'; import { useFeedbackPrompt } from '@/components/use-feedback-prompt'; +import { useSignOutConfirmation } from '@/components/use-sign-out-confirmation'; import { i18n } from '@/i18n'; import { FEATURE_FLAG_PR_REVIEW, useFeatureFlag } from '@/lib/analytics/posthog'; import { useAuth } from '@/lib/auth/auth-context'; @@ -84,6 +86,13 @@ export function ProfileScreen() { // the only place the signed-in address renders. const afterInteractions = useAfterInteractions(); const prReviewEnabled = useFeatureFlag(FEATURE_FLAG_PR_REVIEW, true); + // 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() + ); const { data, isLoading, @@ -126,6 +135,11 @@ export function ProfileScreen() { setCode, } = useDeleteAccount(); + // Delete account keeps the native alert: Android's AppCompat dialog takes its + // panel and action accent from the activity theme, which the + // `plugins/withAndroidAlertDialogTheme` prebuild overlay points at the app + // tokens, and iOS renders the same call as a `UIAlertController` that already + // follows the device appearance. const confirmDeleteAccount = () => { Alert.alert(t('profile.deleteAccountTitle'), t('profile.deleteAccountMessage'), [ { text: t('common.cancel'), style: 'cancel' }, @@ -137,22 +151,6 @@ export function ProfileScreen() { ]); }; - // The sign-out confirmation is the shared native alert on both platforms: - // Android's AppCompat dialog takes its panel and action accent from the - // activity theme, which plugins/withAndroidAlertDialogTheme points at the app - // tokens, and iOS renders the same call as a `UIAlertController` that already - // follows the device appearance. - const confirmSignOut = () => { - Alert.alert(t('profile.signOutTitle'), t('profile.signOutMessage'), [ - { text: t('common.cancel'), style: 'cancel' }, - { - text: t('common.signOut'), - style: 'destructive', - onPress: () => void signOut(), - }, - ]); - }; - const showPrivacyChoices = () => { router.push('/(app)/consent?mode=review' as Href); }; @@ -384,7 +382,7 @@ export function ProfileScreen() { icon={LogOut} label={t('common.signOut')} hue="fern" - onPress={confirmSignOut} + onPress={requestSignOut} /> + {confirmVisible && ( + + )} + {feedbackPrompt.promptDialog} ); diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index 3a506f52c5..83f5c4aa4d 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -1,23 +1,34 @@ +/* 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) { + +/** 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 = TestRenderer.create(element); }); if (!renderer) { @@ -26,6 +37,41 @@ function mount(element: ReactElement) { 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. */ +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; +} + // 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) { @@ -41,34 +87,69 @@ beforeEach(() => { (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; i18nManager.isRTL = false; }); + afterEach(() => { act(() => renderer?.unmount()); renderer = undefined; }); -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). +// 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'); + expect(node.props.className).toContain('tracking-wide'); + expect(ownStyles(node)).toEqual([]); + }); + + 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('lets an explicit caller letterSpacing win over the reset', () => { + i18nManager.isRTL = true; + const styles = ownStyles(mount('المظهر', { letterSpacing: 2 })); + expect(styles.at(-1)).toEqual({ letterSpacing: 2 }); + }); + it.each([false, true])( - 'resets the tracking for Arabic children in RTL only (isRTL=%s)', - isRTL => { + '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 = isRTL; - const text = hostText( - mount(createElement(Text, { className: '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]')); - if (isRTL) { - expect(text.props.style).toContainEqual({ letterSpacing: 0 }); - } else { - expect(text.props.style).toBeUndefined(); - } + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...latin])).toBe(tracking); + expect(resolvedLetterSpacing([{ letterSpacing: tracking }, ...arabic])).toBe(0); } ); +}); + +describe('Text mounted letter spacing', () => { + // 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]' }, 'الجلسات الجارية الآن')) + ); + + expect(text.props.style).toContainEqual({ letterSpacing: 0 }); + }); it('leaves Latin children untouched and keeps LTR style undefined', () => { const text = hostText( - mount(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) ); expect(text.props.style).toBeUndefined(); @@ -76,7 +157,7 @@ describe('Text mounted letter spacing', () => { it('resets the eyebrow variant tracking for Arabic children in RTL', () => { i18nManager.isRTL = true; - const text = hostText(mount(createElement(Text, { variant: 'eyebrow' }, 'عرض الكل'))); + const text = hostText(renderRoot(createElement(Text, { variant: 'eyebrow' }, 'عرض الكل'))); expect(text.props.style).toContainEqual({ letterSpacing: 0 }); }); @@ -84,7 +165,7 @@ describe('Text mounted letter spacing', () => { it('resets the tab label tracking for Arabic children in RTL', () => { i18nManager.isRTL = true; const text = hostText( - mount(createElement(Text, { className: 'tracking-[0.2px]' }, 'الرئيسية')) + renderRoot(createElement(Text, { className: 'tracking-[0.2px]' }, 'الرئيسية')) ); expect(text.props.className as string).toContain('tracking-[0.2px]'); @@ -96,7 +177,7 @@ describe('Text mounted letter spacing', () => { it('keeps the RTL paragraph direction and no reset for Latin children', () => { i18nManager.isRTL = true; const text = hostText( - mount(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) + renderRoot(createElement(Text, { className: 'tracking-[1.5px]' }, 'Live now')) ); expect(text.props.style).toContainEqual({ writingDirection: 'rtl' }); @@ -111,7 +192,9 @@ describe('Text eyebrow letterspacing', () => { // RTL interface. it.each([false, true])('keeps the eyebrow display treatment for Latin copy (RTL=%s)', isRTL => { i18nManager.isRTL = isRTL; - const classes = hostClasses(mount(createElement(Text, { variant: 'eyebrow' }, 'Live now'))); + const classes = hostClasses( + renderRoot(createElement(Text, { variant: 'eyebrow' }, 'Live now')) + ); expect(classes).toEqual( expect.arrayContaining([ 'font-mono-medium', @@ -123,22 +206,31 @@ 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( - mount(createElement(Text, { variant: 'eyebrow' }, 'الجلسات الجارية الآن')) + 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]'])); } }); 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(renderRoot(createElement(Text, null, 'Live now'))); expect(classes).not.toContain('uppercase'); expect(classes.some(name => name.startsWith('tracking'))).toBe(false); }); @@ -146,7 +238,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')) + renderRoot(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) ); expect(classes).toEqual(expect.arrayContaining(['text-[10px]'])); if (isRTL) { @@ -162,14 +254,14 @@ 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( - mount(createElement(Text, { variant: 'mono' }, 'الجلسات الجارية الآن')) + renderRoot(createElement(Text, { variant: 'mono' }, 'الجلسات الجارية الآن')) ); expect(classes.some(name => name.startsWith('font-mono'))).toBe(false); @@ -177,7 +269,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'); }); 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..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 @@ -91,13 +91,15 @@ describe('Text eyebrow in an RTL interface', () => { expect(label.props.style).toEqual([{ writingDirection: 'rtl' }, undefined, 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..883375221a 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,16 @@ 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); const classes = cn( textVariants({ variant }), variant === 'eyebrow' && !(isRTL && isRtlScript) && EYEBROW_LATIN_DISPLAY, @@ -91,13 +103,17 @@ function Text({ ); return ( diff --git a/apps/mobile/src/components/use-sign-out-confirmation.ts b/apps/mobile/src/components/use-sign-out-confirmation.ts new file mode 100644 index 0000000000..52e747cf69 --- /dev/null +++ b/apps/mobile/src/components/use-sign-out-confirmation.ts @@ -0,0 +1,35 @@ +import { useCallback, useState } from 'react'; + +/** + * Sign-out confirmation. + * + * 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. + * + * 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 [confirmVisible, setConfirmVisible] = useState(false); + + const requestSignOut = useCallback(() => { + setConfirmVisible(true); + }, []); + + const dismissConfirm = useCallback(() => { + setConfirmVisible(false); + }, []); + + const confirmSignOut = useCallback(() => { + setConfirmVisible(false); + onSignOut(); + }, [onSignOut]); + + return { confirmVisible, requestSignOut, dismissConfirm, confirmSignOut }; +} diff --git a/apps/mobile/src/lib/alert-dialog-platform-parity.test.ts b/apps/mobile/src/lib/alert-dialog-platform-parity.test.ts index d95dbd4ae6..8e620c831f 100644 --- a/apps/mobile/src/lib/alert-dialog-platform-parity.test.ts +++ b/apps/mobile/src/lib/alert-dialog-platform-parity.test.ts @@ -9,8 +9,8 @@ // light/dark appearance and exposes no app-token override, so there is no iOS // half to write. This suite runs the plugin's mods in node and holds the path // to that: the one platform-specific module is the Android plugin, it writes -// the app tokens for day and night, it adds nothing on iOS, and the shared -// sign-out confirmation carries no per-platform branch. +// the app tokens for day and night, it adds nothing on iOS, and neither the +// delete-account native alert nor the sign-out dialog branches on a platform. // eslint-disable-next-line import/no-nodejs-modules -- vitest-only parity check, runs in node, never bundled into the app import { readFileSync } from 'node:fs'; @@ -36,7 +36,7 @@ const withAlertDialogTheme = plugin as unknown as (config: AlertThemeConfig) => const DIRECTORY = fileURLToPath(new URL('./', import.meta.url)); const PLUGIN_PATH = `${DIRECTORY}../../plugins/withAndroidAlertDialogTheme.js`; const CONFIG_PATH = `${DIRECTORY}../../app.config.ts`; -const SIGN_OUT_PATH = `${DIRECTORY}../components/profile-screen.tsx`; +const PROFILE_PATH = `${DIRECTORY}../components/profile-screen.tsx`; /** * A per-platform branch in shared JS: a `Platform.OS`/`Platform.select` check, @@ -174,17 +174,20 @@ describe('one implementation for both platforms on the alert dialog path', () => ); }); - it('runs the one shared confirmation on both platforms', () => { + it('runs the shared native alert on both platforms', () => { expect( readFileSync(CONFIG_PATH, 'utf8').match(/withAndroidAlertDialogTheme/g) ?? [] ).toHaveLength(1); - const confirmation = readFileSync(SIGN_OUT_PATH, 'utf8'); - expect(confirmation, 'the sign-out confirmation is the shared native alert').toMatch( - /Alert\.alert\(t\('profile\.signOutTitle'\)/ + const profile = readFileSync(PROFILE_PATH, 'utf8'); + // Signing out is the in-app `DestructiveConfirmDialog` (see + // `use-sign-out-confirmation.ts`), so `Alert.alert` is not on that path; + // deleting the account still confirms through the shared native alert this + // plugin restyles, on both platforms. + expect(profile, 'the delete-account confirmation is the shared native alert').toMatch( + /Alert\.alert\(t\('profile\.deleteAccountTitle'\)/ + ); + expect(PLATFORM_BRANCH.test(profile), 'profile-screen.tsx carries a per-platform branch').toBe( + false ); - expect( - PLATFORM_BRANCH.test(confirmation), - 'profile-screen.tsx carries a per-platform branch' - ).toBe(false); }); }); diff --git a/apps/mobile/src/lib/rtl-text.test.ts b/apps/mobile/src/lib/rtl-text.test.ts index 6d586d2939..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); @@ -92,16 +85,23 @@ describe('JOINED_SCRIPT', () => { }); describe('containsJoinedScript', () => { - it.each(['الجلسات الجارية الآن', 'الرئيسية', 'الوكلاء', 'الملف الشخصي', 'عرض الكل'])( - 'detects Arabic in %j', - value => { - expect(containsJoinedScript(value)).toBe(true); - } - ); + 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([ ['Arabic base', '\u0600'], ['Arabic Supplement', '\u0750'], + ['Arabic Extended-B', '\u0870'], ['Arabic Extended-A', '\u08A0'], ['Arabic Presentation Forms-A', '\uFB50'], ['Arabic Presentation Forms-B', '\uFE70'], @@ -109,11 +109,40 @@ 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 => { + expect(containsJoinedScript(value)).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('detects Arabic inside a mixed array of strings', () => { expect(containsJoinedScript(['عرض', ' ', 'الكل'])).toBe(true); expect(containsJoinedScript(['Live now', 'عرض الكل'])).toBe(true); }); + 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('is false for Latin, Hebrew, digits and punctuation', () => { expect(containsJoinedScript('Live now')).toBe(false); expect(containsJoinedScript('SEE ALL')).toBe(false); @@ -134,6 +163,11 @@ describe('containsJoinedScript', () => { expect(containsJoinedScript(4)).toBe(false); expect(containsJoinedScript(createElement('Text', null, 'الرئيسية'))).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); + }); }); describe('withoutMonoFamily', () => { @@ -151,15 +185,3 @@ describe('withoutMonoFamily', () => { expect(withoutMonoFamily('font-mono-bold text-sm')).toBe('font-mono-bold text-sm'); }); }); - -describe('textLetterSpacing', () => { - 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; -}