From fc3af68e3be01a5013b102ae3c4ed62b36f9e0bc Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Tue, 10 Oct 2017 20:17:40 -0400 Subject: [PATCH] Add RTL support for Bootstrap CSS LEARNER-1705 --- cms/static/sass/bootstrap/_legacy.scss | 19 +++ cms/static/sass/bootstrap/studio-main.scss | 3 + lms/static/sass/bootstrap/_base.scss | 6 + lms/static/sass/bootstrap/lms-main.scss | 3 +- lms/templates/main.html | 11 +- package.json | 1 + pavelib/assets.py | 34 +++++ pavelib/paver_tests/test_assets.py | 138 +++++++++++++-------- 8 files changed, 163 insertions(+), 52 deletions(-) create mode 100644 cms/static/sass/bootstrap/_legacy.scss create mode 100644 lms/static/sass/bootstrap/_base.scss diff --git a/cms/static/sass/bootstrap/_legacy.scss b/cms/static/sass/bootstrap/_legacy.scss new file mode 100644 index 0000000000..c53fb99f9e --- /dev/null +++ b/cms/static/sass/bootstrap/_legacy.scss @@ -0,0 +1,19 @@ +// ------------------------------------ +// 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; +} diff --git a/cms/static/sass/bootstrap/studio-main.scss b/cms/static/sass/bootstrap/studio-main.scss index f664fceb39..af0730a5fd 100644 --- a/cms/static/sass/bootstrap/studio-main.scss +++ b/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'; diff --git a/lms/static/sass/bootstrap/_base.scss b/lms/static/sass/bootstrap/_base.scss new file mode 100644 index 0000000000..98e514f7c4 --- /dev/null +++ b/lms/static/sass/bootstrap/_base.scss @@ -0,0 +1,6 @@ +// Open edX: LMS base styles +// ============================ + +body { + text-align: initial !important; // Bootstrap hard-codes left alignment +} diff --git a/lms/static/sass/bootstrap/lms-main.scss b/lms/static/sass/bootstrap/lms-main.scss index 108c3cfecd..9f2b7a6230 100644 --- a/lms/static/sass/bootstrap/lms-main.scss +++ b/lms/static/sass/bootstrap/lms-main.scss @@ -9,7 +9,8 @@ // Legacy support @import 'legacy'; -// Variables +// Base +@import 'base'; @import 'variables'; // Elements diff --git a/lms/templates/main.html b/lms/templates/main.html index 7bc555dda9..cf1477d041 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -65,8 +65,15 @@ from pipeline_mako import render_require_js_path_overrides <%static:css group='style-vendor'/> - % if uses_bootstrap or '/' in self.attr.main_css: - + % if '/' in self.attr.main_css: + % if get_language_bidi(): + <% + rtl_css_file = self.attr.main_css.replace('.css', '-rtl.css') + %> + + % else: + + % endif % else: <%static:css group='${self.attr.main_css}'/> % endif diff --git a/package.json b/package.json index 4d2a900cb9..38d0e71ca4 100644 --- a/package.json +++ b/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", diff --git a/pavelib/assets.py b/pavelib/assets.py index afca91b335..1ddf112683 100644 --- a/pavelib/assets.py +++ b/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. diff --git a/pavelib/paver_tests/test_assets.py b/pavelib/paver_tests/test_assets.py index 7859d26339..181a766f97 100644 --- a/pavelib/paver_tests/test_assets.py +++ b/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)