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 PostCSS-RTLCSS approach to the documentation #39863

Merged

Conversation

elchininet
Copy link
Contributor

@elchininet elchininet commented Apr 3, 2024

Description

This pull request modifies the LTR and RTL at the same time section of the documentation to explain how to achieve that goal using PostCSS-RTLCSS.

Motivation & Context

Searching topics that included PostCSS-RTLCSS on the Internet, I found this StackOverflow question asking how to achieve LTR and RTL in Bootstrap at the same time, and the most voted answer mentioned PostCSS-RTLCSS as a better alternative to the instructions given in the documentation.

As I am the manintainer of the PostCSS-RTLCSS plugin, I opened this discussion explaining the benefits of achieving LTR and RTL at the same time using postcss-rtlcss instead. Then as an outcome of the discussion, it was agreed to mention the plugin in the respective section in the documentation.

Type of changes

  • Documentation enhancement

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

https://github.com/orgs/twbs/discussions/39726

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @elchininet!
The explanation looks good to me and has the right level of information; not too much, but enough to understand how PostCSS RTLCSS could be embedded in one's project.
I'll let a native English speaker double-check the wording and also pinging @ffoodd for feedback (does it look OK to you too?).

@ffoodd
Copy link
Member

ffoodd commented Apr 4, 2024

Seems great! Thanks for the contribution.

The third point makes me wonder: could we move rtl:remove and rtl:ignore directives to their block version, to ease the move?

This shouldn't have any impact on our side.

@elchininet
Copy link
Contributor Author

Hi @ffoodd,

The third point makes me wonder: could we move rtl:remove and rtl:ignore directives to their block version, to ease the move?

What do you mean moving them to their block version?

I have plans of making this transparent for clients. As the intention of the RTLCSS directive is the same (do not create an RTL counterpart in the new stylesheet), what I want to achieve is that if someone uses a remove directive, this is translated to an ignore directive behind the scenes. If this is implemented in some moment, it is possible to remove the third point in the Important things to take into account callout warning.

But just let me know if you think that I should change that point or remove something from it at the moment.

Regards

@ffoodd
Copy link
Member

ffoodd commented Apr 4, 2024

My point is that moving directives as recommendend in your third point can be easily achieved un Bootstrap.

If we do it, would it ease your plugin usage?

@elchininet
Copy link
Contributor Author

My point is that moving directives as recommendend in your third point can be easily achieved un Bootstrap.
If we do it, would it ease your plugin usage?

Ah, got it. I would not change anything in the Bootstrap source code because rtl:ignore directives also exist on RTLCSS and they have a different functionality than the rtl:remove ones. At the moment Bootstrap uses the rtl:remove directives to avoid including specific rules into the *.rtl.css bundles. If they are replaced with rtl:ignore ones, those rules will be included in the *.rtl.css bundles but without flipping and I think that is not the intention.

@ffoodd
Copy link
Member

ffoodd commented Apr 4, 2024

Ah, got it too. Thanks, that's clearer 👌

@XhmikosR XhmikosR merged commit f0252f5 into twbs:main Apr 5, 2024
14 checks passed
@elchininet elchininet deleted the add_postcss_rtlcss_to_the_documentation branch April 5, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants