Repository navigation
feat: read Arabic markdown blocks right-to-left - #6575
haithamassoli44 wants to merge 8 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces new RTL text direction handling for markdown - a new user-facing capability affecting both web and mobile rendering. Multiple unresolved comments identify UI consistency issues where RTL content in certain contexts (blockquotes, code blocks, details) would render incorrectly. You can customize Macroscope's approvability policy. Learn more. |
Chat markdown renders under one inherited direction, so an Arabic paragraph keeps its trailing punctuation on the wrong side and a line that opens in Arabic is laid out from the wrong edge. Tag each text block with dir="auto" from a rehype plugin that runs after the sanitizer, letting the browser's own bidi algorithm pick direction per block from its first strong character. Only the outermost block of a nest is tagged: the auto algorithm skips text inside descendants that carry their own dir, so tagging both leaves the outer one blind and silently LTR. Lists stay untagged for the same reason, so their items keep a direction each. Code fences, <pre>, and table column order are left alone. The physical paddings and table text-align inside .chat-markdown become logical properties, and lists are padded on both inline sides so a right-to-left item has room for the marker on the side it lands on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3bd16d0 to
d265330
Compare
The mobile clients had the same problem the web client did: a list of Arabic items kept its bullets on the left, opposite the text they label, and an Arabic quote kept its rule on the left. React Native has no dir="auto" — Yoga will not mirror a row or resolve start/end padding without an explicit direction — so the first-strong character rule the browser applies for free is applied by hand here, per block, matching the web client. The text itself needs no help: iOS resolves natural alignment from the paragraph's own base writing direction and Android's default text direction is first-strong. Applied in the native iOS renderer for blockquotes and list items, and in the list renderer the other platforms fall back to. The physical paddings and borders those two touch become logical, so they follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haithamassoli44
changed the title
github-actions
Bot
added
size:L
…rection # Conflicts: # apps/web/src/index.css
There was a problem hiding this comment.
Reviewed the web-side bidi changes (ChatMarkdown.tsx, markdown-text-direction.ts, index.css). Three concrete issues: the new plugin never runs for user-authored messages, the list gutter is now applied to both inline sides for every LTR list, and the summary tag in the plugin's set is dropped by the real details renderer.
Posted via Macroscope — UI Consistency
| "th", | ||
| "dd", | ||
| "dt", | ||
| "summary", |
There was a problem hiding this comment.
summary is tagged here, but in ChatMarkdown the details renderer forwards children to MarkdownDetails, which discards the <summary> element and re-renders only summaryNode.props.children inside a CollapsibleTrigger (text-left). The dir="auto" is dropped, so a <details> with Arabic content reads RTL in its body paragraphs but LTR in its title. markdown-text-direction.test.tsx renders bare ReactMarkdown without the app's component map, so it can't catch this. Either apply the resolved direction on the trigger in MarkdownDetails, or drop "summary" from this set so the contract matches what actually reaches the DOM.
Posted via Macroscope — UI Consistency
- Run the direction plugin for user messages too: it only sat in the raw-HTML
rehype array, and every user bubble renders with parseRawHtml={false}, so
Arabic typed by the user — the case the change is for — never got dir="auto".
- Give the details trigger its own dir: MarkdownDetails drops the <summary>
element and re-renders only its children, taking the plugin's dir with it.
- Let a nested list item read its own direction. A list is a container, not a
text block, so its items restart the outermost-only rule.
- Pad only the end side of lists that hold a right-to-left item, rather than
both sides of every list, which inset English lists from the text around
them. The plugin marks those lists: :dir(rtl) reads well but the CSS build
lowers it to a :lang() list that never matches a dir="auto" element.
- Pin code and tables to ltr on both clients. They are left untagged on
purpose, but an Arabic block around one still handed it a direction to
inherit, mirroring a code header or reversing columns.
- Mobile: return "ltr" rather than undefined for a left-to-right block, since
Yoga inherits direction and an English item under an Arabic one kept the
mirrored chrome; add the four remaining right-to-left scripts; and wrap task
list items, the one row in the fallback renderer that never mirrored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the web-side bidi changes. The three findings from the previous run (direction plugin skipped for parseRawHtml={false} messages, unconditional end-side list padding, dropped <summary> dir) are all addressed. Two remaining gaps in the "code and layout containers read in source order" rule, both in apps/web/src/index.css.
Posted via Macroscope — UI Consistency
…rection - Pin `ul`/`ol` to ltr. The gutter rules read the list box as left-to-right and nothing kept it that way: a list nested in an Arabic item or quote inherited the direction and swapped the gutter to the side its markers aren't on. - Isolate inline code. A Latin run in a right-to-left paragraph has its bounding neutrals reordered, so `--flag` read `flag--` and `foo()` read `()foo`. Only paragraphs that resolve RTL were affected, which is new here. - Move the table's ltr onto `.chat-markdown-table-container`. The scroller is the ScrollArea viewport inside it, so pinning the table alone still left an overflowing table opening on its last column with its fades flipped. - Restrict the nested-list rule to lists under an item. A quote's whole content can be a list, and tagging those items left `dir="auto"` on the quote with nothing to read, pinning the rail left while the items read right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
One finding on the latest commit: the new direction: ltr intended for the table container also lands on p and blockquote, which are the blocks this PR tags with dir="auto".
Posted via Macroscope — UI Consistency
The table container's `direction: ltr` went into the shared margin rule, whose selector list also holds `p` and `blockquote` — the two blocks the plugin tags with dir="auto". An author `direction` outranks the UA rule that `auto` resolves through, so every Arabic paragraph read left-to-right again and the quote rail stayed left, undoing what the branch is for. It gets its own rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| /* The gutter rules below read the list box as left-to-right, and nothing else would keep it | ||
| that way: a list nested in an Arabic item or quote inherits its direction, which would swap | ||
| the gutter to the side the markers aren't on. Items carry their own dir and are unaffected. */ | ||
| .chat-markdown ul, | ||
| .chat-markdown ol { | ||
| direction: ltr; | ||
| } |
There was a problem hiding this comment.
🟡 Medium src/index.css:1650
RTL lists nested in a dir="auto" blockquote render with their marker and gutter on the left, so markdown such as > - عنصر has incorrect list direction. The unconditional direction: ltr overrides the inherited RTL direction on the nested ul/ol; remove this rule so the list inherits its containing block's direction.
-/* The gutter rules below read the list box as left-to-right, and nothing else would keep it
- that way: a list nested in an Arabic item or quote inherits its direction, which would swap
- the gutter to the side the markers aren't on. Items carry their own dir and are unaffected. */
-.chat-markdown ul,
-.chat-markdown ol {
- direction: ltr;
-}
-🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/index.css around lines 1650-1656:
RTL lists nested in a `dir="auto"` blockquote render with their marker and gutter on the left, so markdown such as `> - عنصر` has incorrect list direction. The unconditional `direction: ltr` overrides the inherited RTL direction on the nested `ul`/`ol`; remove this rule so the list inherits its containing block's direction.
There was a problem hiding this comment.
Two direction-inheritance gaps in the web markdown chrome, both introduced by this PR's new RTL blocks.
Posted via Macroscope — UI Consistency
|
|
||
| .chat-markdown pre { | ||
| /* Code reads in source order, whatever direction the block around it resolved to. */ | ||
| direction: ltr; |
There was a problem hiding this comment.
pre is pinned to LTR, but the chrome wrapped around it isn't. A fenced block renders through MarkdownCodeBlock, which puts the pre inside .chat-markdown-codeblock with a justify-between header (pt-1.5 pr-1.5 pb-0 pl-3, ChatMarkdown.tsx L685-689). Nested in an Arabic li/blockquote/cell — all of which now resolve RTL — that wrapper inherits direction: rtl, so the header row mirrors (title right, toolbar left) while its physical pr/pl stay put, leaving the toolbar against the 3-unit edge and the title against the 1.5. The table container got the whole-container treatment for exactly this reason (L1612), and the mobile side wraps the entire code block in a direction: "ltr" view.
Suggest pinning the wrapper rather than only the pre:
+.chat-markdown .chat-markdown-codeblock {
+ direction: ltr;
+}
+
.chat-markdown pre {Posted via Macroscope — UI Consistency
| className="flex w-full items-center gap-2 py-2 text-left text-sm font-medium text-foreground data-panel-open:[&_svg]:rotate-90" | ||
| // The summary element itself is dropped here, so its `dir` has to be reapplied on the | ||
| // row that replaces it — otherwise an Arabic title reads left-to-right under its own body. | ||
| dir="auto" |
There was a problem hiding this comment.
dir="auto" on the trigger mirrors only the summary row: the trigger is flex … gap-2, so an Arabic title moves the chevron to the right, while the panel body below it still inherits LTR from the Collapsible root and keeps its ps-6 inset on the left (L577). The title and its body then hang off opposite edges.
Suggest putting dir="auto" on the Collapsible root instead of the trigger — the root's auto resolution skips the body paragraphs (they carry their own dir) and reads the summary text, so the title resolves the same way and ps-6 follows it.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c221eb7. Configure here.
| .chat-markdown ul, | ||
| .chat-markdown ol { | ||
| direction: ltr; | ||
| } |
There was a problem hiding this comment.
Quote lists lose RTL base
Medium Severity
rehypeAutoTextDirection leaves li elements inside blockquote untagged so the quote can resolve dir="auto", but .chat-markdown ul, ol { direction: ltr } then forces those items to an LTR base. Arabic list-only quotes get a correct rail with incorrect item text direction and punctuation, which the nearby CSS comment assumes cannot happen because items always carry their own dir.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c221eb7. Configure here.
| lineHeight: markdownFontSizes.bodyLineHeight, | ||
| textAlign: ordered ? "right" : "center", | ||
| // A number hugs the text it labels, which the mirrored row moved to its left. | ||
| textAlign: ordered ? (itemDirection === "rtl" ? "left" : "right") : "center", |
There was a problem hiding this comment.
RTL lists mirror code fences
Medium Severity
On the Android/nitro path, Arabic list rows set Yoga direction on a wrapper that also contains nested fences via MarkdownCodeBlock, which has no direction: "ltr" guard. iOS wraps code_block (and tables) in an LTR view for this reason; without the same shield, code headers and horizontal scroll chrome mirror inside RTL items.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit c221eb7. Configure here.
|
closing in favor of #7574, the newer generalized RTL implementation for Hebrew and Arabic Markdown. |
1 participant
Problem
Markdown renders under a single inherited direction, so Arabic text comes out wrong: the sentence is reordered, trailing punctuation lands on the wrong side, and list markers sit opposite the text they label. Mixed Arabic/English conversations — one Arabic paragraph, one English paragraph — have no single direction that is correct for both.
Before / After (web)
Same message, same font, same theme.
Web
A small rehype plugin (
apps/web/src/markdown-text-direction.ts) tags each text block withdir="auto", which hands the block to the browser's own bidi algorithm: direction comes from the block's first strong character, per block, so an Arabic paragraph and the English one under it each get their own.rehypeSanitize, sodiris ours rather than the message's and the sanitize schema is untouched.dir, so tagging both leaves the outer one with nothing to read and it falls back to LTR — a quote whose paragraph is Arabic would keep its rule on the left. Lists are left untagged for the same reason, so their items keep a direction each.p,li, headings,blockquote,td/th,dd/dt,summary,figcaption. Code fences,<pre>, and table column order are deliberately left alone: reversing those changes meaning, not presentation.text-align: leftinside.chat-markdownbecome logical properties, and lists are padded on both inline sides so a right-to-left item has room for its marker on the side it lands on. No visual change for LTR content.Everything routes through
ChatMarkdown, so chat messages, plans, PR bodies, and the file preview are covered by the one plugin. The desktop app loads the web client, so it comes along.Mobile
Same rule, applied by hand, because React Native has no
dir="auto"— Yoga will not mirror a row or resolve start/end padding without an explicitdirection.markdownTextDirectionreads the first letter of a block and returns"rtl"or nothing.paddingStart,borderStartWidth,marginEnd) so they follow the direction.No font changes
Nothing here touches typography — no bundled font, no font-family or size rules.
Testing
vp test run src/markdown-text-direction.test.tsx(web) — 3 tests: blocks getdir="auto", code and table containers do not, and only the outermost block of a nest is tagged.vp test run modules/t3-markdown-text/src/markdownTextDirection.test.ts(mobile) — 2 tests: first-letter detection across scripts, and descending to the first child that reads as text.vp test run src/markdown-links.test.ts src/markdown-github-alerts.test.tsx src/markdown-list-indentation.test.tsx src/components/chat/MessagesTimeline.test.tsx— 63 pass in total.tsgo --noEmitandvp lintclean on the changed scope in both apps. (apps/mobilehas 55 pre-existing navigation-typing errors, unchanged by this branch.)Mobile is not screenshotted yet. The change is JS-only, but confirming it needs a simulator build; happy to attach before/after from a simulator if you want it before review.
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
Note
Low Risk
Presentation-only markdown/i18n layout with targeted tests; no auth, data, or API changes. Residual risk is edge cases in first-strong detection vs browser bidi for unusual scripts or nested structures.
Overview
Mixed Arabic/English markdown now picks per-block direction from the first strong letter (aligned with
dir="auto"), instead of inheriting one direction for the whole message.Web: New
rehypeAutoTextDirectionruns after sanitize (and alone for non-HTML user messages) to setdir="auto"on outermost text blocks anddata-rtl-itemon lists that contain RTL items..chat-markdownswitches to logical padding/borders, pins lists, tables,pre, and inlinecodeto LTR where mirroring would change meaning, and fixes collapsible summary rows withdir="auto".Mobile:
markdownTextDirection/markdownNodeDirectionmirror the same first-letter rule; iOSNativeMarkdownBlockandThreadFeedlist renderers setdirectionper list item and blockquote, use start/end margins, and wrap code/tables indirection: "ltr". Exported via@t3tools/mobile-markdown-text/direction.Tests added for web rehype output and mobile direction helpers.
Reviewed by Cursor Bugbot for commit c221eb7. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Render Arabic and other RTL markdown blocks right-to-left on web and mobile
rehypeAutoTextDirectionrehype plugin (markdown-text-direction.ts) that setsdir="auto"on outermost text blocks and marks lists containing RTL items withdata-rtl-itemfor CSS-driven gutter adjustments.border-inline-start,padding-inline-start, etc.) so blockquotes, lists, and inline code render correctly in both LTR and RTL; tables and code blocks are pinned to LTR.markdownTextDirectionandmarkdownNodeDirectionutilities (markdownTextDirection.ts) for the mobile markdown renderer, applying per-item direction to list rows and blockquotes inNativeMarkdownBlockandThreadFeed.Macroscope summarized c221eb7.