Stop dynamically generating XModule SCSS #32292

kdmccormick · 2023-05-23T16:46:54Z

Background

See the parent issue: Remove special XModule asset handling #31624

Goal

All XModule SCSS should be committed to the repository rather than generated as part of the build process. Benefits:

Simplify the build process, partially unblocking us from removing Python from the build process.
Make XModule assets more similar to standard XBlock assets.
Make it so XModule SCSS can be read by a human being; no tooling or special context necessary.
Make the next step (stop dynamically generating XModule JS) easier & safer by getting SCSS out of the equation.

PRs

Give feedback

Sum of the changes

The PRs do a better job of describing each atomic change, but as whole, here's what's changing.

Before the first PR...

XModule CSS is:

defined by various SCSS files under xmodule/css;
synthesized together for each XModule under its studio_view_css and preview_view_css class variables;
processed by the xmodules_assets console script (defined in xmodule/static_content.py), which:
- for each XModule,
  - copies each file in the XModule's studio_view_css from
    - xmodule/css/<blah>/<yada>.scss to
    - common/static/xmodule/descriptors/css/_001-<md5-hash-of-contents-28f7a87e12f>.scss
  - generates an entrypoint SCSS file named like common/static/xmodule/descriptors/css/AnnotatableBlockStudio.be69909d83985d31e206fad272906958.scss, with contents like:
```
.xmodule_edit.xmodule_AboutBlock {
  @import "9bdcda00f046f78be79aca7791e1d4fb.scss";
  @import "a10fc3e0fd6aca63426a89e75fe69c31.scss";
}
```
  - copies each file in the XModule's preview_view_css from
    - xmodule/css/<blah>/<yada>.scss to
    - common/static/xmodule/modules/js/_001-<md5-hash-of-contents-28f7a87e12f>.scss
  - generates an entrypoint SCSS file named like common/static/xmodule/modules/css/AnnotatableBlockPreview.7e95b106aa0a61824f4290da1374960d.scss;
compiled from each SCSS entrypoint file a CSS file, eg, common/static/css/xmodule/AnnotatableBlockPreview.7e95b106aa0a61824f4290da1374960d.css, which are
included into Webpack via xmodule/util/xmodule_django.py:XModuleWebpackLoader; and
rendered to XModule via the xmodule/util/xmodule_django.py:add_webpack_to_fragment .

After the last PR...

XModule SCSS will be:

defined by various SCSS files under xmodule/static/sass/include;
synthesized together for each XModule by SCSS files named:
- xmodule/static/sass/cms/AnnotatableBlockStudio.scss (editing views)
- xmodule/static/sass/lms/AnnotatableBlockPreview.scss (display views)
compiled from xmodule/static/sass/(lms|cms)/*.scss to CSS files, eg, xmodule/static/css/AnnotatableBlockPreview.css, which are
included into Webpack via xmodule/util/xmodule_django.py:XModuleWebpackLoader (no change); and
rendered to XModule via the xmodule/util/xmodule_django.py:add_webpack_to_fragment (no change).

The text was updated successfully, but these errors were encountered:

…2286) For the XBlocks types that use legacy XModule-style assets (specifically, those that inherit from `HTMLSnippet`), this is small refactor that brings them a bit closer to being like standard XBlocks. Given these class attributes: class SomeXModuleLikeBlock(..., HTMLSnippet, ...): ... studio_view_css = { ... } preview_view_css = { ... } studio_view_js = { ... } preview_view_js = { ... } ... we make it so their values are *paths to the resources* rather than *the actual content of the resources*. This is a no-op change, but it'll enable future XModule asset refactorings which require us to operate on asset paths rather than contents. Part of: #32292

The `xmodule_assets` command copies SCSS files from xmodule/css to common/static/xmodule/{modules|descriptors}/scss. It renames the files to the format: _{INDEX}-{HASH}.scss where an XModule's first SCSS resource will have INDEX==0, the next will have INDEX==1, ...and that's it because no XModule has more than two SCSS resources. The output looks like this: common/static/xmodule/descriptors/scss: _000-808fcbb4c5109c5156ae3c0c9729c8be.scss ... _001-a10fc3e0fd6aca63426a89e75fe69c31.scss common/static/xmodule/modules/scss: _000-1ad2f05db822d3176affd203d70319c0.scss ... _001-482ebc752ab6e41946651ceb0f3e7f55.scss These indexes serve no purpose. Reading the comments and git-blame in xmodule/static_content.py, one can glean that the indexes might have been intended to enforce dependency relationships between the assets, but this is unnecessary, because the ordering of the copied SCSS is *already preserved* by the order which they're included into the `{BLOCK_NAME}{Studio|Preivew}.{HASH}.scss` SCSS entrypoint files. I have to assume that this is an unnecessary relic from the time when the XModule system was more heavily utilized, rather than just a legacy corner of the XBlock framework as it is today. So, we remove the indexes, which lets us simplify the logic of xmodule/static_content.py. This is a minor refactoring, but it'll make it easier for the next steps on our way to deleting xmodule/static_content.py entirely. The new output looks like this: common/static/xmodule/descriptors/scss: _808fcbb4c5109c5156ae3c0c9729c8be.scss ... _d41921b4c5d45188759ef3d04fd9a78a.scss common/static/xmodule/modules/scss: _1ad2f05db822d3176affd203d70319c0.scss ... _b80300e1a5f290f6a850e35874068427.scss Part of: #32292

Similar to openedx#32287, this change removes another unnecessary step from the `xmodule_assets` script. The script, which generates XModule SCSS "entrypoint" files (synthesizing one or more "source" SCSS resources), was appending MD5 hashes to each SCSS entrypoint filename: common/static/xmodule/descriptors/scss: AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss ... WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss common/static/xmodule/modules/scss: AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss ... WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss It's unclear why these MD5 hashes needed to be appended. A comment in xmodule/static_content.py hints that it might have something to do with de-duplication, but that doesn't make any sense, because each XModule has exactly two SCSS entrypoint files (one for studio_view and one for other student/author_views) and none of those entrypoint files can possibly be shared between XModules. Soon, as part of deleting the `xmodule_assets` script, we would like to just check these SCSS files into version control rather than generating them. In order to do that, we will need to drop the hashes. This commit does that. The new output looks like this: common/static/xmodule/descriptors: AboutBlockStudio.scss ... WordCloudBlockStudio.scss common/static/xmodule/modules: AboutBlockPreview.scss ... WordCloudBlockPreview.scss Part of: openedx#32292

Similar to #32287, this change removes another unnecessary step from the `xmodule_assets` script. The script, which generates XModule SCSS "entrypoint" files (synthesizing one or more "source" SCSS resources), was appending MD5 hashes to each SCSS entrypoint filename: common/static/xmodule/descriptors/scss: AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss ... WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss common/static/xmodule/modules/scss: AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss ... WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss It's unclear why these MD5 hashes needed to be appended. A comment in xmodule/static_content.py hints that it might have something to do with de-duplication, but that doesn't make any sense, because each XModule has exactly two SCSS entrypoint files (one for studio_view and one for other student/author_views) and none of those entrypoint files can possibly be shared between XModules. Soon, as part of deleting the `xmodule_assets` script, we would like to just check these SCSS files into version control rather than generating them. In order to do that, we will need to drop the hashes. This commit does that. The new output looks like this: common/static/xmodule/descriptors: AboutBlockStudio.scss ... WordCloudBlockStudio.scss common/static/xmodule/modules: AboutBlockPreview.scss ... WordCloudBlockPreview.scss Part of: #32292

The `xmodule_assets` command copies SCSS files from xmodule/css to common/static/xmodule/{modules|descriptors}/scss. It renames the files to the format: _{INDEX}-{HASH}.scss where an XModule's first SCSS resource will have INDEX==0, the next will have INDEX==1, ...and that's it because no XModule has more than two SCSS resources. The output looks like this: common/static/xmodule/descriptors/scss: _000-808fcbb4c5109c5156ae3c0c9729c8be.scss ... _001-a10fc3e0fd6aca63426a89e75fe69c31.scss common/static/xmodule/modules/scss: _000-1ad2f05db822d3176affd203d70319c0.scss ... _001-482ebc752ab6e41946651ceb0f3e7f55.scss These indexes serve no purpose. Reading the comments and git-blame in xmodule/static_content.py, one can glean that the indexes might have been intended to enforce dependency relationships between the assets, but this is unnecessary, because the ordering of the copied SCSS is *already preserved* by the order which they're included into the `{BLOCK_NAME}{Studio|Preivew}.{HASH}.scss` SCSS entrypoint files. I have to assume that this is an unnecessary relic from the time when the XModule system was more heavily utilized, rather than just a legacy corner of the XBlock framework as it is today. So, we remove the indexes, which lets us simplify the logic of xmodule/static_content.py. This is a minor refactoring, but it'll make it easier for the next steps on our way to deleting xmodule/static_content.py entirely. The new output looks like this: common/static/xmodule/descriptors/scss: _808fcbb4c5109c5156ae3c0c9729c8be.scss ... _d41921b4c5d45188759ef3d04fd9a78a.scss common/static/xmodule/modules/scss: _1ad2f05db822d3176affd203d70319c0.scss ... _b80300e1a5f290f6a850e35874068427.scss Part of: openedx#32292

Similar to openedx#32287, this change removes another unnecessary step from the `xmodule_assets` script. The script, which generates XModule SCSS "entrypoint" files (synthesizing one or more "source" SCSS resources), was appending MD5 hashes to each SCSS entrypoint filename: common/static/xmodule/descriptors/scss: AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss ... WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss common/static/xmodule/modules/scss: AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss ... WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss It's unclear why these MD5 hashes needed to be appended. A comment in xmodule/static_content.py hints that it might have something to do with de-duplication, but that doesn't make any sense, because each XModule has exactly two SCSS entrypoint files (one for studio_view and one for other student/author_views) and none of those entrypoint files can possibly be shared between XModules. Soon, as part of deleting the `xmodule_assets` script, we would like to just check these SCSS files into version control rather than generating them. In order to do that, we will need to drop the hashes. This commit does that. The new output looks like this: common/static/xmodule/descriptors: AboutBlockStudio.scss ... WordCloudBlockStudio.scss common/static/xmodule/modules: AboutBlockPreview.scss ... WordCloudBlockPreview.scss Part of: openedx#32292

The `xmodule_assets` command copies SCSS files from xmodule/css to common/static/xmodule/{modules|descriptors}/scss. It renames the files to the format: _{INDEX}-{HASH}.scss where an XModule's first SCSS resource will have INDEX==0, the next will have INDEX==1, ...and that's it because no XModule has more than two SCSS resources. The output looks like this: common/static/xmodule/descriptors/scss: _000-808fcbb4c5109c5156ae3c0c9729c8be.scss ... _001-a10fc3e0fd6aca63426a89e75fe69c31.scss common/static/xmodule/modules/scss: _000-1ad2f05db822d3176affd203d70319c0.scss ... _001-482ebc752ab6e41946651ceb0f3e7f55.scss These indexes serve no purpose. Reading the comments and git-blame in xmodule/static_content.py, one can glean that the indexes might have been intended to enforce dependency relationships between the assets, but this is unnecessary, because the ordering of the copied SCSS is *already preserved* by the order which they're included into the `{BLOCK_NAME}{Studio|Preivew}.{HASH}.scss` SCSS entrypoint files. I have to assume that this is an unnecessary relic from the time when the XModule system was more heavily utilized, rather than just a legacy corner of the XBlock framework as it is today. So, we remove the indexes, which lets us simplify the logic of xmodule/static_content.py. This is a minor refactoring, but it'll make it easier for the next steps on our way to deleting xmodule/static_content.py entirely. The new output looks like this: common/static/xmodule/descriptors/scss: _808fcbb4c5109c5156ae3c0c9729c8be.scss ... _d41921b4c5d45188759ef3d04fd9a78a.scss common/static/xmodule/modules/scss: _1ad2f05db822d3176affd203d70319c0.scss ... _b80300e1a5f290f6a850e35874068427.scss Part of: openedx#32292

Similar to openedx#32287, this change removes another unnecessary step from the `xmodule_assets` script. The script, which generates XModule SCSS "entrypoint" files (synthesizing one or more "source" SCSS resources), was appending MD5 hashes to each SCSS entrypoint filename: common/static/xmodule/descriptors/scss: AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss ... WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss common/static/xmodule/modules/scss: AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss ... WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss It's unclear why these MD5 hashes needed to be appended. A comment in xmodule/static_content.py hints that it might have something to do with de-duplication, but that doesn't make any sense, because each XModule has exactly two SCSS entrypoint files (one for studio_view and one for other student/author_views) and none of those entrypoint files can possibly be shared between XModules. Soon, as part of deleting the `xmodule_assets` script, we would like to just check these SCSS files into version control rather than generating them. In order to do that, we will need to drop the hashes. This commit does that. The new output looks like this: common/static/xmodule/descriptors: AboutBlockStudio.scss ... WordCloudBlockStudio.scss common/static/xmodule/modules: AboutBlockPreview.scss ... WordCloudBlockPreview.scss Part of: openedx#32292

The `xmodule_assets` command copies SCSS source files from xmodule/css to common/static/xmodule/scss, renaming them to `{MD5_HASH}.scss` in order to "remove duplicates". The copied files are then included into the generated SCSS entrypoint files (eg AnnotatableBlockStudio.scss). The "de-deplication" is completely unnecessary: there are only a couple dozen SCSS files, and none of them are duplicates. This copying process is confusing, it complicates our build process, and it makes our SCSS harder to understand. So, in the generated SCSS entrypoint files, we stop importing the *copied* SCSS sources, and just import the *original* SCSS sources instead. For example, common/static/xmodule/descriptors/scss/AboutBlockStudio.scss is changed from: .xmodule_edit.xmodule_AboutBlock { @import "9bdcda00f046f78be79aca7791e1d4fb.scss"; @import "a10fc3e0fd6aca63426a89e75fe69c31.scss"; } to: .xmodule_edit.xmodule_AboutBlock { @import "editor/edit.scss"; @import "html/edit.scss"; } In order to make the `@import` lines work, we add xmodule/css to the list of lookup dirs for XModule SCSS compilation. We also remove the copying logic from `xmodule_assets`, as it is no longer needed. Part of: openedx#32292

The `xmodule_assets` command copies SCSS source files from xmodule/css to common/static/xmodule/scss, renaming them to `{MD5_HASH}.scss` in order to "remove duplicates". The copied files are then included into the generated SCSS entrypoint files (eg AnnotatableBlockStudio.scss). The "de-deplication" is completely unnecessary: there are only a couple dozen SCSS files, and none of them are duplicates. This copying process is confusing, it complicates our build process, and it makes our SCSS harder to understand. So, in the generated SCSS entrypoint files, we stop importing the *copied* SCSS sources, and just import the *original* SCSS sources instead. For example, common/static/xmodule/descriptors/scss/AboutBlockStudio.scss is changed from: .xmodule_edit.xmodule_AboutBlock { @import "9bdcda00f046f78be79aca7791e1d4fb.scss"; @import "a10fc3e0fd6aca63426a89e75fe69c31.scss"; } to: .xmodule_edit.xmodule_AboutBlock { @import "editor/edit.scss"; @import "html/edit.scss"; } In order to make the `@import` lines work, we add xmodule/css to the list of lookup dirs for XModule SCSS compilation. We also remove the copying logic from `xmodule_assets`, as it is no longer needed. Part of: #32292

Now that all XModule SCSS is located in xmodule/static/sass, it would make sense to co-locate the CSS there as well. We also add a README to explain the purpose of this new folder. In the future, we will move xmodule/js and xmodule/assets into xmodule/static as well. Part of: openedx#32292

Now that all XModule SCSS is located in xmodule/static/sass, it would make sense to co-locate the CSS there as well. We also add a README to explain the purpose of this new folder. In the future, we will move xmodule/js and xmodule/assets into xmodule/static as well. Part of: #32292

kdmccormick · 2023-06-21T17:24:13Z

This is done!

…" (#32526) This reverts commit 5671dab. The original PR is causing styling issues due to broken CSS references on studio.edx.org. See #32292 for follow-up.

In ~Palm and earlier, all built-in XBlock Sass was included into LMS and CMS styles before being compiled. The generated CSS was coupled together with broader LMS/CMS CSS. This means that comprehensive themes have been able to modify built-in XBlock appearance by setting certain Sass variables. We say that built-in XBlock Sass was, and is expected to be, "theme-aware". Shortly after Palm, we decoupled XBlock Sass from LMS and CMS Sass [1]. Each built-in block's Sass is now compiled into two separate CSS targets, one for block editing and one for block display. The CSS, now located at `common/static/css/xmodule`, is injected into the running Webpack context with the new `XModuleWebpackLoader`. Built-in XBlocks already used `add_webpack_to_fragment` in order to add JS Webpack bundles to their view fragments, so when CSS was added to Webpack, it Just Worked. This unlocked a slieu of simplifications for static asset processing [2]; however, it accidentally made XBlock Sass theme-*unaware*, or perhaps theme-confused, since the CSS was targeted at `common/static/css/xmodule` regardless of the theme. The result of this is that **built-in XBlock views will use CSS based on the Sass variables _last theme to be compiled._** Sass variables are only used in a handful of places in XBlocks, so the bug is subtle, but it is there for those running off of master. For example, using edX.org's theme on master, we can see that there is a default blue underline in the Studio sequence nav [3]. With this bugfix, it becomes the standard edX.org greenish-black [4]. This commit makes several changes, firstly to fix the bug, and secondly to leave ourselves with a more comprehensible asset setup in the `xmodule/` directory. * We remove the `XModuleWebpackLoader`, thus taking built-in XBlock Sass back out of Webpack. * We compile XBlock Sass not to `common/static/css/xmodule`, but to: * `[lms|cms]/static/css` for the default theme, and * `<THEME_ROOT>/[lms|cms]/static/css`, for any custom theme. This is where the comprehensive theming system expects to find themable assets. Unfortunately, this does mean that the Sass is compiled twice, both for LMS and CMS. We would have liked to compile it once to somewhere in the `common/`, but comprehensive theming does not consider `common/` assets to be themable. * We split `add_webpack_to_fragment` into two more specialized functions: * `add_webpack_js_to_fragment` , for adding *just* JS from a Webpack bundle, and * `add_sass_to_fragment`, for adding static links to CSS compiled themable Sass (not Webpack). Both these functions are moved to a new module `xmodule/util/builtin_assets.py`, since the original module (`xmodule/util/xmodule_django.py`) didn't make a ton of sense. * In an orthogonal bugfix, we merge Sass `CourseInfoBlock`, `StaticTabBlock`, `AboutBlock` into the `HtmlBlock` Sass files. The first three were never used, as their styling was handled by `HtmlBlock` (their shared parent class). * As a refactoring, we change Webpack bundle names and Sass module names to be less misleading: * student_view, public_view, and author_view: was `<Name>BlockPreview`, is now `<Name>BlockDisplay`. * studio_view: was `<Name>BlockStudio`, is now `<Name>BlockEditor`. * As a refactoring, we move the contents of `xmodule/static` into the existing `xmodule/assets` directory, and adopt its simper structure. We now have: * `xmodule/assets/*.scss`: Top-level compiled Sass modules. These could be collapsed away in a future refactoring. * `xmodule/assets/<blocktype>/*`: Resources for each block, including both JS modules and Sass includes (underscore-prefixed so that they aren't compiled). This structure maps closely with what externally-defined XBlocks do. * `xmodule/js` still exists, but it will soon be folded into the `xmodule/assets`. * We add a new README [4] to explain the new structure, and also update a docstring in `openedx/lib/xblock/utils` which had fallen out of date with reality. * Side note: We avoid the term "XModule" in all of this, because that's (thankfully) become a much less useful/accurate way to describe these blocks. Instead, we say "built-in XBlocks". Refs: 1. openedx#32018 2. openedx#32292 3. https://github.com/openedx/edx-platform/assets/3628148/8b44545d-0f71-4357-9385-69d6e1cca86f 4. https://github.com/openedx/edx-platform/assets/3628148/d0b7b309-b8a4-4697-920a-8a520e903e06 5. https://github.com/openedx/edx-platform/tree/master/xmodule/assets#readme Part of: openedx#32292

In ~Palm and earlier, all built-in XBlock Sass was included into CMS (and LMS) styles before being compiled. So, if a site theme was meant to affect built-in XBlock styling, those changes would be manifested directly in the base CMS CSS that is included into every single Studio page. When the user provided the `?site_theme` querystring parameter, which is intended to allow devs & admins to view Studio through a given theme, CMS would look up the given theme and serve the corresponding base CMS CSS, which would affect the built-in XBlocks views (as expected). After ~Palm, built-in XBlocks styles are handled more similarly to to pure XBlock styles, in that they are only requested when CMS tries to render the block. In Studio, blocks are not rendered by the original request, but by a subsequent AJAX request to the `/container_preview` enpoint. Thus, passing the `?site_theme` query parameter to the original request will apply the given theme to Studio's chrome, but the theme will _not_ apply to built-in XBlock views, whose CSS is now loaded via async request. To fix this, we simply pass Studio's querystring parameters (including `?site_theme`) along to the `/container_view` AJAX request. This will cause CMS to correctly serve the built-in XBlock CSS from the theme specified by `?site_theme`, rather than whatever the current theme is. Part of: openedx#32292

In ~Palm and earlier, all built-in XBlock Sass was included into LMS and CMS styles before being compiled. The generated CSS was coupled together with broader LMS/CMS CSS. This means that comprehensive themes have been able to modify built-in XBlock appearance by setting certain Sass variables. We say that built-in XBlock Sass was, and is expected to be, "theme-aware". Shortly after Palm, we decoupled XBlock Sass from LMS and CMS Sass [1]. Each built-in block's Sass is now compiled into two separate CSS targets, one for block editing and one for block display. The CSS, now located at `common/static/css/xmodule`, is injected into the running Webpack context with the new `XModuleWebpackLoader`. Built-in XBlocks already used `add_webpack_to_fragment` in order to add JS Webpack bundles to their view fragments, so when CSS was added to Webpack, it Just Worked. This unlocked a slieu of simplifications for static asset processing [2]; however, it accidentally made XBlock Sass theme-*unaware*, or perhaps theme-confused, since the CSS was targeted at `common/static/css/xmodule` regardless of the theme. The result of this is that **built-in XBlock views will use CSS based on the Sass variables _last theme to be compiled._** Sass variables are only used in a handful of places in XBlocks, so the bug is subtle, but it is there for those running off of master. For example, using edX.org's theme on master, we can see that there is a default blue underline in the Studio sequence nav [3]. With this bugfix, it becomes the standard edX.org greenish-black [4]. This commit makes several changes, firstly to fix the bug, and secondly to leave ourselves with a more comprehensible asset setup in the `xmodule/` directory. * We remove the `XModuleWebpackLoader`, thus taking built-in XBlock Sass back out of Webpack. * We compile XBlock Sass not to `common/static/css/xmodule`, but to: * `[lms|cms]/static/css` for the default theme, and * `<THEME_ROOT>/[lms|cms]/static/css`, for any custom theme. This is where the comprehensive theming system expects to find themable assets. Unfortunately, this does mean that the Sass is compiled twice, both for LMS and CMS. We would have liked to compile it once to somewhere in the `common/`, but comprehensive theming does not consider `common/` assets to be themable. * We split `add_webpack_to_fragment` into two more specialized functions: * `add_webpack_js_to_fragment` , for adding *just* JS from a Webpack bundle, and * `add_sass_to_fragment`, for adding static links to CSS compiled themable Sass (not Webpack). Both these functions are moved to a new module `xmodule/util/builtin_assets.py`, since the original module (`xmodule/util/xmodule_django.py`) didn't make a ton of sense. * In an orthogonal bugfix, we merge Sass `CourseInfoBlock`, `StaticTabBlock`, `AboutBlock` into the `HtmlBlock` Sass files. The first three were never used, as their styling was handled by `HtmlBlock` (their shared parent class). * As a refactoring, we change Webpack bundle names and Sass module names to be less misleading: * student_view, public_view, and author_view: was `<Name>BlockPreview`, is now `<Name>BlockDisplay`. * studio_view: was `<Name>BlockStudio`, is now `<Name>BlockEditor`. * As a refactoring, we move the contents of `xmodule/static` into the existing `xmodule/assets` directory, and adopt its simper structure. We now have: * `xmodule/assets/*.scss`: Top-level compiled Sass modules. These could be collapsed away in a future refactoring. * `xmodule/assets/<blocktype>/*`: Resources for each block, including both JS modules and Sass includes (underscore-prefixed so that they aren't compiled). This structure maps closely with what externally-defined XBlocks do. * `xmodule/js` still exists, but it will soon be folded into the `xmodule/assets`. * We add a new README [4] to explain the new structure, and also update a docstring in `openedx/lib/xblock/utils` which had fallen out of date with reality. * Side note: We avoid the term "XModule" in all of this, because that's (thankfully) become a much less useful/accurate way to describe these blocks. Instead, we say "built-in XBlocks". Refs: 1. #32018 2. #32292 3. https://github.com/openedx/edx-platform/assets/3628148/8b44545d-0f71-4357-9385-69d6e1cca86f 4. https://github.com/openedx/edx-platform/assets/3628148/d0b7b309-b8a4-4697-920a-8a520e903e06 5. https://github.com/openedx/edx-platform/tree/master/xmodule/assets#readme Part of: #32292

In ~Palm and earlier, all built-in XBlock Sass was included into CMS (and LMS) styles before being compiled. So, if a site theme was meant to affect built-in XBlock styling, those changes would be manifested directly in the base CMS CSS that is included into every single Studio page. When the user provided the `?site_theme` querystring parameter, which is intended to allow devs & admins to view Studio through a given theme, CMS would look up the given theme and serve the corresponding base CMS CSS, which would affect the built-in XBlocks views (as expected). After ~Palm, built-in XBlocks styles are handled more similarly to to pure XBlock styles, in that they are only requested when CMS tries to render the block. In Studio, blocks are not rendered by the original request, but by a subsequent AJAX request to the `/container_preview` enpoint. Thus, passing the `?site_theme` query parameter to the original request will apply the given theme to Studio's chrome, but the theme will _not_ apply to built-in XBlock views, whose CSS is now loaded via async request. To fix this, we simply pass Studio's querystring parameters (including `?site_theme`) along to the `/container_view` AJAX request. This will cause CMS to correctly serve the built-in XBlock CSS from the theme specified by `?site_theme`, rather than whatever the current theme is. Part of: #32292

kdmccormick · 2023-07-06T18:06:32Z

Closing, now that #32592 is merged.

kdmccormick self-assigned this May 23, 2023

kdmccormick changed the title ~~Stop dynamic generating XModule SCSS~~ Stop dynamically generating XModule SCSS May 23, 2023

kdmccormick closed this as completed Jun 21, 2023

kdmccormick reopened this Jun 22, 2023

kdmccormick mentioned this issue Jun 28, 2023

XModule Style Decoupling: Follow-up fixes & refactoring #32592

Merged

kdmccormick closed this as completed Jul 6, 2023

kdmccormick mentioned this issue Jul 7, 2023

Build edx-platform assets without Python #31800

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop dynamically generating XModule SCSS #32292

Stop dynamically generating XModule SCSS #32292

kdmccormick commented May 23, 2023 •

edited

Loading

PRs

kdmccormick commented Jun 21, 2023

kdmccormick commented Jul 6, 2023

Stop dynamically generating XModule SCSS #32292

Stop dynamically generating XModule SCSS #32292

Comments

kdmccormick commented May 23, 2023 • edited Loading

Background

Goal

PRs

Sum of the changes

Before the first PR...

After the last PR...

kdmccormick commented Jun 21, 2023

kdmccormick commented Jul 6, 2023

kdmccormick commented May 23, 2023 •

edited

Loading