Skip to content

fix(mobile): clear letter-spacing on joined-script Arabic labels - #6497

Merged
iscekic merged 1 commit into
mainfrom
kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3
Sep 23, 2026
Merged

iscekic merged 1 commit into
mainfrom
kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3

Conversation

@iscekic

@iscekic iscekic commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Changelog for users

  • The Arabic home overline, its action, and the bottom-nav labels render as joined words with no in-word gaps.
  • The English home keeps its tracked uppercase overline, action, and tab labels.
  • Latin runs inside an Arabic screen keep their letter-spacing.

Changelog for maintainers

  • The shared Text component sets an inline letterSpacing: 0 for joined-script children, overriding any tracking-* class.
  • rtl-text.ts adds JOINED_SCRIPT, containsJoinedScript, and textLetterSpacing for the Arabic, Arabic Supplement, Extended-A, and Presentation Forms blocks.
  • Detection reads direct string children and recurses arrays; a nested Text element applies the rule in its own run.
  • Latin, Hebrew, digits, and punctuation keep their tracking and their RTL writing direction.
  • The override is placed before the caller style, relying on NativeWind merging the class style first.
  • Look first at the style array in text.tsx and at any stand-alone tracking-* applied to Arabic text.
  • Tests cover block ranges, array and element children, mounted Text at both isRTL values, and the section header label and action.

E2E proof

e1

e2

e3

e4

e5

Owner request

kwf-fix: proof-05bd533

kwf-fix-pr: #6497

Prove the behaviour of PR #6497 with a live end-to-end run, and make no code change.
The pull request description carries no evidence: the '## E2E proof' section is empty.

Run the pull request's own scenarios on a device or a simulator, for every platform its diff touches. Capture the decisive log lines always, and a screenshot as well for a user-visible change. Never a recording: they are gone (owner, 2026-09-16).
Write the evidence under a '## E2E proof' heading in your own pull request description. Never run gh: the driver publishes.
If a scenario fails, name it and say why, and still change no product code: this section proves what the branch already carries.

The pull request description as it stands now

This is the live body; you never need to fetch it, and you must not edit it. The driver merges what your final summary says into it.

## Changelog for users

- The Arabic home overline, its action, and the bottom-nav labels render as joined words with no in-word gaps.
- The English home keeps its tracked uppercase overline, action, and tab labels.
- Latin runs inside an Arabic screen keep their letter-spacing.

## Changelog for maintainers

- The shared `Text` component sets an inline `letterSpacing: 0` for joined-script children, overriding any `tracking-*` class.
- `rtl-text.ts` adds `JOINED_SCRIPT`, `containsJoinedScript`, and `textLetterSpacing` for the Arabic, Arabic Supplement, Extended-A, and Presentation Forms blocks.
- Detection reads direct string children and recurses arrays; a nested `Text` element applies the rule in its own run.
- Latin, Hebrew, digits, and punctuation keep their tracking and their RTL writing direction.
- The override is placed before the caller `style`, relying on NativeWind merging the class style first.
- Look first at the style array in `text.tsx` and at any stand-alone `tracking-*` applied to Arabic text.
- Tests cover block ranges, array and element children, mounted `Text` at both `isRTL` values, and the section header label and action.

## E2E proof



## Open findings (not fixed here)
- and y121-135/y120-136 (e3-scene.log); the pixel-level letter join is the visual reviewer's on e3.png."},{"name":"e4","result":"pass","evidence":"/Users/igor/.local/share/kwf/ios/runs/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3/evidence/e4-scene.log","note":"ios; …[truncated]
- =1). --- shard 1 ---
VERDICT {"verdict":"passed","scenarios":[{"name":"e1","result":"pass","evidence":"/Users/igor/.local/share/kwf/ios/runs/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3/evidence/e1-scene.log","note":"ios; signed-in English home digest shows 'LIVE NOW', 'See a
- not proved live: e1.png is no longer on the host that took it, so no publish can carry it
- not proved live: e2-home-ar.png is no longer on the host that took it, so no publish can carry it
- not proved live: e6-acct-tap.png is no longer on the host that took it, so no publish can carry it
- not proved live: e6-en.png is no longer on the host that took it, so no publish can carry it
- not proved live: e6-prefs.png is no longer on the host that took it, so no publish can carry it
- not proved live: e6.png is no longer on the host that took it, so no publish can carry it
- not proved live: home-arabic.png is no longer on the host that took it, so no publish can carry it
- the '## E2E proof' section is empty

![e1-probe](https://github.com/user-attachments/assets/3f72aff3-72f4-45e2-8164-5bb35dda237b)

<details>
<summary>Owner request</summary>

> Explorer finding: home-arabic: The Arabic overline and bottom-nav labels inherit the tracked all-caps letter-spacing, which splits connected script mid-word (e.g. الوكلاء renders as 'الوكلا ء' and الجلسات الجارية الآن breaks into separate letters).
> 
> The user-agent explorer found this while using the app like a user.
> One finding per item; the explorer never edits product code.
> 
> Flow: home-arabic
> Found on revision: f2181ae79
> 
> Repro:
> 1. set this state first: reviews 4; credits 25; seed app:github-account e2e-mobile-cloud-android@example.com
> 2. open the app on 1F6F1503-9C26-4120-82D4-5F8768CBE42F
> 3. reach home-arabic
> 4. the capture shows the defect named below
> 
> Observed: The Arabic overline and bottom-nav labels inherit the tracked all-caps letter-spacing, which splits connected script mid-word (e.g. الوكلاء renders as 'الوكلا ء' and الجلسات الجارية الآن breaks into separate letters).
> Expected: the screen renders without this defect
> 
> Evidence (from the device run):

</details>
## Open findings (not fixed here) - and y121-135/y120-136 (e3-scene.log); the pixel-level letter join is the visual reviewer's on e3.png."},{"name":"e4","result":"pass","evidence":"/Users/igor/.local/share/kwf/ios/runs/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3/evidence/e4-scene.log","note":"ios; …[truncated] - =1). --- shard 1 --- VERDICT {"verdict":"passed","scenarios":[{"name":"e1","result":"pass","evidence":"/Users/igor/.local/share/kwf/ios/runs/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3/evidence/e1-scene.log","note":"ios; signed-in English home digest shows 'LIVE NOW', 'See a - not proved live: e1.png is no longer on the host that took it, so no publish can carry it - not proved live: e2-home-ar.png is no longer on the host that took it, so no publish can carry it - not proved live: e6-acct-tap.png is no longer on the host that took it, so no publish can carry it - not proved live: e6-en.png is no longer on the host that took it, so no publish can carry it - not proved live: e6-prefs.png is no longer on the host that took it, so no publish can carry it - not proved live: e6.png is no longer on the host that took it, so no publish can carry it - not proved live: home-arabic.png is no longer on the host that took it, so no publish can carry it - the '## E2E proof' section is empty

e1-probe

@iscekic
iscekic marked this pull request as draft September 21, 2026 17:09
@kilo-code-bot

kilo-code-bot Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The rewritten joined-script letter-spacing fix is correct at bf5959b08: LTR Arabic children get an inline letterSpacing: 0, RTL keeps the prior unconditional reset, and the detection/fallback logic introduces no regression on the changed lines.

Files Reviewed (5 files)
  • apps/mobile/src/components/home/section-header.mounted.test.tsx
  • apps/mobile/src/components/ui/text.mounted.test.tsx
  • apps/mobile/src/components/ui/text.tsx
  • apps/mobile/src/lib/rtl-text.test.ts
  • apps/mobile/src/lib/rtl-text.ts

The previously reviewed SHA (6aa171d93) is no longer an ancestor of the branch head, so the review was re-run against the full current PR diff. JOINED_SCRIPT is stateless (no g flag) and spans Arabic, Arabic Supplement, Extended-A, and both Presentation Forms blocks; containsJoinedScript recurses arrays and treats numbers, elements, and empty children as non-strings. In text.tsx, ownStyles falls back to RTL_NO_LETTER_SPACING when no joined script is present, so an RTL interface resets tracking for every run exactly as the base did, while LTR adds letterSpacing: 0 only for joined-script children and leaves a Latin LTR style undefined. The pre-existing text.rtl-tracking.mounted.test.tsx assertions still hold (deep equality, so the separate but structurally identical NATURAL_LETTER_SPACING satisfies them). No memory leaks (no effects, listeners, or subscriptions added). The mounted tests mock react-native, so they assert the emitted style/className props rather than NativeWind's className-vs-inline precedence on device, and the PR body states the visual E2E proof is not live; the on-device visual result remains unverified here.


Reviewed at bf5959b08.

Previous Review Summaries (4 snapshots, latest commit 6aa171d)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 6aa171d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The rewritten joined-script letter-spacing fix (6aa171d93) is correct: it restores the unconditional RTL letterSpacing: 0 reset that the prior revision dropped and adds the LTR-only override for Arabic children, with no regression on the changed lines.

Files Reviewed (8 files)
  • apps/mobile/src/components/code-reviewer/manual-review-screen.mounted.test.tsx
  • apps/mobile/src/components/home/section-header.mounted.test.tsx
  • apps/mobile/src/components/ui/segmented-control.tsx
  • apps/mobile/src/components/ui/text.mounted.test.tsx
  • apps/mobile/src/components/ui/text.rtl-tracking.mounted.test.tsx
  • apps/mobile/src/components/ui/text.tsx
  • apps/mobile/src/lib/rtl-text.test.ts
  • apps/mobile/src/lib/rtl-text.ts

The branch was rewritten (05bd5339c is no longer an ancestor of 6aa171d93), so the review was re-run against the current PR diff. JOINED_SCRIPT is stateless (no g flag) and covers Arabic, Arabic Supplement, Extended-A, and the two Presentation Forms blocks; containsJoinedScript recurses arrays and treats numbers, elements, and empty children as non-strings (a nested Text is its own run). In text.tsx, ownStyles now falls back to RTL_NO_LETTER_SPACING when no joined script is present, so an RTL interface resets tracking for every run exactly as the base did, while the LTR branch adds letterSpacing: 0 only for joined-script children and leaves Latin LTR style undefined. SectionHeader and TabBarLabel both pass the Arabic label as a direct string child to this Text, so the overline, action, and bottom-nav paths are reached. The segmented-control.tsx change only removes a duplicate numberOfLines={1} prop. No memory leaks (no effects, listeners, or subscriptions added). The mounted tests mock react-native, so they assert the emitted style/className props rather than NativeWind's className-vs-inline precedence on device; the PR body states the visual E2E proof is not live, so the on-device visual result remains unverified here.


Reviewed at 6aa171d93.

Previous review (commit 05bd533)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-verification of the joined-script letter-spacing fix at the current head found no new issues on the changed lines.

Files Reviewed (5 files)
  • apps/mobile/src/components/home/section-header.mounted.test.tsx
  • apps/mobile/src/components/ui/text.mounted.test.tsx
  • apps/mobile/src/components/ui/text.tsx
  • apps/mobile/src/lib/rtl-text.test.ts
  • apps/mobile/src/lib/rtl-text.ts

Re-verified at 05bd5339c: JOINED_SCRIPT is stateless (no g flag) and covers the intended Arabic, Supplement, Extended-A, and Presentation Forms blocks; containsJoinedScript recurses arrays and treats nested Text, numbers, elements, and empty children as non-string (inner runs apply the rule themselves so Latin runs keep their tracking); and the ownStyles array (letterSpacing: 0, then RTL_WRITING_DIRECTION, then the caller style) preserves the prior LTR/Latin behavior while overriding a tracking-* class. TabBarLabel and SectionHeader both render through this Text, so the bottom-nav label path named in the PR scope is reached. The mounted tests mock react-native, so they assert the emitted style/className props rather than NativeWind's className-vs-inline precedence on device, and the PR body states the visual E2E proof is not live; the on-device visual result remains unverified here.

Previous review (commit ce38481)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • apps/mobile/src/components/home/section-header.mounted.test.tsx
  • apps/mobile/src/components/ui/text.mounted.test.tsx
  • apps/mobile/src/components/ui/text.tsx
  • apps/mobile/src/lib/rtl-text.test.ts
  • apps/mobile/src/lib/rtl-text.ts

Re-verified the current head (ce384817d) after the branch rewrite: JOINED_SCRIPT covers the intended blocks and is stateless (no g flag), containsJoinedScript handles string/array/undefined/number/element children, and the ownStyles array (letterSpacing: 0 then RTL_WRITING_DIRECTION, then the caller style) preserves the previous RTL/Latin behavior. The eyebrow variant and SectionHeader action retain their LTR-only tracking, so the joined-script override is the only new path. The mounted tests mock react-native and therefore assert the emitted style/className props rather than NativeWind's className-vs-inline precedence on device; the PR body also states the visual E2E proof is not live, so the on-device visual result remains unverified here.

Previous review (commit 0847a09)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • apps/mobile/src/components/home/section-header.mounted.test.tsx
  • apps/mobile/src/components/ui/text.mounted.test.tsx
  • apps/mobile/src/components/ui/text.tsx
  • apps/mobile/src/lib/rtl-text.test.ts
  • apps/mobile/src/lib/rtl-text.ts

Reviewed the joined-script letter-spacing override: the regex block ranges, the ReactNode string/array recursion, the ownStyles merge order relative to props.style, and the RTL-writing-direction behavior are all consistent with the tests and preserve the prior LTR/non-Arabic behavior. Note that the mounted tests mock react-native, so they assert the emitted style prop but do not exercise NativeWind's className-vs-inline-style precedence on device; the PR body also states the visual E2E proof is not live.


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3 branch from fad502e to ce38481 Compare September 21, 2026 18:59
@iscekic
iscekic marked this pull request as ready for review September 21, 2026 19:17
@iscekic
iscekic marked this pull request as draft September 21, 2026 22:31
@iscekic
iscekic force-pushed the kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3 branch from ce38481 to 05bd533 Compare September 21, 2026 23:25
@iscekic
iscekic marked this pull request as ready for review September 21, 2026 23:44
@iscekic
iscekic marked this pull request as draft September 22, 2026 01:19
@iscekic
iscekic force-pushed the kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3 branch from 05bd533 to 6aa171d Compare September 22, 2026 03:39
@iscekic
iscekic marked this pull request as ready for review September 22, 2026 04:00
@iscekic
iscekic marked this pull request as draft September 22, 2026 04:48
@iscekic
iscekic force-pushed the kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3 branch from 6aa171d to bf5959b Compare September 23, 2026 04:59
@iscekic
iscekic marked this pull request as ready for review September 23, 2026 05:22
@iscekic

iscekic commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

This description names a scenario the proof did not capture:

  • not proved live: e1.png is no longer on the host that took it, so no publish can carry it

A repeated proof run rebuilds the same evidence, so no proof run is dispatched for a named gap. Merging with this gap open is your decision.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 23, 2026
@iscekic iscekic self-assigned this Sep 23, 2026
@iscekic
iscekic merged commit b37b5e4 into main Sep 23, 2026
29 checks passed
@iscekic
iscekic deleted the kwf/explorer-home-arabic-the-arabic-overline-and-bottom-nav-l-f3af6-f2a3 branch September 23, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants