Skip to content

Commit

Permalink
Decouple XModule styles from LMS/Studio styles (attempt 2) (#32188)
Browse files Browse the repository at this point in the history
This basically changes how the xmodule static files are
generated and consumed in order to separate the Xblock
styles from general style files. Includes:

* build: decople XModule style assets by using a custom webpack loader
* build: move scss imports to its specific file
* build: fix: add system dirs to theme lookup paths. 

This is an amendment to #32018

Addressing the issue #31624
  • Loading branch information
andrey-canon authored May 5, 2023
1 parent 065f894 commit c34f8ef
Show file tree
Hide file tree
Showing 18 changed files with 167 additions and 14 deletions.
3 changes: 2 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,8 @@
WEBPACK_LOADER = {
'DEFAULT': {
'BUNDLE_DIR_NAME': 'bundles/',
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json'),
'LOADER_CLASS': 'xmodule.util.xmodule_django.XModuleWebpackLoader',
},
'WORKERS': {
'BUNDLE_DIR_NAME': 'bundles/',
Expand Down
2 changes: 0 additions & 2 deletions cms/static/sass/_build-v1.scss
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@

// +Xmodule
// ====================
@import 'xmodule/modules/css/module-styles.scss';
@import 'xmodule/descriptors/css/module-styles.scss';
@import 'xmodule/headings';
@import 'elements/xmodules'; // styling for Studio-specific contexts
@import 'developer'; // used for any developer-created scss that needs further polish/refactoring
Expand Down
3 changes: 2 additions & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2696,7 +2696,8 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
WEBPACK_LOADER = {
'DEFAULT': {
'BUNDLE_DIR_NAME': 'bundles/',
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json'),
'LOADER_CLASS': 'xmodule.util.xmodule_django.XModuleWebpackLoader',
},
'WORKERS': {
'BUNDLE_DIR_NAME': 'bundles/',
Expand Down
2 changes: 0 additions & 2 deletions lms/static/sass/_build-course.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
@import 'course/base/mixins';
@import 'course/base/base';
@import 'course/base/extends';
@import 'xmodule/modules/css/module-styles.scss';
@import 'course/courseware/courseware';
@import 'course/courseware/sidebar';
@import 'course/courseware/amplifier';
Expand Down Expand Up @@ -57,7 +56,6 @@
@import "course/gradebook";
@import "course/instructor/instructor_2";
@import "course/instructor/email";
@import "xmodule/descriptors/css/module-styles.scss";

// course - ccx_coach
@import "course/ccx_coach/dashboard";
Expand Down
23 changes: 23 additions & 0 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ def get_theme_sass_dirs(system, theme_dir):
css_dir = theme_dir / system / "static" / "css"
certs_sass_dir = theme_dir / system / "static" / "certificates" / "sass"
certs_css_dir = theme_dir / system / "static" / "certificates" / "css"
xmodule_sass_folder = "modules" if system == 'lms' else "descriptors"
xmodule_sass_dir = path("common") / "static" / "xmodule" / xmodule_sass_folder / "scss"

dependencies = SASS_LOOKUP_DEPENDENCIES.get(system, [])
if sass_dir.isdir():
Expand Down Expand Up @@ -197,6 +199,16 @@ def get_theme_sass_dirs(system, theme_dir):
],
})

dirs.append({
"sass_source_dir": xmodule_sass_dir,
"css_destination_dir": path("common") / "static" / "css" / "xmodule",
"lookup_paths": dependencies + [
sass_dir / "partials",
system_sass_dir / "partials",
system_sass_dir,
],
})

# now compile theme sass files for certificate
if system == 'lms':
dirs.append({
Expand All @@ -223,6 +235,8 @@ def get_system_sass_dirs(system):
dirs = []
sass_dir = path(system) / "static" / "sass"
css_dir = path(system) / "static" / "css"
xmodule_sass_folder = "modules" if system == 'lms' else "descriptors"
xmodule_sass_dir = path("common") / "static" / "xmodule" / xmodule_sass_folder / "scss"

dependencies = SASS_LOOKUP_DEPENDENCIES.get(system, [])
dirs.append({
Expand All @@ -234,6 +248,15 @@ def get_system_sass_dirs(system):
],
})

dirs.append({
"sass_source_dir": xmodule_sass_dir,
"css_destination_dir": path("common") / "static" / "css" / "xmodule",
"lookup_paths": dependencies + [
sass_dir / "partials",
sass_dir,
],
})

if system == 'lms':
dirs.append({
"sass_source_dir": path(system) / "static" / "certificates" / "sass",
Expand Down
4 changes: 4 additions & 0 deletions xmodule/css/annotatable/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
* that the LMS did, so the quick fix was to localize the LMS variables not shared by the CMS.
* -Abarrett and Vshnayder
*/
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'lms/theme/variables-v1';

$annotatable--border-color: $gray-l3;
$annotatable--body-font-size: em(14);

Expand Down
3 changes: 3 additions & 0 deletions xmodule/css/capa/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
// * +Problem - Choice Text Group
// * +Problem - Image Input Overrides
// * +Problem - Annotation Problem Overrides
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'lms/theme/variables-v1';

// +Variables - Capa
// ====================
Expand Down
6 changes: 6 additions & 0 deletions xmodule/css/editor/edit.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'cms/theme/variables-v1';
@import 'mixins';

// This is shared CSS between the xmodule problem editor and the xmodule HTML editor.
.editor {
position: relative;
Expand Down
3 changes: 3 additions & 0 deletions xmodule/css/html/display.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'lms/theme/variables-v1';

// HTML component display:
* {
Expand Down
6 changes: 6 additions & 0 deletions xmodule/css/lti/lti.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'bootstrap/scss/variables';
@import 'lms/theme/variables-v1';
@import 'base/mixins';

h2.problem-header {
display: inline-block;
}
Expand Down
5 changes: 5 additions & 0 deletions xmodule/css/poll/display.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'bootstrap/scss/variables';
@import 'lms/theme/variables-v1';

div.poll_question {
@media print {
display: block;
Expand Down
7 changes: 7 additions & 0 deletions xmodule/css/sequence/display.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'bootstrap/scss/variables';
@import 'bootstrap/scss/mixins/breakpoints';
@import 'lms/theme/variables-v1';

$seq-nav-border-color: $border-color !default;
$seq-nav-hover-color: rgb(245, 245, 245) !default;
$seq-nav-link-color: $link-color !default;
Expand Down
5 changes: 5 additions & 0 deletions xmodule/css/tabs/tabs.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
// styles duped from _unit.scss - Edit Header (Component Name, Mode-Editor, Mode-Settings)
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'cms/theme/variables-v1';
@import 'mixins';


.tabs-wrapper {
Expand Down
2 changes: 2 additions & 0 deletions xmodule/css/video/accessible_menu.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import 'base/mixins';

$a11y--gray: rgb(127, 127, 127);
$a11y--blue: rgb(0, 159, 230);
$a11y--gray-d1: shade($gray, 20%);
Expand Down
5 changes: 5 additions & 0 deletions xmodule/css/video/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
// --------
// Defaults: what displays if the icon font doesn't load.
// --------
@import 'vendor/bi-app/bi-app-ltr';
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'bootstrap/scss/variables';
@import 'bootstrap/scss/mixins/breakpoints';
@import 'lms/theme/variables-v1';

// the html target is necessary for xblocks and xmodules, but works across the board

Expand Down
5 changes: 5 additions & 0 deletions xmodule/css/word_cloud/display.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
@import 'bourbon/bourbon';
@import 'lms/theme/variables';
@import 'bootstrap/scss/variables';
@import 'lms/theme/variables-v1';

.input-cloud {
margin: ($baseline/4);
}
Expand Down
17 changes: 9 additions & 8 deletions xmodule/static_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method

def write_module_styles(output_root):
"""Write all registered XModule css, sass, and scss files to output root."""
return _write_styles('.xmodule_display', output_root, XBLOCK_CLASSES, 'get_preview_view_css')
return _write_styles('.xmodule_display', output_root, XBLOCK_CLASSES, 'get_preview_view_css', 'Preview')


def write_module_js(output_root):
Expand All @@ -101,7 +101,7 @@ def write_module_js(output_root):

def write_descriptor_styles(output_root):
"""Write all registered XModuleDescriptor css, sass, and scss files to output root."""
return _write_styles('.xmodule_edit', output_root, XBLOCK_CLASSES, 'get_studio_view_css')
return _write_styles('.xmodule_edit', output_root, XBLOCK_CLASSES, 'get_studio_view_css', 'Studio')


def write_descriptor_js(output_root):
Expand All @@ -120,7 +120,7 @@ def _ensure_dir(directory):
raise


def _write_styles(selector, output_root, classes, css_attribute):
def _write_styles(selector, output_root, classes, css_attribute, suffix):
"""
Write the css fragments from all XModules in `classes`
into `output_root` as individual files, hashed by the contents to remove
Expand All @@ -147,17 +147,18 @@ def _write_styles(selector, output_root, classes, css_attribute):
for class_ in classes:
css_imports[class_].add(fragment_name)

module_styles_lines = []

for class_, fragment_names in sorted(css_imports.items()):
module_styles_lines = []

fragment_names = sorted(fragment_names)
module_styles_lines.append("""{selector}.xmodule_{class_} {{""".format(
class_=class_, selector=selector
))
module_styles_lines.extend(f' @import "{name}";' for name in fragment_names)
module_styles_lines.append('}')
file_hash = hashlib.md5("".join(fragment_names).encode('ascii')).hexdigest()

contents['_module-styles.scss'] = '\n'.join(module_styles_lines)
contents[f"{class_}{suffix}.{file_hash}.scss"] = '\n'.join(module_styles_lines)

_write_files(output_root, contents)

Expand Down Expand Up @@ -305,9 +306,9 @@ def main():
root = path(args['<output_root>'])

descriptor_files = write_descriptor_js(root / 'descriptors/js')
write_descriptor_styles(root / 'descriptors/css')
write_descriptor_styles(root / 'descriptors/scss')
module_files = write_module_js(root / 'modules/js')
write_module_styles(root / 'modules/css')
write_module_styles(root / 'modules/scss')
write_webpack(root / 'webpack.xmodule.config.js', module_files, descriptor_files)


Expand Down
80 changes: 80 additions & 0 deletions xmodule/util/xmodule_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,89 @@
"""


from os import listdir

import webpack_loader
# NOTE: we are importing this method so that any module that imports us has access to get_current_request
from crum import get_current_request
from django.conf import settings
from webpack_loader.loader import WebpackLoader


class XModuleWebpackLoader(WebpackLoader):
"""
Custom webpack loader that consumes the output generated by webpack-bundle-tracker
and the files from XModule style generation stage. Briefly, this allows to use the
generated js bundles and compiled assets for XModule-style during Xblocks rendering.
"""

def load_assets(self):
"""
This function will append XModule css files to the standard load_assets results.
The standard WebpackLoader load_assets method returns a dictionary like the following:
{
'status': 'done',
'chunks': {
'AnnotatableBlockPreview': [
{
'name': 'AnnotatableBlockPreview.js',
'path': '/openedx/edx-platform/common/static/bundles/AnnotatableBlockPreview.js'
},
{
'name': 'AnnotatableBlockPreview.js.map',
'path': '/openedx/edx-platform/common/static/bundles/AnnotatableBlockPreview.js.map'
}
],
...
}
}
Chunks key contains the data for every file in /openedx/edx-platform/common/static/bundles/, that
is a folder created during the compilation theme, this method will append the listed files in
common/static/css/xmodule to the correspondent key, the result will be the following:
{
'status': 'done',
'chunks': {
'AnnotatableBlockPreview': [
{
'name': 'AnnotatableBlockPreview.js',
'path': '/openedx/edx-platform/common/static/bundles/AnnotatableBlockPreview.js'
},
{
'name': 'AnnotatableBlockPreview.js.map',
'path': '/openedx/edx-platform/common/static/bundles/AnnotatableBlockPreview.js.map'
},
{
'name': 'AnnotatableBlockPreview.85745121.css',
'path': 'common/static/css/xmodule/AnnotatableBlockPreview.85745121.css',
'publicPath': '/static/css/xmodule/AnnotatableBlockPreview.85745121.css'
}
],
...
}
}
Returns:
dict: Assets dictionary as described above.
"""
assets = super().load_assets()

css_path = "common/static/css/xmodule"
css_files = listdir(css_path)

for css_file in css_files:
name = css_file.split(".")[0]
css_chunk = {
"name": css_file,
"path": f"{css_path}/{css_file}",
"publicPath": f"{settings.STATIC_URL}css/xmodule/{css_file}",
}
assets["chunks"][name].append(css_chunk)

return assets


def get_current_request_hostname():
Expand Down

0 comments on commit c34f8ef

Please sign in to comment.