-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add RTL support #13338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add RTL support #13338
Conversation
- Add is_rtl theme option for right-to-left text direction - Implement automatic layout mirroring for RTL languages - Keep code blocks in LTR direction
CHANGES.rst
Outdated
| * #13326: Remove hardcoding from handling :class:`~sphinx.addnodes.productionlist` | ||
| nodes in all writers, to improve flexibility. | ||
| Patch by Adam Turner. | ||
| * #10385: Add RTL (right-to-left) support for all Sphinx themes via ``is_rtl`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * #10385: Add RTL (right-to-left) support for all Sphinx themes via ``is_rtl`` | |
| * #10385: Add RTL (right-to-left) support for all Sphinx themes via the ``is_rtl`` |
CHANGES.rst
Outdated
| * #10385: Add RTL (right-to-left) support for all Sphinx themes via ``is_rtl`` | ||
| theme option. Includes automatic layout mirroring, and bidirectional text | ||
| support. | ||
| Patch by Revisto. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Patch by Revisto. | |
| Patch by Alireza Shabani. |
Usually we use names (see above entries) rather than github @s but its up to you.
| - Keep code blocks in LTR direction | ||
|
|
||
| Defaults to ``False``. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A version added block would be a nice addition
| div.admonition { | ||
| border-right: 0.2em solid black !important; | ||
| border-left: none !important; | ||
| padding: 2px 7px 1px 7px !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| padding: 2px 7px 1px 7px !important; |
This is redundant? It is already defined, you are overriding the exact same thing
| padding: 2px 7px 1px 7px; |
I presume there are more cases of this
- Add "the" before "is_rtl" - Fix line wrapping in CHANGES.rst entry
- Change "Revisto" to "Alireza Shabani"
|
Hi @StanFromIreland, thank you so much for your review. I fixed the grammar issue, and changed my name to |
AA-Turner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reduce duplication in the rtl.css files?
|
Hi, I tried to remove the duplications, hope it helps. |
|
Hi all, just following up on this PR 👋 I’ve addressed the feedback (grammar fixes, Happy to make any further changes if needed, thanks! |
thibaudcolas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have little to no experience with Sphinx’s built-in themes and browser support, but it puzzles me to see RTL support added like this in 2025. With logical properties and values, it should be possible to add RTL support to the themes without introducing so many CSS overrides, that are very prone to falling out of sync with the LTR variants. You’ll get the same results with:
- Next to no extra CSS added, just change existing definitions
- No need for
!important - No concern about keeping things in sync long-term
The only drawback is not everyone knows about those properties and values, and when changing CSS in the future people will need to use those properties / avoid ever using physical properties with left/right (it’s fine to retain top/bottom usage). Linting works very well to guarantee that.
There are also a few CSS properties that do not have logical equivalents, namely transforms and background-position. Those could be duplicated, or set with a calc with a variable that switches between 1 and -1 depending on the direction.
| body, | ||
| div.header div.rel { | ||
| direction: rtl !important; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m surprised this is done via CSS rather than the dir attribute in HTML. If CSS fails to load, surely the HTML document on its own should be RTL-friendly?
| body { | ||
| text-align: right !important; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to set text-align like this for RTL support. If the HTML document’s dir is set to rtl, then all text that isn’t forced to a particular alignment will switch to right by default.
| } | ||
|
|
||
| p.logo { | ||
| float: left !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really shouldn’t be needed. Just change the agogo.css.jinja’s definition to use flow-relative values:
p.logo {
float: inline-start;
}
Purpose
Add RTL (right-to-left) support for all Sphinx themes to improve accessibility for RTL languages like Farsi (Persian), Arabic, and Hebrew.
Key features:
is_rtltheme option for enabling RTL layoutImplementation Details
is_rtl(defaults tofalse)Questions
References
I'm eager to improve this implementation and would love to continue working on it. Open to suggestions and ready to make any necessary changes to align with Sphinx's standards.