Repository navigation
fix(session-ui): keep mixed-direction text in its dominant language - #57803
itsAbdulelah wants to merge 4 commits into
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes #54471: I re-ran the issue's reproduction in headless Chromium with the PR's real sanitizeMarkdown and CSS. Persian/Arabic text in li, blockquote p, th, td, and a paragraph that starts with React, now lays out RTL, while English blocks and code stay LTR. The PR's component specs (40/40) and typecheck pass.
Public API: unchanged (session-ui internals only).
Two regressions are worth fixing before merge (details inline):
- Scripts other than Latin/Greek/Cyrillic count as neutral. A Chinese, Japanese, Korean or Hindi paragraph that contains a single Arabic or Hebrew word now gets
dir="rtl". On the base it rendered LTR, and I confirmed the flip in Chromium. - The user bubble loses per-line direction. With
unicode-bidi: plaintextandpre-wrap, each line of a prompt got its own direction. A singledirfor the whole text now lays out an English line RTL when the rest of the prompt is Arabic, or the reverse.
Tests: the two word-position layout tests in markdown.spec.ts and session-timeline.spec.ts repeat a long text-walking helper. The dir assertions on sanitizeMarkdown output, plus a single computed direction check, would cover the same behaviour with much less code.
| const rightToLeftScript = /[\u0590-\u08ff\ufb1d-\ufdff\ufe70-\ufeff]/g | ||
|
|
||
| // Left-to-right scripts that carry letters: Latin, Greek, Cyrillic. | ||
| const leftToRightScript = /[a-z\u00c0-\u00d6\u00d8-\u00f6\u00f8-\u02af\u0370-\u03ff\u0400-\u04ff\u1e00-\u1eff]/gi |
There was a problem hiding this comment.
Only Latin, Greek and Cyrillic letters count as LTR here, so CJK, Devanagari, Thai, Korean and other LTR scripts are treated as neutral. As a result, 这是一个中文段落，提到了 אבא 这个名字 returns rtl, and so do Japanese, Korean and Hindi text with a single RTL word. In Chromium that Chinese paragraph flips from LTR on the base to RTL with this PR. Counting every letter that isn't RTL as LTR (e.g. /\p{L}/gu minus the RTL ranges) would avoid this.
| <div data-slot="user-message-text" dir="auto" data-comments={comments().length > 0 ? "true" : undefined}> | ||
| <div | ||
| data-slot="user-message-text" | ||
| dir={textDirection(props.text ?? "")} |
There was a problem hiding this comment.
Before this change the bubble had white-space: pre-wrap plus unicode-bidi: plaintext, so each line of the prompt got its own direction. One dir for the whole text now applies the majority direction to every line. In Chromium, Fix the login bug please\nثم أضف اختباراً… lays out the English first line RTL, where the base laid it out LTR. Could the bubble keep unicode-bidi: plaintext (per-line direction) and use the computed dir only as a fallback, or set a direction per line?
|
Both regressions are fixed in 3143842.
Re-measured in Storybook with I also replaced both word-walking helpers with |
`dir="auto"` and `unicode-bidi: plaintext` both resolve direction from the first strong character, so an Arabic paragraph that opens with a Latin identifier such as "React" rendered left to right and its word order looked reversed. Set an explicit `dir` on every text block from the script most of its characters belong to, skipping code and math. List containers follow the majority of their items so their markers stay inside the box, and the user message bubble uses the same helper. `unicode-bidi: plaintext` is removed because it overrides `dir`.
Counting only Latin, Greek and Cyrillic as left to right left CJK, Devanagari, Thai and Korean text neutral, so a single Arabic or Hebrew word flipped a Chinese or Hindi paragraph to right to left. Count every letter and let its script decide instead. The user bubble also lost its per-line direction when `unicode-bidi: plaintext` was removed: one `dir` for the whole prompt laid an English line out as right to left when the rest was Arabic. Render each line in its own isolated span with a `dir` computed from that line, and keep the message direction on the bubble for alignment.
3143842 to
20889dd
Compare
There was a problem hiding this comment.
Both earlier points are addressed. textDirection now counts every non-RTL letter as LTR, so CJK and Devanagari paragraphs with a single Arabic or Hebrew word stay LTR (confirmed in Chromium). The user bubble renders each line in its own isolated span with its own dir. The layout tests are also much smaller now.
I re-ran the #54471 reproduction in headless Chromium with the PR's sanitizeMarkdown and CSS, and the nested Persian/Arabic blocks still lay out RTL. The new unit test, the two component spec files (40/40) and typecheck pass.
Public API: unchanged.
No further concerns.
There was a problem hiding this comment.
The latest push only merges dev. The merge kept the direction logic intact, and the user-message link segments from dev still render inside the per-line spans. I re-ran the #54471 reproduction in headless Chromium with the PR's sanitizeMarkdown and CSS: nested Persian/Arabic blocks lay out RTL and a CJK paragraph stays LTR. text-direction.test.ts, the two component spec files (46/46) and tsc -b pass.
Public API: unchanged.
One new concern, inline: URLs and @file mentions count toward a prompt line's direction, so a short Arabic sentence followed by a link or path flips to LTR.
packages/session-ui/src/message/message-content.tsx:770: URLs and@filementions add their Latin letters to the count here. Soراجع هذا الرابط https://github.com/anomalyco/opencode/pull/57803andهل يعمل @packages/session-ui/src/message/message-content.tsxboth resolve toltr, and a short Arabic line that ends in a link or path now renders LTR. On the base it rendered RTL. Skippinglink,fileandagentsegments when computing the direction would avoid this, both per line and for the whole-bubbledironuser-message-text. If they are the only content, the line can fall back to the ambient direction.
Issue for this PR
Closes #54471
Type of change
What does this PR do?
dir="auto"on the Markdown container resolves the direction from the first strong character, and every nested block inherits that one answer. An Arabic paragraph that opens with a Latin identifier (React هو...) therefore renders as LTR and its reading order flips. The user message bubble had the same problem throughunicode-bidi: plaintext, which overridesdir.This computes the direction from the script most of the text's letters belong to instead of its first letter:
textDirection(text)counts right-to-left letters against every other letter, so Chinese, Japanese, Korean, Devanagari and Thai stay left to right. Digits, punctuation, and whitespace are neutral.markdown-cache.tsxsets an explicitdiron each text block (p, headings,li,dt/dd, list containers,blockquote,table,caption,figcaption,summary,td,th). Code and math are excluded because they always render left to right.message-content.tsxrenders every prompt line in its own isolated span with adircomputed from that line, so a multi-line prompt can mix directions. The bubble keeps the whole-message direction for alignment.unicode-bidi: plaintextis removed because it overridesdirand made the attribute ineffective.dirsets the base direction of the paragraph, so the bidi algorithm keeps the Arabic run in reading order while the Latin words still render left to right inside it. The existing CSS already uses logical properties (padding-inline-start,border-inline-start,text-align: start), so alignment, list markers, table columns, and blockquote borders follow automatically.How did you verify your code works?
text-direction.test.tscovers mixed Arabic/Latin, Chinese, Japanese, Korean, Devanagari, neutral digits, and ties.markdown.spec.ts: mixed Arabic/English paragraphs, lists, tables, and blockquotes keep RTL in completed and streaming renders; English and code stay LTR; CJK and Hindi paragraphs that contain one Arabic word stay LTR.session-timeline.spec.ts: an Arabic prompt that opens with "React" keepsdir="rtl", and a two-line prompt keeps an LTR first line and an RTL second line.bun typecheckfrompackages/session-uipasses,oxlintreports no warnings on the touched files, and 40/40 component tests pass.bun run checkexits 29 on this Windows machine even on a clean tree (turbo), so the pre-push hook was skipped with--no-verify; CI runs the same check on Linux.Screenshots / recordings
Storybook story
current-session-timeline-rows--arabic-promptreproduces the case. WithFix the login bug pleasefollowed by a newline andمرحبا كيف حالك؟ سأراجع الكود الآن., the first line resolves todir="ltr"with its words at x=669/693/718/754/783 and the second todir="rtl"with its words at x=797/767/728/685/651/622.Checklist
Added by the opencode bot (AI-generated).
Related: #26915, #16875