Skip to content

Add opt-in RTL compatibility for markdown rendering#63

Merged
omers-oai merged 17 commits intomainfrom
codex/rtl-compatibility
Mar 27, 2026
Merged

Add opt-in RTL compatibility for markdown rendering#63
omers-oai merged 17 commits intomainfrom
codex/rtl-compatibility

Conversation

@omers-oai
Copy link
Copy Markdown

@omers-oai omers-oai commented Mar 23, 2026

Codex:

Summary

  • add opt-in RichTextRenderOptions.enableRtlCompatibility for markdown paragraph and heading alignment on top of [codex] Extract RTL compatibility samples #66
  • keep behavior outside compatibility mode unchanged
  • align compatibility-mode paragraphs and headings by their first strong letter while leaving neutral-only text on the ambient side
  • update the RTL sample screen to opt into the compatibility mode

Testing

  • ./gradlew :richtext-ui:allTests :richtext-markdown:allTests :android-sample:testDebugUnitTest

@omers-oai omers-oai requested review from rjmarsan and rz-openai March 23, 2026 20:06
@omers-oai omers-oai marked this pull request as ready for review March 23, 2026 20:08
@omers-oai omers-oai marked this pull request as draft March 24, 2026 21:06
@omers-oai omers-oai force-pushed the codex/rtl-compatibility branch from 4d65c94 to c35efd3 Compare March 25, 2026 17:35
@omers-oai omers-oai changed the base branch from main to codex/rtl-compatibility-samples March 25, 2026 17:35
@omers-oai omers-oai force-pushed the codex/rtl-compatibility branch from c35efd3 to aa018ee Compare March 25, 2026 18:11
@omers-oai omers-oai force-pushed the codex/rtl-compatibility branch from 93636e9 to 7aaf63a Compare March 25, 2026 18:20
@omers-oai omers-oai changed the base branch from codex/rtl-compatibility-samples to main March 25, 2026 18:20
@omers-oai omers-oai marked this pull request as ready for review March 26, 2026 17:31
Comment on lines +19 to +20
* Lets list and quote containers override paragraph alignment without forcing a different
* direction-detection rule for the text inside the item.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this is better than wrapping the Text with a different LocalLayoutDirection value?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think LayoutDirection solves some of it but not all.
We have cases of an item in a list, which may be LTR or RTL, and the list itself (decide by first item) may have the list marked on the left or right. So we have both LTR and RTL text that we may need to align either left or right depending on the case.

val markdownAnimationState = remember { MarkdownAnimationState() }

if (richTextRenderOptions.enableRtlCompatibility && astNode.type is AstDocument) {
BasicRichText(modifier = Modifier.width(IntrinsicSize.Max)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would just using a Column here work? Seems like we have other simpler options available than putting another BasicRichText...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I had a long back and forth with Codex about this. It claimed it won't, I made it do it, and it ended up winning since it broke direction on some things and I didn't fully understand why, but it said we need to reset the lists and stuff so all items will be considered part of the same doc or something.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do want to remove the duplicaiton here, should I extract a local Composable function? I'm not a fan too much of it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get it... why would we only need to reset lists if we want to change the RTL compatibility bit?

@omers-oai omers-oai merged commit 0296d87 into main Mar 27, 2026
1 check passed

is AstUnorderedList,
is AstOrderedList -> {
val items = remember(astNode) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like before we didn't remember this by astNode, I hope this doesn't break anything, I'm assuming it shouldn't....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants