Skip to content
This repository has been archived by the owner on Apr 19, 2019. It is now read-only.

[RFC] RTL testing #166

Closed
andrepereiradasilva opened this issue Nov 15, 2016 · 28 comments
Closed

[RFC] RTL testing #166

andrepereiradasilva opened this issue Nov 15, 2016 · 28 comments

Comments

@andrepereiradasilva
Copy link
Contributor

is this template being tested in RTL languages?

@brianteeman
Copy link
Contributor

Clearly not yet

For reference you do not need to install an RTL language to test.
Just go the en-GB.xml file in the admin language folder and change this line
<rtl>1</rtl>

image

@dgrammatiko
Copy link
Contributor

@andrepereiradasilva @brianteeman reading @C-Lodder 's comment's in the UX group, that will come later on when BS is beta or RC

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 15, 2016

ok ok. just warning about this
anyway this can be easily be done with html[dir=rtl] whatever css overrides.
Please don't create a css only for rtl like isis ...

@C-Lodder
Copy link
Member

Yup, dont worry, it will be added but I can't add it until a later date. Bare in mind that BS4 won't officially suport RTL, so I'm going to use the code from a closed PR or write it myself.

@brianteeman
Copy link
Contributor

brianteeman commented Nov 15, 2016 via email

@C-Lodder
Copy link
Member

Ok turns out they will welcome PR's for it, but once it's stable:

twbs/bootstrap#19555 (comment)

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

Please don't create a css only for rtl like isis ...

If you do it right or somewhat similar to what I did for the joomla.org RTL, you're not going to want to include all of your RTL CSS in the default file, especially when the vast majority of your userbase aren't using the interface in a RTL language. Insert some of @DGT41 arguments against jQuery here. Bootstrap 2 and 3 are crap when it comes to RTL support and I don't see v4 being any better right now. One example being the mixins, because we took the approach of the RTL CSS overriding the base CSS instead of having a fully compiled separate RTL CSS, this is the resulting mixins.less just to correctly override the grid.

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 15, 2016

If you do it right or somewhat similar to what I did for the joomla.org RTL, you're not going to want to include all of your RTL CSS in the default file, especially when the vast majority of your userbase aren't using the interface in a RTL language.

@mbabker what i mean is isis duplicate all css code in template_rtl.css which IMHO we could do better.

you have several strategies for this, each with advantages and disavantages:

  • Option 1: All in one css file = Primary CSS + RTL CSS overrides in one file. No change in the template files to render an extra CSS file. 1 bigger HTTP request (this bigger, really depends on how many css overrides are - normally the margins, paddings, floating, etc right or left).
  • Option 2: Primary CSS file for all (except RTL CSS) + CSS file for RTL CSS override. This means the template needs to check rtl and load that file. 2 smaller HTTP requests.
  • Other options?

I don't know how joomla.org does it, since i cannot change it to a RTL language (where is the select language button? - or do i need to change the browser language to see it ...?).

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

https://downloads.joomla.org and https://www.joomla.org/3 both support a RTL language (sadly most of the other sites are barely English only). Or view https://www.joomla.org/templates/joomla/css/template-rtl.css for the uncompressed CSS file.

With HTTP2 the preference seems to be more for the multiple file approach.

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 15, 2016

so joomla.org uses the isis strategy...

yes, HTTP2 uses multiplexing.

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

isis duplicate all css code in template_rtl.css which IMHO we could do better

We aren't duplicating anything in the RTL CSS, everything there is overriding the base styles.

And it all depends how you look at it. IMO each ways are fine, it boils down to preference.

You can have an if/else to load a single template.css for LTR or RTL properly generated from SCSS; for the most part your template specific stuff is still in a single location and this probably offers the "cleanest" CSS solution.

Or you can have a conditional extra CSS for RTL specific overrides.

Or you can include all the RTL CSS into the main template.css and just have one file with all the declarations served to all the users.

@andrepereiradasilva I added you to https://github.com/joomla/jdotorgtemplate so you can see all the LESS files. I in essence forked the BS2 stuff, removed anything I didn't need to override, and have a lot of "reset" type declarations in there to get the RTL stuff to display correctly. This isn't very efficient as it's adding a lot of extra CSS you can potentially avoid. So the first option is really the cleanest but probably requires the most upkeep.

@C-Lodder
Copy link
Member

C-Lodder commented Nov 15, 2016

We're using SCSS so we simply need an rtl.scss file. When compiling, we can then generate 2 CSS files. One with the RTL styling and one without. Then load one of the CSS files, depending on the RTL setting.

No need to have 2 HTTP requests or load the RTL code in the primary CSS file even though user's may not need it.

@andrepereiradasilva
Copy link
Contributor Author

As long is easy to mantain 😉 i have no issue with that.

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

Charlie you have access to that repo too, so you can see what I went through for "proper" RTL just with core BS2. I still don't know how I'm maintaining a template but ¯\_(ツ)_/¯. Just as long as in the end it's not as bug filled as the current RTL implementation.

@C-Lodder
Copy link
Member

@andrepereiradasilva - Maintaining styling will require Node + Ruby. I'll write up the documentation but people will have to be prepared for this change.

@mbabker - I can imagine the trial and error process was tedious

@andrepereiradasilva
Copy link
Contributor Author

Node + Ruby ?

@C-Lodder
Copy link
Member

C-Lodder commented Nov 15, 2016

Node JS.......Yes, cause we're using SCSS. I'm not going to leave SCSS compiling down to anything other than NPM packages for production. Ruby is then used for the SCSS linting

We also use NPM for vendor libraries and minifying.

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 15, 2016

so, let me see if i understand this: a simple contributor wants to contribute changing a few lines of css
he need to:

  1. Change SCSS files - not touch CSS files
  2. Install Node + Ruby to compile the SCSS?

I understand what you guys are doing, but do you guys really think this is reasonable?

I speak for me also 😉 , i never worked with node or ruby, neither ever installed it.
I have no problem in learn how to do that, but still ... for simple contributors...

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

As long as folks can help with that it's fine. The PHP compilers have too
many issues to be relied on, they're only good for being executable in a
language people already know and can run.

Also consider even today you can't just make a CSS change via GitHub alone.
You have to clone the repo locally. This just changes the toolset.

On Tue, Nov 15, 2016 at 8:39 AM andrepereiradasilva <
[email protected]> wrote:

so, let me see if i understand this a simple contributor wants to
contribute changeing a few lines of css
we need to:

  1. Change SCSS files - not touch CSS files
  2. Install Node + Ruby to compile the SCSS?

I understand what you guys are doing, but do you guys really think this is
reasonable?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#166 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoeIAzIaKK3okpo4aYtXobaP-1Lqyks5q-cQJgaJpZM4KycbE
.

  • Michael Please pardon any errors, this message was sent from my iPhone.

@andrepereiradasilva
Copy link
Contributor Author

Also consider even today you can't just make a CSS change via GitHub alone.

Yes, i know that.
TBH, i actually do manually the changes in less and the css and really avoid doing that because i'm lazy 😉.

That's why i'm saying that can cause a barrier on contributions on this, but if you guys think it's ok...

@andrepereiradasilva
Copy link
Contributor Author

probably not, but is it possible for travis or some robots to do that compilation on-the-fly when PR are submited?

@C-Lodder
Copy link
Member

C-Lodder commented Nov 15, 2016

Yes, that's correct and for me, that's reasonable. People don't even use the LESS files correctly in J3.x, nevermind CSS. The current LESS files just make me want to burst our in tears sometimes, they're that bad.

Using Node and Ruby allows:

  • Autoprefixing (something I don't see happen properly in J3.x)
  • Linting (again, doesn't happen in J3.x. The CSS structure is absolutely dreadful)
  • Update vendor libraries easily
  • Minify everything

As mentioned, I will write up a documentation page explaining everything, step by step. If the steps are followed properly, it will be easy.

Even now in J3.x, the generatecss.php need to be executed, and that's assuming you've added PHP to your PATHS (in Windows, not sure about OSX)

@andrepereiradasilva
Copy link
Contributor Author

andrepereiradasilva commented Nov 15, 2016

@C-Lodder as said i understand WHY you do this.

But please read my comment above

probably not, but is it possible for travis or some robots to do that compilation on-the-fly when PR are submited?

I mean can't we make travis compile the stuff when checking PR? This way you only would need to change css and js original files, the rest would be auto compiled.

@C-Lodder
Copy link
Member

That would mean if a user want to make some CSS changes, they need to use an alternative to compile locally when testing. Travis compiler will be different to the local compiler. As Babker said, the PHP compiles have lots of issues.

No idea if Travis supports SCSS compiling though.

Either way, Node is used for other things, not just the SCSS

@mbabker
Copy link
Contributor

mbabker commented Nov 15, 2016

Yes we can have CI compile the stuff on the fly but our CI (either Travis or Jenkins) would have to be smart enough to be able to pull the branch, run the compiler, and push back to it if it detects a file difference.

We can't make the CI take the CSS changes and merge them back to SCSS, a user has to edit the right file(s) for it to work.

Yes, it might be an extra barrier to contribution, but either we can do things right or do things easy with broken tooling. 3.x already has broken tooling and makes it semi-easy.

@andrepereiradasilva
Copy link
Contributor Author

don't misunderstood me, i'm 100% in favour of consistency and error prone code as you may notice from my PRs.

i'm making a little the devil advocat part here 😉 i just want to point out that issue.

if we, somehow, could make it automatticly it would be easier for everyone and easier to mantain too.
just my 2 cents

@mbabker mbabker reopened this Nov 23, 2016
@C-Lodder
Copy link
Member

When I initialy suggested this, I think people seemed to be under the impression it was only supposed to be used for those controbuting towards the Joomla core. However this will also be used for template providers. It will allow people to add their own SCSS to a custom.scss and compile on 3rd party templates.

@ciar4n ciar4n closed this as completed in 27c090e Jan 23, 2017
@C-Lodder C-Lodder reopened this Jan 23, 2017
@C-Lodder
Copy link
Member

Done on main repo joomla/joomla-cms#16593

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

No branches or pull requests

5 participants