[codex] Fix RTL chat message rendering#3674
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
b29c4f8 to
2c2d462
Compare
| } | ||
|
|
||
| /* Bidi: each line resolves its own base direction (UAX#9 plaintext). */ | ||
| .bidi-plaintext { |
There was a problem hiding this comment.
🟡 Medium src/index.css:547
unicode-bidi: plaintext on .bidi-plaintext and the markdown block selectors makes the browser infer direction from the text's first strong character, ignoring the explicit dir attribute set on those elements. As a result, RTL messages whose content begins with LTR characters (e.g. code-prefixed Hebrew) are rendered LTR regardless of the dir value. Consider using unicode-bidi: isolate instead, which still isolates the element's direction from surrounding text but honors the explicit dir attribute.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/index.css around line 547:
`unicode-bidi: plaintext` on `.bidi-plaintext` and the markdown block selectors makes the browser infer direction from the text's first strong character, ignoring the explicit `dir` attribute set on those elements. As a result, RTL messages whose content begins with LTR characters (e.g. code-prefixed Hebrew) are rendered LTR regardless of the `dir` value. Consider using `unicode-bidi: isolate` instead, which still isolates the element's direction from surrounding text but honors the explicit `dir` attribute.
| </h6> | ||
| ); | ||
| }, | ||
| ul({ node, children, ...props }) { |
There was a problem hiding this comment.
🟡 Medium components/ChatMarkdown.tsx:1427
The ul, ol, and li component overrides compute dir by calling blockDirForMarkdownNode(text, node, children), but a list node's position spans the entire subtree including nested lists. A top-level English list that contains a long nested Hebrew/Arabic sublist will resolve the outer <ul>/<ol> and parent <li> as dir="rtl", placing the top-level bullets/numbers on the wrong side even though the parent items are LTR. Consider computing the direction from only the node's own direct text rather than the full subtree range.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 1427:
The `ul`, `ol`, and `li` component overrides compute `dir` by calling `blockDirForMarkdownNode(text, node, children)`, but a list node's `position` spans the entire subtree including nested lists. A top-level English list that contains a long nested Hebrew/Arabic sublist will resolve the outer `<ul>`/`<ol>` and parent `<li>` as `dir="rtl"`, placing the top-level bullets/numbers on the wrong side even though the parent items are LTR. Consider computing the direction from only the node's own direct text rather than the full subtree range.
| } | ||
|
|
||
| /* Source code stays LTR even inside RTL messages. */ | ||
| .chat-markdown pre, |
There was a problem hiding this comment.
🟡 Medium src/index.css:565
Setting direction: ltr on .chat-markdown .chat-markdown-table-container forces every markdown table to render left-to-right, so an RTL table (e.g. Hebrew/Arabic content) has its column order and cell direction reversed instead of following the surrounding message direction. Unlike pre/code, tables are general markdown content, so forcing LTR is wrong here. Consider removing .chat-markdown-table-container from the direction: ltr rule and instead letting it inherit the bidi context, keeping only unicode-bidi: isolate if needed.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/index.css around line 565:
Setting `direction: ltr` on `.chat-markdown .chat-markdown-table-container` forces every markdown table to render left-to-right, so an RTL table (e.g. Hebrew/Arabic content) has its column order and cell direction reversed instead of following the surrounding message direction. Unlike `pre`/`code`, tables are general markdown content, so forcing LTR is wrong here. Consider removing `.chat-markdown-table-container` from the `direction: ltr` rule and instead letting it inherit the bidi context, keeping only `unicode-bidi: isolate` if needed.
| return ( | ||
| <div className="whitespace-pre-wrap wrap-break-word text-sm leading-relaxed text-foreground"> | ||
| <div | ||
| dir={dirFor(props.text)} |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.tsx:1566
UserMessageBody computes the wrapper dir from dirFor(props.text) alone, so when the message body is empty or neutral/LTR but the terminalContexts chip labels are RTL, the wrapper stays ltr and the inline chips are ordered and aligned incorrectly. Consider deriving dir from the combined text of props.text and the terminal context headers (e.g. dirFor(props.text + inlinePrefix)) so RTL chip content is respected.
Also found in 1 other location(s)
apps/web/src/components/ComposerPromptEditor.tsx:1398
ComposerBidiDirectionPluginderives the editor direction from$getRoot().getTextContent(), but inline token nodes do not expose their rendered chip labels there. For example,ComposerTerminalContextNode.getTextContent()returns onlyINLINE_TERMINAL_CONTEXT_PLACEHOLDER, while the visible chip label comes fromformatTerminalContextLabel(context). A composer containing only RTL terminal-context chips (or RTL display labels on other chips) will therefore keepdir="ltr", so the new bidi fix fails for those prompts.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.tsx around line 1566:
`UserMessageBody` computes the wrapper `dir` from `dirFor(props.text)` alone, so when the message body is empty or neutral/LTR but the `terminalContexts` chip labels are RTL, the wrapper stays `ltr` and the inline chips are ordered and aligned incorrectly. Consider deriving `dir` from the combined text of `props.text` and the terminal context headers (e.g. `dirFor(props.text + inlinePrefix)`) so RTL chip content is respected.
Also found in 1 other location(s):
- apps/web/src/components/ComposerPromptEditor.tsx:1398 -- `ComposerBidiDirectionPlugin` derives the editor direction from `$getRoot().getTextContent()`, but inline token nodes do not expose their rendered chip labels there. For example, `ComposerTerminalContextNode.getTextContent()` returns only `INLINE_TERMINAL_CONTEXT_PLACEHOLDER`, while the visible chip label comes from `formatTerminalContextLabel(context)`. A composer containing only RTL terminal-context chips (or RTL display labels on other chips) will therefore keep `dir="ltr"`, so the new bidi fix fails for those prompts.
Summary
Resolves #1771
Validation
pnpm --dir apps/web exec vp test run src/lib/rtl.test.ts src/components/ChatMarkdown.test.tsx --passWithNoTests --project unitpnpm exec vp run typecheckpnpm exec vp check(passes with existing warnings)Note
Fix RTL rendering for chat messages and composer editor
dirForanddirForMarkdownutilities that detect text direction by analyzing strong bidirectional characters, with markdown-aware stripping of code spans, fences, and URLs.ChatMarkdownto setdir="rtl"ordir="ltr"on block-level elements (p,li,h1–h6,ul,ol,blockquote) based on the markdown source slice for each node.ComposerBidiDirectionPlugintoComposerPromptEditorthat updates the rootcontentEditabledirattribute as the user types.MessagesTimelineto setdiron inline user message wrappers and appliesunicode-bidi: plaintextvia a new CSS class.index.cssto use logical CSS properties for list/blockquote spacing and isolates code blocks and tables to LTR regardless of surrounding direction.📊 Macroscope summarized 2c2d462. 5 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.