Merge pull request #16200 from edx/andya/bootstrap-rtl-support

Add RTL support for Bootstrap CSS

cms/static/sass/bootstrap/_legacy.scss

+// ------------------------------------
+// Studio support for legacy Sass partials
+//
+// The intention is that this makes it
+// easier to reuse existing partials
+// that were not built with Bootstrap
+// in mind.
+// ------------------------------------
+
+@import 'vendor/bi-app/bi-app-ltr'; // set the layout for left to right languages
+
+@mixin font-size($sizeValue: 16) {
+  font-size: ($sizeValue/10) + rem;
+}
+
+// Support .sr as a synonym for .sr-only
+.sr {
+    @extend .sr-only;
+}

cms/static/sass/bootstrap/studio-main.scss

@@ -6,6 +6,9 @@
 @import 'cms/bootstrap/theme';
 @import 'bootstrap/scss/bootstrap';
 
+// Legacy support
+@import 'legacy';
+
 // Variables
 @import 'mixins';
 @import 'variables';

lms/static/sass/bootstrap/_base.scss

+// Open edX: LMS base styles
+// ============================
+
+body {
+  text-align: initial !important;  // Bootstrap hard-codes left alignment
+}

lms/static/sass/bootstrap/lms-main.scss

@@ -9,7 +9,8 @@
 // Legacy support
 @import 'legacy';
 
-// Variables
+// Base
+@import 'base';
 @import 'variables';
 
 // Elements

lms/templates/main.html

@@ -65,8 +65,15 @@ from pipeline_mako import render_require_js_path_overrides
   <link rel="icon" type="image/x-icon" href="${static.url(static.get_value('favicon_path', settings.FAVICON_PATH))}" />
 
   <%static:css group='style-vendor'/>
-  % if uses_bootstrap or '/' in self.attr.main_css:
-    <link rel="stylesheet" href="${static.url(self.attr.main_css)}" type="text/css" media="all" />
+  % if '/' in self.attr.main_css:
+    % if get_language_bidi():
+      <%
+      rtl_css_file = self.attr.main_css.replace('.css', '-rtl.css')
+      %>
+      <link rel="stylesheet" href="${unicode(static.url(rtl_css_file))}" type="text/css" media="all" />
+    % else:
+      <link rel="stylesheet" href="${static.url(self.attr.main_css)}" type="text/css" media="all" />
+    % endif
   % else:
     <%static:css group='${self.attr.main_css}'/>
   % endif

package.json

@@ -29,6 +29,7 @@
     "react": "^15.5.4",
     "react-dom": "^15.5.4",
     "requirejs": "~2.3.2",
+    "rtlcss": "^2.2.0",
     "string-replace-webpack-plugin": "^0.1.3",
     "uglify-js": "2.7.0",
     "underscore": "~1.8.3",

pavelib/assets.py

@@ -597,11 +597,45 @@ def _compile_sass(system, theme, debug, force, timing_info):
                 source_comments=source_comments,
                 output_style=output_style,
             )
+
+        # For Sass files without explicit RTL versions, generate
+        # an RTL version of the CSS using the rtlcss library.
+        for sass_file in glob.glob(sass_source_dir + '/**/*.scss'):
+            if should_generate_rtl_css_file(sass_file):
+                source_css_file = sass_file.replace(sass_source_dir, css_dir).replace('.scss', '.css')
+                target_css_file = source_css_file.replace('.css', '-rtl.css')
+                sh("rtlcss {source_file} {target_file}".format(
+                    source_file=source_css_file,
+                    target_file=target_css_file,
+                ))
+
+        # Capture the time taken
+        if not dry_run:
             duration = datetime.now() - start
             timing_info.append((sass_source_dir, css_dir, duration))
     return True
 
 
+def should_generate_rtl_css_file(sass_file):
+    """
+    Returns true if a Sass file should have an RTL version generated.
+    """
+    # Don't generate RTL CSS for partials
+    if path(sass_file).name.startswith('_'):
+        return False
+
+    # Don't generate RTL CSS if the file is itself an RTL version
+    if sass_file.endswith('-rtl.scss'):
+        return False
+
+    # Don't generate RTL CSS if there is an explicit Sass version for RTL
+    rtl_sass_file = path(sass_file.replace('.scss', '-rtl.scss'))
+    if rtl_sass_file.exists():
+        return False
+
+    return True
+
+
 def process_npm_assets():
     """
     Process vendor libraries installed via NPM.

pavelib/paver_tests/test_assets.py

@@ -14,7 +14,7 @@ from ..utils.envs import Env
 from .utils import PaverTestCase
 
 ROOT_PATH = path(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
-TEST_THEME = ROOT_PATH / "common/test/test-theme"  # pylint: disable=invalid-name
+TEST_THEME_DIR = ROOT_PATH / "common/test/test-theme"  # pylint: disable=invalid-name
 
 
 @ddt.ddt
@@ -40,30 +40,36 @@ class TestPaverAssetTasks(PaverTestCase):
         """
         parameters = options.split(" ")
         system = []
-        if "--system=studio" not in parameters:
-            system += ["lms"]
-        if "--system=lms" not in parameters:
-            system += ["studio"]
-        debug = "--debug" in parameters
-        force = "--force" in parameters
+        if '--system=studio' not in parameters:
+            system += ['lms']
+        if '--system=lms' not in parameters:
+            system += ['studio']
+        debug = '--debug' in parameters
+        force = '--force' in parameters
         self.reset_task_messages()
-        call_task('pavelib.assets.compile_sass', options={"system": system, "debug": debug, "force": force})
+        call_task('pavelib.assets.compile_sass', options={'system': system, 'debug': debug, 'force': force})
         expected_messages = []
         if force:
-            expected_messages.append("rm -rf common/static/css/*.css")
-        expected_messages.append("libsass common/static/sass")
+            expected_messages.append('rm -rf common/static/css/*.css')
+        expected_messages.append('libsass common/static/sass')
 
         if "lms" in system:
             if force:
-                expected_messages.append("rm -rf lms/static/css/*.css")
-            expected_messages.append("libsass lms/static/sass")
+                expected_messages.append(u'rm -rf lms/static/css/*.css')
+            expected_messages.append(u'libsass lms/static/sass')
+            expected_messages.append(
+                u'rtlcss lms/static/css/bootstrap/lms-main.css lms/static/css/bootstrap/lms-main-rtl.css'
+            )
             if force:
-                expected_messages.append("rm -rf lms/static/certificates/css/*.css")
-            expected_messages.append("libsass lms/static/certificates/sass")
+                expected_messages.append(u'rm -rf lms/static/certificates/css/*.css')
+            expected_messages.append(u'libsass lms/static/certificates/sass')
         if "studio" in system:
             if force:
-                expected_messages.append("rm -rf cms/static/css/*.css")
-            expected_messages.append("libsass cms/static/sass")
+                expected_messages.append(u'rm -rf cms/static/css/*.css')
+            expected_messages.append(u'libsass cms/static/sass')
+            expected_messages.append(
+                u'rtlcss cms/static/css/bootstrap/studio-main.css cms/static/css/bootstrap/studio-main-rtl.css'
+            )
 
         self.assertEquals(self.task_messages, expected_messages)
 
@@ -92,52 +98,83 @@ class TestPaverThemeAssetTasks(PaverTestCase):
         parameters = options.split(" ")
         system = []
 
-        if "--system=studio" not in parameters:
-            system += ["lms"]
+        if '--system=studio' not in parameters:
+            system += ['lms']
         if "--system=lms" not in parameters:
-            system += ["studio"]
-        debug = "--debug" in parameters
-        force = "--force" in parameters
+            system += ['studio']
+        debug = '--debug' in parameters
+        force = '--force' in parameters
 
         self.reset_task_messages()
         call_task(
             'pavelib.assets.compile_sass',
-            options={"system": system, "debug": debug, "force": force, "theme_dirs": [TEST_THEME.dirname()],
-                     "themes": [TEST_THEME.basename()]},
+            options=dict(
+                system=system,
+                debug=debug,
+                force=force,
+                theme_dirs=[TEST_THEME_DIR.dirname()],
+                themes=[TEST_THEME_DIR.basename()]
+            ),
         )
         expected_messages = []
         if force:
-            expected_messages.append("rm -rf common/static/css/*.css")
-        expected_messages.append("libsass common/static/sass")
-
-        if "lms" in system:
-            expected_messages.append("mkdir_p " + repr(TEST_THEME / "lms/static/css"))
+            expected_messages.append(u'rm -rf common/static/css/*.css')
+        expected_messages.append(u'libsass common/static/sass')
 
+        if 'lms' in system:
+            expected_messages.append(u'mkdir_p ' + repr(TEST_THEME_DIR / 'lms/static/css'))
             if force:
-                expected_messages.append("rm -rf " + str(TEST_THEME) + "/lms/static/css/*.css")
+                expected_messages.append(
+                    u'rm -rf {test_theme_dir}/lms/static/css/*.css'.format(test_theme_dir=str(TEST_THEME_DIR))
+                )
             expected_messages.append("libsass lms/static/sass")
+            expected_messages.append(
+                u'rtlcss {test_theme_dir}/lms/static/css/bootstrap/lms-main.css'
+                u' {test_theme_dir}/lms/static/css/bootstrap/lms-main-rtl.css'.format(
+                    test_theme_dir=str(TEST_THEME_DIR),
+                )
+            )
             if force:
-                expected_messages.append("rm -rf " + str(TEST_THEME) + "/lms/static/css/*.css")
-            expected_messages.append("libsass " + str(TEST_THEME) + "/lms/static/sass")
+                expected_messages.append(
+                    'rm -rf {test_theme_dir}/lms/static/css/*.css'.format(test_theme_dir=str(TEST_THEME_DIR))
+                )
+            expected_messages.append(u'libsass {test_theme_dir}/lms/static/sass'.format(test_theme_dir=str(TEST_THEME_DIR)))
             if force:
-                expected_messages.append("rm -rf lms/static/css/*.css")
-            expected_messages.append("libsass lms/static/sass")
+                expected_messages.append(u'rm -rf lms/static/css/*.css')
+            expected_messages.append(u'libsass lms/static/sass')
+            expected_messages.append(
+                u'rtlcss lms/static/css/bootstrap/lms-main.css lms/static/css/bootstrap/lms-main-rtl.css'
+            )
             if force:
-                expected_messages.append("rm -rf lms/static/certificates/css/*.css")
-            expected_messages.append("libsass lms/static/certificates/sass")
+                expected_messages.append(u'rm -rf lms/static/certificates/css/*.css')
+            expected_messages.append(u'libsass lms/static/certificates/sass')
 
         if "studio" in system:
-            expected_messages.append("mkdir_p " + repr(TEST_THEME / "cms/static/css"))
+            expected_messages.append(u'mkdir_p ' + repr(TEST_THEME_DIR / 'cms/static/css'))
             if force:
-                expected_messages.append("rm -rf " + str(TEST_THEME) + "/cms/static/css/*.css")
-            expected_messages.append("libsass cms/static/sass")
+                expected_messages.append(
+                    u'rm -rf {test_theme_dir}/cms/static/css/*.css'.format(test_theme_dir=str(TEST_THEME_DIR))
+                )
+            expected_messages.append(u'libsass cms/static/sass')
+            expected_messages.append(
+                u'rtlcss {test_theme_dir}/cms/static/css/bootstrap/studio-main.css'
+                u' {test_theme_dir}/cms/static/css/bootstrap/studio-main-rtl.css'.format(
+                    test_theme_dir=str(TEST_THEME_DIR),
+                )
+            )
             if force:
-                expected_messages.append("rm -rf " + str(TEST_THEME) + "/cms/static/css/*.css")
-            expected_messages.append("libsass " + str(TEST_THEME) + "/cms/static/sass")
-
+                expected_messages.append(
+                    u'rm -rf {test_theme_dir}/cms/static/css/*.css'.format(test_theme_dir=str(TEST_THEME_DIR))
+                )
+            expected_messages.append(
+                u'libsass {test_theme_dir}/cms/static/sass'.format(test_theme_dir=str(TEST_THEME_DIR))
+            )
             if force:
-                expected_messages.append("rm -rf cms/static/css/*.css")
-            expected_messages.append("libsass cms/static/sass")
+                expected_messages.append(u'rm -rf cms/static/css/*.css')
+            expected_messages.append(u'libsass cms/static/sass')
+            expected_messages.append(
+                u'rtlcss cms/static/css/bootstrap/studio-main.css cms/static/css/bootstrap/studio-main-rtl.css'
+            )
 
         self.assertEquals(self.task_messages, expected_messages)
 
@@ -190,10 +227,10 @@ class TestPaverWatchAssetTasks(TestCase):
         Test the Paver watch asset tasks with theming enabled.
         """
         self.expected_sass_directories.extend([
-            path(TEST_THEME) / 'lms/static/sass',
-            path(TEST_THEME) / 'lms/static/sass/partials',
-            path(TEST_THEME) / 'cms/static/sass',
-            path(TEST_THEME) / 'cms/static/sass/partials',
+            path(TEST_THEME_DIR) / 'lms/static/sass',
+            path(TEST_THEME_DIR) / 'lms/static/sass/partials',
+            path(TEST_THEME_DIR) / 'cms/static/sass',
+            path(TEST_THEME_DIR) / 'cms/static/sass/partials',
         ])
 
         with patch('pavelib.assets.SassWatcher.register') as mock_register:
@@ -201,8 +238,11 @@ class TestPaverWatchAssetTasks(TestCase):
                 with patch('pavelib.assets.execute_webpack_watch') as mock_webpack:
                     call_task(
                         'pavelib.assets.watch_assets',
-                        options={"background": True, "theme_dirs": [TEST_THEME.dirname()],
-                                 "themes": [TEST_THEME.basename()]},
+                        options={
+                            "background": True,
+                            "theme_dirs": [TEST_THEME_DIR.dirname()],
+                            "themes": [TEST_THEME_DIR.basename()]
+                        },
                     )
                     self.assertEqual(mock_register.call_count, 2)
                     self.assertEqual(mock_webpack.call_count, 1)