Skip to content

Conversation

@Revisto
Copy link

@Revisto Revisto commented Feb 13, 2025

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:

  • Add is_rtl theme option for enabling RTL layout
  • Implement automatic layout mirroring
  • Keep code blocks in the LTR direction
  • Maintain theme-specific styling while supporting RTL

Implementation Details

  1. Added RTL stylesheets to each theme
  2. Updated theme configuration options
  3. Added new theme option is_rtl (defaults to false)
  4. Added documentation for RTL support

Questions

  1. Font Integration
    • Is it a good idea to integrate Vazirmatn font for RTL support?
    • If yes, what's the best approach:
      • Bundle font files with Sphinx?
      • Use CDN?

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.

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``

Choose a reason for hiding this comment

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

Suggested change
* #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.

Choose a reason for hiding this comment

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

Suggested change
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``.

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;

Choose a reason for hiding this comment

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

Suggested change
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

@Revisto
Copy link
Author

Revisto commented Feb 15, 2025

Hi @StanFromIreland, thank you so much for your review.

I fixed the grammar issue, and changed my name to CHANGES.rst.
I added the versionadded block in theming documentation.
I removed the redundant RTL overrides.

Copy link
Member

@AA-Turner AA-Turner left a 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?

@Revisto
Copy link
Author

Revisto commented Feb 16, 2025

Hi, I tried to remove the duplications, hope it helps.
I'm looking forward to hearing your thoughts.

@Revisto
Copy link
Author

Revisto commented May 8, 2025

Hi all, just following up on this PR 👋

I’ve addressed the feedback (grammar fixes, versionadded, and CSS cleanups) and would love to get this moving.
RTL support would really help the ongoing Python Farsi translation and other RTL languages.

Happy to make any further changes if needed, thanks!

Copy link
Contributor

@thibaudcolas thibaudcolas left a 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;
}
Copy link
Contributor

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?

Comment on lines +9 to +11
body {
text-align: right !important;
}
Copy link
Contributor

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;
Copy link
Contributor

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;
}

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.

Support for RTL Languages

4 participants