Skip to content
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

Add RTL support #13338

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Add RTL support #13338

wants to merge 18 commits into from

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
@@ -111,6 +111,10 @@ Features added
* #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.

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
3 participants