Skip to content

fix(mobile): stop Latin tracking from splitting Arabic labels - #6654

Closed
iscekic wants to merge 7 commits into
mainfrom
kwf/explorer-home-ar-rtl-arabic-labels-in-the-tracked-eyebrow-60e9e-13a6
Closed

iscekic wants to merge 7 commits into
mainfrom
kwf/explorer-home-ar-rtl-arabic-labels-in-the-tracked-eyebrow-60e9e-13a6

Conversation

@iscekic

@iscekic iscekic commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • Arabic, Persian, Urdu and other joined-script labels on Home no longer draw with extra space between their letters.
  • The tracked eyebrow header and its action, the section label above the Explore rows, and the bottom tab labels render with their natural connected letterforms.
  • Latin labels keep the design's letter-spacing.

Changelog for maintainers

  • The shared Text component adds an inline letterSpacing: 0 when its own string children contain a joined script, and keeps the RTL paragraph direction.
  • The reset is inline, so it overrides the tracking-* className while the className stays on the element.
  • The caller's style is merged last, so an explicit caller letter spacing still wins.
  • New helpers containsJoinedScript, textLetterSpacing and NATURAL_LETTER_SPACING live in apps/mobile/src/lib/rtl-text.ts; the range covers Arabic, Arabic Supplement, Arabic Extended-A and both Arabic Presentation Forms.
  • Only a node's own string children count; a nested Text run applies its own reset.
  • The inline style is an array for joined-script and RTL runs, so two existing component tests changed from [direction, undefined] to [direction].
  • Review the inline-versus-className merge and the style ordering first; the mounted tests pin both the reset and the kept tracking class.

E2E proof

Owner request

Explorer finding: home-ar-rtl: Arabic labels in the tracked eyebrow header (top row), the section label above the Explore rows, and the four bottom tab labels render with Latin letter-spacing, so the connected script breaks into spaced, disconnected 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-ar-rtl
Found on revision: f2181ae

Repro:

  1. set this state first: org; credits 25; reviews 3
  2. open the app on 1F6F1503-9C26-4120-82D4-5F8768CBE42F
  3. reach home-ar-rtl
  4. the capture shows the defect named below

Observed: Arabic labels in the tracked eyebrow header (top row), the section label above the Explore rows, and the four bottom tab labels render with Latin letter-spacing, so the connected script breaks into spaced, disconnected letters.
Expected: the screen renders without this defect

Evidence (from the device run):

Open findings (not fixed here)

  • 493; letterform appearance is the visual reviewer's on e3.png."},{"name":"e4","result":"pass","evidence":"evidence/e4-scene.log","note":"ios: e4-scene.log shows the English Home digest 'LIVE NOW' [14,121][199,135], 'See all' [206,120][388,136], 'EXPLORE' [14,351][388,365] and tab labels …[truncated]
  • --- shard 1 ---
    VERDICT {"verdict":"passed","scenarios":[{"name":"e1","result":"pass","evidence":"evidence/e1-scene.log","note":"ios: e1-scene.log shows the Arabic Home digest with the three label groups '\u0627\u0644\u062c\u0644\u0633\u0627\u062a \u0627\u0644\u062c\u0627\u0631\u064a\u0629 \u0627\u
  • not proved live: e2-account-sheet.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e2-sheet-scroll.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e2.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e3.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e4.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e5-en-switch.png is no longer on the host that took it, so no publish can carry it
  • not proved live: e5-en.png is no longer on the host that took it, so no publish can carry it
  • not proved live: home-ar-rtl.png is no longer on the host that took it, so no publish can carry it
  • not proved live: now-state.png is no longer on the host that took it, so no publish can carry it
  • the '## E2E proof' section is empty

…kwf explorer-home-ar-rtl-arabic-labels-in-the-tracked-eyebrow-60e9e-13a6/s1)
@iscekic
iscekic marked this pull request as draft September 23, 2026 18:03
@kilo-code-bot

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

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

No changed lines to review: the branch was reset so the PR head tree (9bff89e28ee850fca38919fb2b289d1d96f0f102) is identical to the base tree, and gh pr diff 6654 reports 0 changed files and 0 changed lines. The joined-script letter-spacing behavior described in the PR body is already present in main and is not carried as a diff here, so no inline findings apply.

Files Reviewed (0 files)
  • No changed files in the current PR diff.
Previous Review Summary (commit 60c22b4)

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

Previous review (commit 60c22b4)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The joined-script letter-spacing reset in the shared Text component is self-contained and correct: the inline style-array ordering preserves caller overrides and the RTL direction, the non-global regex avoids stateful .test() bugs, and the updated mounted tests match the new style shape.

Files Reviewed (7 files)
  • apps/mobile/src/components/agents/chat-composer.test.ts
  • apps/mobile/src/components/agents/new-session-prompt-initial-prompt.test.ts
  • apps/mobile/src/components/home/section-header.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

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

Review guidance: REVIEW.md from base branch main

The branch had dropped main's 0254-0257 migrations, so its journal stopped at
idx 253 while main was at 259. That is not a migration this PR authored: the
branch changes no schema and its packages/db/src/schema.ts matches main.

Merge origin/main and take main's migration folder verbatim. The drizzle CLI
then reports "No schema changes, nothing to migrate", so no migration is
generated and none of main's migrations remain dropped.

Guards: the packages/db jest suite passes (7 suites, 34 tests), including
migration-journal.test.ts.
This reverts commit 0bcb7a2.

Revert "fix(mobile): reset tracked joined-script text in LTR"

This reverts commit 2e865fd.
@iscekic

iscekic commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

Status: superseded — this PR contains no changes

The PR head tree is identical to the PR base tree, so GitHub reports 0 changed files.

Evidence:

  • git rev-parse 90623ef7bf848d93f159b4e45f0cacbaee5d5abf^{tree} → 9bff89e28ee850fca38919fb2b289d1d96f0f102
  • git rev-parse 89aadf333d18447aa9583ec441f171aac6e42bd1^{tree} → 9bff89e28ee850fca38919fb2b289d1d96f0f102
  • git merge-base origin/main HEAD → 90623ef7bf; git diff --stat origin/main...HEAD → empty
  • GitHub API GET /repos/Kilo-Org/cloud/pulls/6654/files → []

The behaviour this PR describes is already in main, from #6440, #6497 and #6495:

  • apps/mobile/src/components/ui/text.tsx sets RTL_WRITING_DIRECTION and RTL_NO_LETTER_SPACING as inline styles for RTL-script copy in an RTL interface, and pushes the caller's style last.
  • apps/mobile/src/lib/rtl-text.ts holds containsJoinedScript, textLetterSpacing, NATURAL_LETTER_SPACING and the joined-script range (Arabic, Arabic Supplement, Arabic Extended-A, both Presentation Forms).
  • main pins the LTR-interface case the other way: apps/mobile/src/components/ui/text.rtl-labels.mounted.test.tsx:94 ("keeps the mono family and adds no letter spacing for Arabic in an LTR interface") expects no inline style. A reset in an LTR interface breaks that test, so the reset stays RTL-gated.
  • All three label groups from the finding render through Text with tracking classes — the eyebrow (variant: 'eyebrow' + EYEBROW_LATIN_DISPLAY), the section-header action, and the tab labels ((tabs)/_layout.tsx uses tracking-[0.2px]) — so the RTL reset reaches them.

CI at head 95f73b2844a641171effa10a1863fd66fc0f83a4 is green: kilo-app CI (mobile-changes, format-check, typecheck, lint, test, i18n-leftover, check-unused), CI (changes, format-check, typecheck, lint, drizzle-check), extension CI, Kilo MCP catalog, mobile-native-build, Secret Scanning.

Not run, by instruction: local tests, builds and device E2E. No new screenshot proof is added.

Note on this branch: I pushed 2e865fd664 and 0bcb7a2dd9 to try a reset for joined-script copy in an LTR interface, then reverted them in 95f73b2844 because main pins that case as no-reset. The net diff is zero.

Recommendation: close this PR as superseded, or state which behaviour is still missing from main.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 25, 2026
@iscekic
iscekic marked this pull request as ready for review September 25, 2026 13:49
@iscekic iscekic closed this Sep 25, 2026
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.

1 participant