bmbrands / theme_bootstrap

A Moodle theme based on the Bootstrap CSS framework
113 stars 112 forks source link

Start planning for RTL from Bootstrap 3.2 #141

Open ds125v opened 10 years ago

ds125v commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 :smile:

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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/bmbrands/theme_bootstrap/issues/141#issuecomment-37934960 .

nadavkav commented 10 years ago

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 commented 10 years ago

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/bmbrands/theme_bootstrap/issues/141#issuecomment-40732982 .

nadavkav commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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/bmbrands/theme_bootstrap/issues/141#issuecomment-41209310 .

ds125v commented 10 years ago

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 commented 10 years ago

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

gjb2048 commented 10 years ago

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: https://github.com/twbs/bootstrap/issues/13026 and https://github.com/twbs/bootstrap/pull/12949 so progress is being made.

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