Skip to content

Commit

Permalink
Revert "build: Decouple XModule styles from LMS/Studio styles (#32018)"
Browse files Browse the repository at this point in the history
This reverts commit 471ba91.
  • Loading branch information
connorhaugh authored May 4, 2023
1 parent 471ba91 commit aea3b4b
Show file tree
Hide file tree
Showing 18 changed files with 14 additions and 166 deletions.
3 changes: 1 addition & 2 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1439,8 +1439,7 @@
WEBPACK_LOADER = {
'DEFAULT': {
'BUNDLE_DIR_NAME': 'bundles/',
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json'),
'LOADER_CLASS': 'xmodule.util.xmodule_django.XModuleWebpackLoader',
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
},
'WORKERS': {
'BUNDLE_DIR_NAME': 'bundles/',
Expand Down
2 changes: 2 additions & 0 deletions cms/static/sass/_build-v1.scss
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@

// +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: 1 addition & 2 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2696,8 +2696,7 @@ 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'),
'LOADER_CLASS': 'xmodule.util.xmodule_django.XModuleWebpackLoader',
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-stats.json')
},
'WORKERS': {
'BUNDLE_DIR_NAME': 'bundles/',
Expand Down
2 changes: 2 additions & 0 deletions lms/static/sass/_build-course.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
@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 @@ -56,6 +57,7 @@
@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
22 changes: 0 additions & 22 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,6 @@ 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 @@ -199,15 +197,6 @@ 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 @@ -234,8 +223,6 @@ 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 @@ -247,15 +234,6 @@ 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: 0 additions & 4 deletions xmodule/css/annotatable/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
* 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: 0 additions & 3 deletions xmodule/css/capa/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@
// * +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: 0 additions & 6 deletions xmodule/css/editor/edit.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
@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: 0 additions & 3 deletions xmodule/css/html/display.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
@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: 0 additions & 6 deletions xmodule/css/lti/lti.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
@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: 0 additions & 5 deletions xmodule/css/poll/display.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
@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: 0 additions & 7 deletions xmodule/css/sequence/display.scss
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
@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: 0 additions & 5 deletions xmodule/css/tabs/tabs.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
// 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: 0 additions & 2 deletions xmodule/css/video/accessible_menu.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@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: 0 additions & 5 deletions xmodule/css/video/display.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,7 @@
// --------
// 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: 0 additions & 5 deletions xmodule/css/word_cloud/display.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
@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: 8 additions & 9 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', 'Preview')
return _write_styles('.xmodule_display', output_root, XBLOCK_CLASSES, 'get_preview_view_css')


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', 'Studio')
return _write_styles('.xmodule_edit', output_root, XBLOCK_CLASSES, 'get_studio_view_css')


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

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

for class_, fragment_names in sorted(css_imports.items()):
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[f"{class_}{suffix}.{file_hash}.scss"] = '\n'.join(module_styles_lines)
contents['_module-styles.scss'] = '\n'.join(module_styles_lines)

_write_files(output_root, contents)

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

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


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


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 aea3b4b

Please sign in to comment.