Skip to content

Commit

Permalink
feat: decople XModule style assets by using a custom webpack loader
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.

Addressing the issue openedx#31624
  • Loading branch information
andrey-canon committed Apr 27, 2023
1 parent 8ee1f66 commit 80c055c
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 18 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
2 changes: 1 addition & 1 deletion lms/static/sass/lms-course-rtl.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import 'bourbon/bourbon';
@import 'vendor/bi-app/bi-app-rtl'; // set the layout for right to left languages
@import 'bourbon/bourbon';
@import 'build-course'; // shared app style assets/rendering
2 changes: 1 addition & 1 deletion lms/static/sass/lms-course.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@import 'bourbon/bourbon';
@import 'vendor/bi-app/bi-app-ltr'; // set the layout for left to right languages
@import 'bourbon/bourbon';
@import 'build-course'; // shared app style assets/rendering

.dates-banner {
Expand Down
22 changes: 22 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,15 @@ 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",
sass_dir,
],
})

# now compile theme sass files for certificate
if system == 'lms':
dirs.append({
Expand All @@ -223,6 +234,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 +247,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
51 changes: 41 additions & 10 deletions xmodule/static_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@
import sys
import textwrap
from collections import defaultdict
from pkg_resources import resource_string

import django
from docopt import docopt
from path import Path as path

from pkg_resources import resource_string
from xmodule.annotatable_block import AnnotatableBlock
from xmodule.capa_block import ProblemBlock
from xmodule.conditional_block import ConditionalBlock
Expand Down Expand Up @@ -91,7 +90,23 @@ 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')
module_styles_lines = [
"@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';",
"@import 'base/mixins';",
]

return _write_styles(
'.xmodule_display',
output_root, XBLOCK_CLASSES,
'get_preview_view_css',
module_styles_lines,
'Preview'
)


def write_module_js(output_root):
Expand All @@ -101,7 +116,22 @@ 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')
descriptor_styles_lines = [
"@import 'bourbon/bourbon';",
"@import 'vendor/bi-app/bi-app-ltr';",
"@import 'lms/theme/variables';",
"@import 'cms/theme/variables-v1';",
"@import 'mixins';",
"@import 'mixins-inherited';",
]

return _write_styles(
'.xmodule_edit',
output_root, XBLOCK_CLASSES,
'get_studio_view_css',
descriptor_styles_lines,
'Studio'
)


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


def _write_styles(selector, output_root, classes, css_attribute):
def _write_styles(selector, output_root, classes, css_attribute, base_module_styles_lines, 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 +177,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 = base_module_styles_lines.copy()

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 +336,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 80c055c

Please sign in to comment.