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

Start planning for RTL from Bootstrap 3.2 #141

Open
ds125v opened this issue Mar 4, 2014 · 18 comments
Open

Start planning for RTL from Bootstrap 3.2 #141

ds125v opened this issue Mar 4, 2014 · 18 comments

Comments

@ds125v
Copy link
Collaborator

ds125v commented Mar 4, 2014

Looks like they've figured out what they're going to do in Bootstrap for RTL:

https://github.com/twbs/bootstrap/tree/rtl_via_css_flip/less

Basically they output the CSS as normal and then call another node module that parses through the CSS and changes "left" to "right" and vice versa (and a couple of similar, though more invoved things).

They then need a CSS comment for things that shouldn't get flipped.

It's also meaning things like .blockquote.pull-right are now becoming .blockquote-reversed and various other little tweaks to make Bootstrap more RTL aware.

If we do the same then we can delete most (though not all) of the RTL code from our theme and generate it automatically.

The complications are that the Bootstrap solution seems based on sending one CSS file to RTL people and another CSS to LTR people. Moodle's approach is based on having both sets of CSS in the same file. Moodle's approach isn't very good so maybe we can convince them to switch to a method that only sends the relevant CSS based on current language, though previous attempts to do so have run up against the people who do the RTL work and who feel that if RTL rules were written in different files (to allow them to be easily skipped) then they would be forgotten about. Possibly having automation for 99% of the work (and that 1% being done in the same file) would be good enough for them to support this change.

So as it stands we can easily generate one theme for purely RTL sites, and one for purely LTR sites but sites that use languages in both directions will cause us trouble.

Also plugins etc. will continue to use the old way, but that shouldn't cause any problems as the two can co-exist, just cause a bit of inefficiency.

@bmbrands
Copy link
Owner

bmbrands commented Mar 4, 2014

It would be great if we could remove most of the .dir-rtl from our less files. And If the theme would not be using the moodle css parsing/minifying/caching mechanisms it would be very easy to serve the right CSS file to the right type of user.
Switching between different bootswatches based on user preferences would be very fast and easy.
And I guess we would not have to worry about Source map mechanisms not working because of Moodle's CSS parsing. (from what I understand from #140 , please correct me if I am wrong)

The only thing missing would be converting the [[pix:theme|img]] and [[font]] stuff but I am guessing we could fix that using Grunt too.

I know this would not easily be accepted by Moodle but is there any reason we should use Moodle's CSS parsing?

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 4, 2014

I think it would be a big job. The key problem is that as you (un-)install moodle plugins they each bring their own CSS, which needs to be reflected in the final output. We're already bypassing a lot of their system by using LESS and generating a single, pre-minified CSS file, but I don't think it would be possible to go much further.

Though having said that, it's always been a long term goal of mine to have the basic CSS provided by linking to a CDN hosted copy of Bootstrap's CSS, but we're not quite there yet.

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 10, 2014

Gave this a try by running flip-css manually, seemed to work pretty well (though there's RTL stuff in our Theme and in Moodle that causes issues because it gets flipped twice and ends up the wrong way round, so you have to disable/delete those to see the real effect).

I also tried automatically sending moodle.css or moodle-rtl.css based on current language, which also worked well. Though Moodle will then cache that CSS for the whole site regardless of user language.

There's also a 3rd party version that creates an extra RTL file. If we can't get Moodle core to switch on language then we might need to do something like that for sites with both LTR and RTL content.

@nadavkav
Copy link

Hi David and Bas,
lucky me, I found your discussions while researching the best Moodle bootstrap based theme to start developing an RTL enabled theme for the next academic year.

As you can see on the Tracker, I took major part in fixing Moodle themes over the years to support RTL. And also can probably attribute what was mentioned above ("Moodle's approach is based on having both sets of CSS in the same file.") to my suggestions and preferences before the era of Bootstrap and LESS. So with your permission... I would like to jump in 😄

Considering what was already discussed in : https://tracker.moodle.org/browse/MDL-39094

You (we) should probably wait for the official BS 3.2 + RTL support to come out. Than, clear all the .dir-rtl fixes from the moodle less files and start all over again from scratch. (Immediately after a few hours of silent meditation)

Until Moodle supports splitting CSS files, based on directionality or page layout or context. Which will enable you (us) to send different CSS files to the browser, you should (probably) send one big CSS file where the RTL CSS rules follow the LTR ones.

Indeed it is still easier for me to manage the .dir-rtl CSS rules when they follow the LTR rules in the same LESS or CSS file. And I guess Grunt could probably split them apart when we have the need and ability to send a different CSS file for RTL users. But if not, I will be willing to let it go in favor of better browser rendering performance and less bandwidth used for mobiles.

One more thing to consider, from my experience running many Moodle systems in Israel, All RTL enabled system are also running some LTR courses. So there is no RTL only system out there. As far as I know. And we can not rely on generating only one (LTR or RTL) theme version.

For example, An entire system could be set to RTL (Hebrew or Arabic language) and several courses are set to LTR (English). And while the student browsing the course in English, when he/she goes to the MY/Profile pages they see them in RTL (Hebrew/Arabic). Since they are considered - system context.

What do you think?

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 17, 2014

Hi Nadav,

I was just about to come and ask your opinion on the stuff we've done here, as I think it's now at a stage where it needs someone who actually understands an RTL language to comment.

I don't know if you've downloaded the latest version of this theme, but we've already deleted all the dir-rtl rules. Most are now being auto-generated each time the LTR version is saved by a grunt task. Though some don't have an LTR equivalent as they were for TinyMCE or YUI2 or for form fields that remain LTR in RTL languages, so we've split those out into their own CSS files.

The theme then detects the direction of the current language and sends the correct direction. This works fine in Moodles with only one text direction, but in dual-direction sites, since Moodle's CSS caching doesn't understand that the file is associated with a specific text direction, it'll store that initial file and deliver it for every page regardless of direction when you don't have theme developer mode on.

There's various short term workarounds that could come into play, basically auto-generating two separate themes and applying them to different courses but I think that the best approach long term is for Moodle to request one of the two files on each page. It already knows which direction it needs when writing the body tag and adding the dir-rtl tag, so it can probably request the appropriate CSS for the page at the same time. If you can think of any problems with that approach, I'd be interested in hearing about them. In theory it should be identical to what we do now, but with less CSS.

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 18, 2014

I've started writing a script to generate bootstrap-rtl-plus.css (which is a file that gets appended onto moodle.css and add .dir-rtl versions of some of the rules).

It starts with moodle-rtl.css (which is nicely formatted so easy to rewrite via regex) and strips out any rule that doesn't match the text left, right, background-position, border etc., then adds .dir-rtl to the start of every class definition. It then runs it through the less compiler to strip out any empty rules (and check it's not got mangled in the conversion process)

The only really tricky part is that some rules apply to the body tag and so don't need a space between .dir-rtl and the rule, but others do.

Currently the output is about a third of the size of the single direction rules, but I think I can reduce that a little.

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 18, 2014

So that's mostly working, there's some interaction with the -push and -pull classes added to the blocks, but that's not a CSS thing as such, more to do with the layout. If I remove the classes it all seems to work, except that the columns are in their source order.

Currently it's done with awk and sed, I wonder if there's a grunt awk plugin.

@ds125v
Copy link
Collaborator Author

ds125v commented Mar 18, 2014

I hacked flipcss so that it wouldn't output the non-flipped CSS. However...

In the process of testing that I realised that I forgot that if you have both RTL and LTR in the same file, you not only need to flip the CSS you also need to reset all the LTR rules. From looking at the rework framework for CSS rewriting, I think all that should be possible, but probably with more work than I can put into it right now, especially because I don't think it's the right way forward anyway.

@nadavkav
Copy link

Woow what a progress :-)

I will have more time later this evening to dig into this and send my
feedback.

Keep digging David, keep digging :-)

On 18 March 2014 15:57, David Scotson notifications@github.com wrote:

I hacked flipcss so that it wouldn't output the non-flipped CSS. However...

In the process of testing that I realised that I forgot that if you have
both RTL and LTR in the same file, you not only need to flip the CSS you
also need to reset all the LTR rules. From looking at the rework framework
for CSS rewriting, I think all that should be possible, but probably with
more work than I can put into it right now, especially because I don't
think it's the right way forward anyway.

Reply to this email directly or view it on GitHubhttps://github.com//issues/141#issuecomment-37934960
.

@nadavkav
Copy link

Finally, the time has come (Everyone here, Israel, is on holidays, for a week)
I have pulled the latest bootstrap3 and started testing it on RTL mode.

@gjb2048
Copy link
Collaborator

gjb2048 commented Apr 17, 2014

Nice one Nadav :) - I have Bootstrap as a parent in Shoehorn and have seen
RTL working. I've not noticed anything yet!

http://about.me/gjbarnard

On 17 April 2014 17:21, Nadav Kavalerchik notifications@github.com wrote:

Finally, the time has come (Everyone here, Israel, is on holidays, for a
week)
I have pulled the latest bootstrap3 and started testing it on RTL mode.


Reply to this email directly or view it on GitHubhttps://github.com//issues/141#issuecomment-40732982
.

@nadavkav
Copy link

Indeed, I have gone thought a lot of course administration pages, activities and resources and it looks great!

I will start new issues for anything I come across.

@ds125v
Copy link
Collaborator Author

ds125v commented Apr 23, 2014

Hi Nadav,

thanks for the testing, it's much appreciated.

I've kind of been busy recently with Moodle Moot UK and starting a new job so not had much time for this lately, though at the developer day at the end of the Moot I had a chat with Sam Marshall about the possibility of sending LTR and RTL as two seperate CSS files, sending one or the other based on the current page.

@nadavkav
Copy link

Hi David,

It sound very useful to send two different sets of CSS files to the browser
for LTR and RTL. But as you said before, it involves some core changes and
we will have to take it into the tracker for peer review and discussion.

Congratulations with the new job :-)

On 23 April 2014 23:17, David Scotson notifications@github.com wrote:

Hi Nadav,

thanks for the testing, it's much appreciated.

I've kind of been busy recently with Moodle Moot UK and starting a new job
so not had much time for this lately, though at the developer day at the
end of the Moot I had a chat with Sam Marshall about the possibility of
sending LTR and RTL as two seperate CSS files, sending one or the other
based on the current page.


Reply to this email directly or view it on GitHubhttps://github.com//issues/141#issuecomment-41209310
.

@ds125v
Copy link
Collaborator Author

ds125v commented Apr 24, 2014

Yep, changes to core are definately more work, but that's been part of the strategy from the start. After a while you can't make much progress without it. We can demonstrate the benefits (e.g. having a 99% complete RTL version of upstream Bootstrap 3, saving 10% of the CSS size for both RTL and LTR users etc.) within a theme that works for RTL and LTR but not both at the same time. That'll require some interest from core.

@kisin
Copy link

kisin commented Jul 5, 2014

where is the RTL support for 3.2.0 you all talked about?

@gjb2048
Copy link
Collaborator

gjb2048 commented Jul 5, 2014

The source Bootstrap RTL support is outside of our control and has been delayed a few times. 3.2.0 in the release notes does link to: twbs/bootstrap#13026 and twbs/bootstrap#12949 so progress is being made.

It will happen when it can happen. Is there an outstanding issue you know of?

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

No branches or pull requests

6 participants
@kisin @nadavkav @bmbrands @ds125v @gjb2048 and others