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

RTL Languages Support #6138

Merged
merged 52 commits into from Aug 11, 2023
Merged

RTL Languages Support #6138

merged 52 commits into from Aug 11, 2023

Conversation

AhmedBaset
Copy link
Contributor

@AhmedBaset AhmedBaset commented Jul 7, 2023

This PR addresses right-to-left (RTL) languages styles, directionality, and other relevant adjustments for the React documentation.

Types of changes

  • Updated tailwindcss to the latest version to utilize logical properties.
  • Added the .isRTL property to the siteConfig object note 1.
  • Set the dir attribute on the <html> element to either ltr or rtl based on the isRTL property.
  • Maintained the dir attributes of <InlineCode />, <CodeBlock />, and <Sandpack /> as ltr regardless of the isRTL property.
  • Converted physical and directional properties to logical properties +96% of browsers support logical properties:
    • Replaced pl-*, pr-*, ml-*, and mr-* with ps-*, pe-*, ms-*, and me-*.
    • Replaced text-left and text-right with text-start and text-end.
    • Replaced left-* and right-* with start-* and end-*.
    • Replaced rounded-l-* and rounded-r-* with rounded-s-* and rounded-e-*.
    • Replaced border-l-* and border-r-* with border-s-* and border-e-*.
    • And so on.
  • For the components <IconNavArrow />, <IconArrow />, <IconArrowSmall />, and <IconChevron />, added two additional values to the displayDirection prop: start and end. These values behave differently based on the page direction and should be used instead of left and right values note 2.
  • This PR does not affect anything in LTR languages.

Live demo:

See this live demo of the documentation with some Arabic data (from ar.react.dev)

Notes:

Note 1: The `.isRTL` property

We had several options to choose from:

  • Added a new property to the siteConfig object and set it manually for each language.

    exports.siteConfig = {
      // ...
      languageCode: 'ar',
      isRTL: true,
      // ...
    };
  • Used language detection like rtl-detect to automatically detect the language direction and set the property.

    const siteConfig = {
      // ...
      languageCode: 'ar',
      hasLegacySite: true,
      isRTL: false, // Do NOT ever set this to true. It is deprecated.
      // ...
    };
    
    const rtlDetect = require('rtl-detect');
    siteConfig.isRTL = rtlDetect.isRtlLang(siteConfig.languageCode);
    
    exports.siteConfig = siteConfig;
Note 2: New values for the displayDirection prop
export const IconNavArrow = memo<
  JSX.IntrinsicElements['svg'] & {
    /**
     * The direction the arrow should point.
     * `start` and `end` are relative to the current locale.
     * For example, in LTR, `start` is left and `end` is right.
     */
    displayDirection: 'start' | 'end' | 'right' | 'left' | 'down';
  }
>(function IconNavArrow({displayDirection = 'end', className}) {
  const classes = cn(
    'duration-100 ease-in transition',
    {
      'rotate-0': displayDirection === 'down',
      'rotate-90': displayDirection === 'left',
      '-rotate-90': displayDirection === 'right',
      'rotate-90 rtl:-rotate-90': displayDirection === 'start',
      '-rotate-90 rtl:rotate-90': displayDirection === 'end',
    },
    className
  );

  return (
    <svg
      // ...
      className={classes}
      //...
  );
});

@gaearon, @rickhanlonii, @acdlite, and React Community, could you please review this PR?

1- Update TailwindCSS to use the logical properities such as `ps-1` instead of `pl-1`.
there are logical properities for margin, padding, inset, and text direction.

2- Install `rtl-detect` detect if the language is RTL direction.
Note: this might be uninstalled if we use the manual way to specify rtl language.
instead of directions properties
(e.g. `pr-0` to `pe-0`)
instead of directions properties
(e.g. `pr-0` to `pe-0`)
instead of directions properties
(e.g. `pr-0` to `pe-0`)
instead of directions properties
(e.g. `pr-0` to `pe-0`)
@AhmedBaset
Copy link
Contributor Author

@lunaleaps and @mattcarrollcode, could you please review this? As Arabic users, we have difficulties reading the Arabic docs with LTR directions.

Copy link
Contributor

@lunaleaps lunaleaps left a comment

Choose a reason for hiding this comment

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

Changes look good to me! Just a couple small q's about alt properties and a CSS clarification.

Thanks for doing this work!

Question - is there a lint we can set up to ensure we don't start re-introducing left/right css properties?

@lunaleaps
Copy link
Contributor

Thanks for addressing, looks like just conflicting files and then I can merge!

@AhmedBaset
Copy link
Contributor Author

Thanks for addressing, looks like just conflicting files and then I can merge!

Resolved

@rickhanlonii rickhanlonii merged commit 819518c into reactjs:main Aug 11, 2023
4 checks passed
@rickhanlonii
Copy link
Member

Wow this is awesome, thanks a ton for all that went into this!

@Nabeel20
Copy link

This is amazing, thanks 🙏🏻

rickhanlonii pushed a commit that referenced this pull request Aug 15, 2023
* fix: wronge styles when applying RTL 

it was `top-0 left-0 ...` by wrong I made it `inset-x-0`.

* fix wrong styles

* fix wrong styles

* style canary icon with RTL-friendly styles

* chore: utilize mx-* instead of me-* & ms-*

* utilize relative styles

* chore: use mx-* instead of me-* & ms-*

* style canary icon with RTL-frindly styles

* Update OpenInTypeScriptPlayground.tsx
@AhmedBaset
Copy link
Contributor Author

@lunaleaps: Question: Is there a lint we can set up to ensure we don't start re-introducing left/right CSS properties?

Regarding icons, we can eliminate the "left" and "right" in displayDirection to enforce future updates to employ "start" and "end" directions.

For Tailwindcss classes, I have released eslint-plugin-rtl-friendly to serve this purpose. It reports in cases where you utilize physical properties like pl-*, prompting you to use ps-* instead, with auto-fix by eslint --fix

Note: Up til now, It doesn't detect like className={cn(...)}. It works only for string up to this moment v.0.1.0 and doesn't lint eslint file (StyleLint is needed for that).

We have the option to install and extend it. By configuring no-physical-properties as error (while the default "recommended config" is "warn") to enhance strictness.

What do you think? @lunaleaps @rickhanlonii

@lunaleaps
Copy link
Contributor

Thanks @A7med3bdulBaset for looking into that! I'm curious if we do still need some left/right properties and how we exclude them from the lint? Otherwise, this looks like a great idea!

@AhmedBaset
Copy link
Contributor Author

Thanks @A7med3bdulBaset for looking into that! I'm curious if we do still need some left/right properties and how we exclude them from the lint? Otherwise, this looks like a great idea!

Could you provide an example of left/right properties we need where start/end wouldn't resolve the issue?

A: There are NONE.

The only places we seem to need them are in the <CodeBlock />, <Sandbox/>, and similar components. However, even in those cases, we use dir="ltr", so ps-* becomes pl-*, regardless of the page direction.

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

Successfully merging this pull request may close these issues.

None yet

5 participants