From ceab4ebe0e4cdd1c11015ea9b43b7b438fc128bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 23 Sep 2026 07:56:13 +0200 Subject: [PATCH] fix(mobile): render Arabic Home labels whole in RTL https://github.com/Kilo-Org/cloud/pull/6495 --- .../components/agents/chat-composer.test.ts | 2 +- .../new-session-prompt-initial-prompt.test.ts | 2 +- .../home/section-header.mounted.test.tsx | 34 +++++- .../src/components/ui/text.mounted.test.tsx | 29 ++++- .../ui/text.rtl-labels.mounted.test.tsx | 112 ++++++++++++++++++ .../ui/text.rtl-tracking.mounted.test.tsx | 29 ++++- apps/mobile/src/components/ui/text.tsx | 36 ++++-- apps/mobile/src/lib/rtl-text.test.ts | 49 ++++++++ apps/mobile/src/lib/rtl-text.ts | 54 +++++++-- 9 files changed, 315 insertions(+), 32 deletions(-) create mode 100644 apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx create mode 100644 apps/mobile/src/lib/rtl-text.test.ts diff --git a/apps/mobile/src/components/agents/chat-composer.test.ts b/apps/mobile/src/components/agents/chat-composer.test.ts index 951d818077..3ee40646d7 100644 --- a/apps/mobile/src/components/agents/chat-composer.test.ts +++ b/apps/mobile/src/components/agents/chat-composer.test.ts @@ -28,7 +28,7 @@ const TEXT_DIRECTIONS = [ { direction: 'RTL', isRTL: true, - style: [{ writingDirection: 'rtl' }, { letterSpacing: 0 }, undefined], + style: [{ writingDirection: 'rtl' }, undefined, undefined], }, ]; diff --git a/apps/mobile/src/components/agents/new-session-prompt-initial-prompt.test.ts b/apps/mobile/src/components/agents/new-session-prompt-initial-prompt.test.ts index 572d467b6c..03ee14bb22 100644 --- a/apps/mobile/src/components/agents/new-session-prompt-initial-prompt.test.ts +++ b/apps/mobile/src/components/agents/new-session-prompt-initial-prompt.test.ts @@ -17,7 +17,7 @@ const TEXT_DIRECTIONS = [ { direction: 'RTL', isRTL: true, - style: [{ writingDirection: 'rtl' }, { letterSpacing: 0 }, undefined], + style: [{ writingDirection: 'rtl' }, undefined, undefined], }, ]; 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 5c5d55fd0e..1fd04de692 100644 --- a/apps/mobile/src/components/home/section-header.mounted.test.tsx +++ b/apps/mobile/src/components/home/section-header.mounted.test.tsx @@ -76,12 +76,13 @@ 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 LTR design; RTL renders it unspaced, so - // the Arabic labels keep their joins (see lib/rtl-text.ts). + // The tracked class stays for the Latin design; the RTL letter-spacing + // reset applies to Arabic-script copy only, 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).toContainEqual({ letterSpacing: 0 }); - expect(text.props.style).toContainEqual({ letterSpacing: 0 }); + expect(label.props.style).not.toContainEqual({ letterSpacing: 0 }); + expect(text.props.style).not.toContainEqual({ letterSpacing: 0 }); } else { expect(label.props.style).toBeUndefined(); expect(text.props.style).toBeUndefined(); @@ -107,6 +108,31 @@ describe('SectionHeader mounted layout', () => { expect(text.children).toEqual(['See all']); }); + it('renders Arabic labels without the mono family or letter spacing in RTL', () => { + i18nManager.isRTL = true; + const root = mount( + createElement(SectionHeader, { + label: 'الجلسات الجارية الآن', + actionLabel: 'عرض الكل', + onActionPress: () => undefined, + }) + ); + const label = root.find( + node => Object.is(node.type, 'Text') && node.children.includes('الجلسات الجارية الآن') + ); + const action = root.findByProps({ accessibilityRole: 'button' }); + const text = action.find(node => Object.is(node.type, 'Text')); + + for (const node of [label, text]) { + const classes = (node.props.className as string).split(' '); + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(node.props.style).toContainEqual({ writingDirection: 'rtl' }); + expect(node.props.style).toContainEqual({ letterSpacing: 0 }); + } + expect(label.children).toEqual(['الجلسات الجارية الآن']); + expect(text.children).toEqual(['عرض الكل']); + }); + it.each([{ isRTL: false }, { isRTL: true }])( 'aligns the action with the row edges, never with a physical text align, with RTL=$isRTL', ({ isRTL }) => { diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx index 39b117e005..b2759651c5 100644 --- a/apps/mobile/src/components/ui/text.mounted.test.tsx +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -42,12 +42,27 @@ afterEach(() => { 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 => { + // uppercase letter-spacing and broke apart mid-word ('ال جلسا ت'). The + // display treatment is dropped for RTL-script copy (Arabic, Hebrew) in an + // 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'))); expect(classes).toEqual( - expect.arrayContaining(['font-mono-medium', 'text-[10px]', 'text-muted-foreground']) + expect.arrayContaining([ + 'font-mono-medium', + 'text-[10px]', + 'text-muted-foreground', + 'uppercase', + 'tracking-[1.5px]', + ]) + ); + }); + + it.each([false, true])('drops the treatment from Arabic copy in RTL (RTL=%s)', isRTL => { + i18nManager.isRTL = isRTL; + const classes = hostClasses( + mount(createElement(Text, { variant: 'eyebrow' }, 'الجلسات الجارية الآن')) ); if (isRTL) { expect(classes).not.toContain('uppercase'); @@ -69,12 +84,16 @@ describe('Text eyebrow letterspacing', () => { const classes = hostClasses( mount(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) ); - expect(classes).toEqual(expect.arrayContaining(['font-mono-medium', 'text-[10px]'])); + expect(classes).toEqual(expect.arrayContaining(['text-[10px]'])); if (isRTL) { + // Arabic copy in an RTL interface also drops the mono family. + expect(classes.some(name => name.startsWith('font-mono'))).toBe(false); expect(classes).not.toContain('uppercase'); expect(classes.some(name => name.startsWith('tracking'))).toBe(false); } else { - expect(classes).toEqual(expect.arrayContaining(['uppercase', 'tracking-[1.5px]'])); + expect(classes).toEqual( + expect.arrayContaining(['font-mono-medium', 'uppercase', 'tracking-[1.5px]']) + ); } }); }); 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 new file mode 100644 index 0000000000..cc231b4c4e --- /dev/null +++ b/apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx @@ -0,0 +1,112 @@ +import { createElement, type ReactElement } from 'react'; +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'; + +const i18nManager = vi.hoisted(() => ({ isRTL: false })); +vi.mock('react-native', () => ({ + I18nManager: i18nManager, + Text: 'Text', +})); +vi.mock('@rn-primitives/slot', () => ({ Text: 'Slot.Text' })); + +let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; +function mount(element: ReactElement) { + act(() => { + renderer = TestRenderer.create(element); + }); + if (!renderer) { + throw new Error('Missing text renderer'); + } + return renderer.root; +} + +function hostText(root: TestRenderer.ReactTestInstance) { + return root.find(node => Object.is(node.type, 'Text')); +} + +const ARABIC = 'الجلسات الجارية الآن'; +// U+0870–U+089F, Arabic Extended-B: Arabic-script characters outside the +// blocks the first fix matched. +const ARABIC_EXTENDED_B = '\u0870\u089F'; +// The Hebrew eyebrow copy from `he.json` (`home.agentSessions`): Hebrew is an +// RTL locale the app ships and is not Arabic script, so an Arabic-only +// predicate leaves it with the Latin tracking and mono family. +const HEBREW = 'פעילים עכשיו'; +const LATIN = 'Live now'; + +beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + i18nManager.isRTL = false; +}); +afterEach(() => { + act(() => renderer?.unmount()); + renderer = undefined; +}); + +describe('Text eyebrow in an RTL interface', () => { + it('drops the mono family and letter spacing from an Arabic label', () => { + i18nManager.isRTL = true; + const label = hostText(mount(createElement(Text, { variant: 'eyebrow' }, ARABIC))); + const classes = (label.props.className as string).split(' '); + + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(classes).toEqual(expect.arrayContaining(['text-[10px]', 'text-muted-foreground'])); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); + expect(label.props.style).toContainEqual({ writingDirection: 'rtl' }); + expect(label.children).toEqual([ARABIC]); + }); + + it('drops the mono family and letter spacing from an Extended-B-only label', () => { + i18nManager.isRTL = true; + const label = hostText(mount(createElement(Text, { variant: 'eyebrow' }, ARABIC_EXTENDED_B))); + const classes = (label.props.className as string).split(' '); + + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); + expect(label.children).toEqual([ARABIC_EXTENDED_B]); + }); + + it('drops the mono family and letter spacing from a Hebrew label', () => { + i18nManager.isRTL = true; + const label = hostText(mount(createElement(Text, { variant: 'eyebrow' }, HEBREW))); + const classes = (label.props.className as string).split(' '); + + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(classes).not.toContain('tracking-[1.5px]'); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); + expect(label.props.style).toContainEqual({ writingDirection: 'rtl' }); + expect(label.children).toEqual([HEBREW]); + }); + + it('keeps the tracked mono design for a Latin label', () => { + i18nManager.isRTL = true; + const label = hostText(mount(createElement(Text, { variant: 'eyebrow' }, LATIN))); + const classes = (label.props.className as string).split(' '); + + expect(classes).toContain('font-mono-medium'); + expect(classes).toContain('tracking-[1.5px]'); + 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', () => { + 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(); + }); + + it('applies the same rule to the Eyebrow wrapper', () => { + i18nManager.isRTL = true; + const label = hostText(mount(createElement(Eyebrow, null, ARABIC))); + const classes = (label.props.className as string).split(' '); + + expect(classes.some(token => token.startsWith('font-mono'))).toBe(false); + expect(label.props.style).toContainEqual({ letterSpacing: 0 }); + expect(label.children).toEqual([ARABIC]); + }); +}); 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 caadf94dca..4a28e811a0 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 @@ -48,6 +48,10 @@ afterEach(() => { // and the bottom tab labels. 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. +const HEBREW = 'פעילים עכשיו'; + describe('Text tracked labels in RTL', () => { it.each(TRACKED_CLASSES)( 'draws %s with no letter spacing while a tracked class stays on the element', @@ -62,6 +66,24 @@ describe('Text tracked labels in RTL', () => { } ); + it('resets a caller-tracked Hebrew label, not only Arabic', () => { + i18nManager.isRTL = true; + 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); + }); + + it('drops the eyebrow Latin display treatment from Hebrew copy', () => { + i18nManager.isRTL = true; + const root = mount(createElement(Text, { variant: 'eyebrow' }, HEBREW)); + + 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); + }); + it('leaves no non-zero letter spacing on a tracked label in any class order', () => { i18nManager.isRTL = true; const root = mount( @@ -82,8 +104,10 @@ describe('Text tracked labels in RTL', () => { it('keeps the caller style after the RTL defaults', () => { i18nManager.isRTL = true; const callerStyle = { color: '#ff0000' }; + // RTL-script copy, the copy the merged rule resets (text.rtl-labels: + // Latin labels keep their tracking); the caller style still lands last. const root = mount( - createElement(Text, { className: 'tracking-[1.5px]', style: callerStyle }, '…') + createElement(Text, { className: 'tracking-[1.5px]', style: callerStyle }, 'استكشف') ); expect(hostStyle(root)).toContainEqual(callerStyle); @@ -102,6 +126,9 @@ describe('Text tracked labels in RTL', () => { i18nManager.isRTL = true; const root = mount(createElement(Eyebrow, null, 'استكشف')); + // Arabic-script copy drops the tracked class and the mono family in an RTL + // interface (`withoutMonoFamily`): a zero letter spacing alone does not + // keep a cursive script's joins (text.rtl-labels, text.mounted). // The eyebrow's Latin display treatment (uppercase + tracking) is LTR-only // (see `Text`'s eyebrow variant and `SectionHeader`): the variant owns its // display classes, so an RTL eyebrow drops them — it carries no tracked diff --git a/apps/mobile/src/components/ui/text.tsx b/apps/mobile/src/components/ui/text.tsx index 2b273ed3dc..f80589b9ab 100644 --- a/apps/mobile/src/components/ui/text.tsx +++ b/apps/mobile/src/components/ui/text.tsx @@ -3,7 +3,12 @@ import { cva, type VariantProps } from 'class-variance-authority'; import * as React from 'react'; import { I18nManager, Text as RNText, type Role } from 'react-native'; -import { RTL_NO_LETTER_SPACING, RTL_WRITING_DIRECTION } from '@/lib/rtl-text'; +import { + hasRtlScript, + RTL_NO_LETTER_SPACING, + RTL_WRITING_DIRECTION, + withoutMonoFamily, +} from '@/lib/rtl-text'; import { cn } from '@/lib/utils'; const textVariants = cva('text-foreground text-base font-medium', { @@ -49,10 +54,12 @@ const ARIA_LEVEL = { } satisfies Partial>; /** - * The eyebrow's Latin display treatment: full capitals, letterspaced. It is an - * LTR-only addition to the variant because `letter-spacing` pulls a cursive - * script apart — an Arabic eyebrow renders 'الجلسات' as 'ال جلسا ت'. An RTL - * interface keeps the mono family, size and color and drops both classes. + * The eyebrow's Latin display treatment: full capitals, letterspaced. It is + * dropped for RTL-script copy in an RTL interface (`hasRtlScript`: the app + * 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. * * Exported so the eyebrow-scale labels rendered outside the variant — the * `SectionHeader` action link — carry the identical treatment instead of a @@ -74,20 +81,23 @@ function Text({ }) { const textClass = React.useContext(TextClassContext); const Component = asChild ? Slot.Text : RNText; + const isRTL = I18nManager.isRTL; + const isRtlScript = hasRtlScript(props.children); + const classes = cn( + textVariants({ variant }), + variant === 'eyebrow' && !(isRTL && isRtlScript) && EYEBROW_LATIN_DISPLAY, + textClass, + className + ); return ( 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..ef34a0f470 --- /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 { hasRtlScript } from './rtl-text'; + +vi.mock('react-native', () => ({ I18nManager: { isRTL: false } })); + +// U+0870 and U+089F are the first and last characters of Arabic Extended-B, +// the block between Arabic Supplement (ends U+077F) and Arabic Extended-A +// (starts U+08A0). A label using only its characters must still get the +// joining font the Arabic blocks select. +const FIRST_EXTENDED_B = '\u0870'; +const LAST_EXTENDED_B = '\u089F'; + +describe('hasRtlScript', () => { + it('detects a label written only in Arabic Extended-B characters', () => { + expect(hasRtlScript(FIRST_EXTENDED_B + LAST_EXTENDED_B)).toBe(true); + }); + + it('detects the Arabic blocks around Arabic Extended-B', () => { + expect(hasRtlScript('\u06FF')).toBe(true); + expect(hasRtlScript('\u077F')).toBe(true); + expect(hasRtlScript('\u08A0')).toBe(true); + expect(hasRtlScript('\uFB50')).toBe(true); + expect(hasRtlScript('\uFE70')).toBe(true); + }); + + // Hebrew is an RTL locale the app ships (`RTL_LANGUAGES` in + // `@/i18n/languages`) and is not Arabic script, so the Arabic-only predicate + // this replaced left its copy treated like Latin. + it('detects the Hebrew block and its presentation forms', () => { + expect(hasRtlScript('\u0590')).toBe(true); + expect(hasRtlScript('\u05FF')).toBe(true); + expect(hasRtlScript('\uFB1D')).toBe(true); + expect(hasRtlScript('\uFB4F')).toBe(true); + }); + + it('detects a Hebrew label', () => { + expect(hasRtlScript('פעילים עכשיו')).toBe(true); + }); + + it('walks the child tree to reach an Extended-B label', () => { + expect(hasRtlScript(createElement('Text', null, FIRST_EXTENDED_B))).toBe(true); + }); + + it('leaves Latin copy alone', () => { + expect(hasRtlScript('Live now')).toBe(false); + }); +}); diff --git a/apps/mobile/src/lib/rtl-text.ts b/apps/mobile/src/lib/rtl-text.ts index 160e077f54..87aa74a41e 100644 --- a/apps/mobile/src/lib/rtl-text.ts +++ b/apps/mobile/src/lib/rtl-text.ts @@ -1,3 +1,4 @@ +import { isValidElement, type ReactNode } from 'react'; import { I18nManager, type StyleProp, type TextStyle } from 'react-native'; /** @@ -43,17 +44,56 @@ export function withRtlInputAlignment( /** * Letter-spacing — Tailwind's `tracking-*` — is a Latin typographic device: - * it opens every glyph from its neighbour. The RTL scripts the app ships do - * not take it. An Arabic-script word is one connected shape, so a tracked - * label breaks its joins and renders the letters as isolated forms - * ('استكشف'); a letter-spacing of 0 keeps the paragraph's natural spacing. + * 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 everything that goes through it, in - * the same RTL style array as `RTL_WRITING_DIRECTION`, so a tracked class the - * LTR design owns stays in the className and simply has no effect 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`). */ +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. */ +export function hasRtlScript(node: ReactNode): boolean { + if (Array.isArray(node)) { + return node.some((child: ReactNode) => hasRtlScript(child)); + } + if (isValidElement<{ children?: ReactNode }>(node)) { + return hasRtlScript(node.props.children); + } + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- ReactNode has no non-typeof way to detect its plain-string variant + if (typeof node === 'string') { + return RTL_SCRIPT.test(node); + } + return false; +} + +/** + * 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. + */ +export function withoutMonoFamily(className: string): string { + return className + .split(' ') + .filter(token => !/^(?:[a-z-]+:)*font-mono(?:-(?:medium|semibold))?$/.test(token)) + .join(' '); +} + /** The caller's style with the RTL paragraph direction behind it, in RTL only. */ export function withRtlWritingDirection(style: TextStyle | undefined): TextStyle | undefined { if (!I18nManager.isRTL) {