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

Add RTL support for Bootstrap CSS #16200

Merged
merged 1 commit into from
Oct 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions cms/static/sass/bootstrap/_legacy.scss
Original file line number Diff line number Diff line change
@@ -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;
}
3 changes: 3 additions & 0 deletions cms/static/sass/bootstrap/studio-main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
@import 'cms/bootstrap/theme';
@import 'bootstrap/scss/bootstrap';

// Legacy support
@import 'legacy';

// Variables
@import 'mixins';
@import 'variables';
Expand Down
6 changes: 6 additions & 0 deletions lms/static/sass/bootstrap/_base.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Open edX: LMS base styles
// ============================

body {
text-align: initial !important; // Bootstrap hard-codes left alignment
}
3 changes: 2 additions & 1 deletion lms/static/sass/bootstrap/lms-main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
// Legacy support
@import 'legacy';

// Variables
// Base
@import 'base';
@import 'variables';

// Elements
Expand Down
11 changes: 9 additions & 2 deletions lms/templates/main.html
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,15 @@
<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
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
34 changes: 34 additions & 0 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
138 changes: 89 additions & 49 deletions pavelib/paver_tests/test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
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
Expand All @@ -40,30 +40,36 @@ def test_compile_sass(self, options):
"""
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)

Expand Down Expand Up @@ -92,52 +98,83 @@ def test_compile_theme_sass(self, options):
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)

Expand Down Expand Up @@ -190,19 +227,22 @@ def test_watch_theme_assets(self):
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:
with patch('pavelib.assets.PollingObserver.start'):
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)
Expand Down