joomla-cms
joomla-cms copied to clipboard
Bootstrap RTL the correct way
This PR is using bootstrap the way that bootstrap intended for it to be used since 5.0 for RTL.
removing any rtl specific code that can be automatically handled by rtlcss or by using rtlcss specific markup (just as bootstrap itself does)
All our current css is doing bootstrap the wrong way for rtl support.
The problem is NOT with our own scss as in general this is now correct with logical properties (The few instances of dir=rtl are now handled by rtlcss)
The problem is with the actual bootstrap code. We are using it without addressing any RTL issues with that. With the exception of a very incomplete set of bootstrap overrides in https://github.com/joomla/joomla-cms/blob/7cf5e1228db337881d7678ad1c71915d5c885150/build/media_source/templates/administrator/atum/scss/vendor/bootstrap/_bootstrap-rtl.scss
Technically this will change all left to right etc so we dont need to use logical properties in our own css however I still think its good practice that we continue to use logical properties where available.
Bootstrap 5 is designed to be post-processed using rtlcss - This takes the generated template.css and converts it to template-rtl.css
Benefits
- it is DRY
- we don't need to remember to check it works in rtl (we almost always forget)
- we dont need to waste time creating rtl overrides (in most cases)
- fixes numerous open issues
- replaces numerous open pull requests
- fixes several issues with input buttons not already on the tracker.
Testing
- Apply the pr and then run
npm iThis will generate all the css and install rtlcss
Or
2.Use a prebuuilt package
- test test test
Sample Tests
Global configuration in arabic
Before

After

Add Multi-Factor Authentication
Before

After

With this PR we need to update the build tools. rtlcsss is a postcss plugin that runs on the generated template.css of atum and cassiopeia and after generating the new -rtl.css this needs to be minified as well.
https://github.com/brianteeman/joomla-cms/pull/291
Thanks @dgrammatiko
Thought it was way more work than this.
Well it was 14hours solid so not a small amount - plus dont forget all the PR since you did the first work where I moved our own css to logical properties
Yeah that makes a lot of sense. Still really nice work though :)
Thanks
is there any point in me fixing the conflicts here or is it just going to be ignored like so many pull requests recently?
Your PR's are not ignored .. there are only not enough active testers.
This should be rebased to 4.3 anyway.
its a bug fix not a feature
19 changed files and a new package is a too big change for a bug fix release. I welcome the change, but we really need to bring 4.2 into a stable state, so stuff like this needs to go into the next minor. I know we both have different opinions what a bug fix is, but we should start to handle patch releases as such ones and not getting the risk to become unstable again, even when the chance is small. I really hope you understand that point, this here is not a risky one. Again, the change is very welcome but please for 4.3.
rebased as requested - of ciurse its now untestable until 4.3 brnch is brought up to date
Thanks, merging stuff like this as early as possible brings us more time to iron out regressions, even when the chance is small that there will be one.
except it cant be tested right now :(
Thanks, merging stuff like this as early as possible brings us more time to iron out regressions, even when the chance is small that there will be one.
I hope this means that pull requests wont wait as RTC for weeks and months before merging any more
Updated the branch, should be back to normal.
Thanks @laoneo
I have tested this item :white_check_mark: successfully on 4609b33164939f64dbe574aeade304388d129735
Tested it with farsi. Did browse around and saw no issue. Installed DPCalendar and created events and so on. All did look ok with RTL.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38412.
thanks for testing. dpc was one of the components I developed this with ;)
I have tested this item :white_check_mark: successfully on 95ba902e072a87d0aeed8c3f1acf21ad1f8e657b
I did extensive browsing trying to catch issues.
As a non-rtl reader, I may have missed issues related to rtl that I may not be unaware of, but I honestly think this PR was a great work through.
It would be useful to get a tester using rtl daily.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38412.
made some tests, but i'm not a RTL user... ...so if we don't get any RTL tester... you can consider this as a valid one
fyi I use RTL a lot which is why I spent the time making the pr
I would like to merge this PR into 4.3 early on. I will merge it tomorrow if there are not any issues by then.
Any objection @chmst @HLeithner ?
is it tomorrow yet?
Was hoping to hear from reviewers...
Thank you Brian @brianteeman for this needed fix and for keeping track.