From 2769a00e3998a7e95346775125247ac560408889 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 14 Nov 2024 09:05:30 -0500 Subject: [PATCH] build: Remove unneccessary built-in XBlock Sass built steps (#35833) Since all built-in block Sass is gone, we remove two final pieces of code: * the steps in `npm run compile-sass` which compiled `xmodule/assets`, and * the now-unused `add_sass_to_fragment` function. This should speed up the edx-platform assets build a little bit. This commit also includes some minor dev doc updates. Part of: https://github.com/openedx/edx-platform/issues/35300 --- scripts/compile_sass.py | 52 ---------------------------------- xmodule/assets/README.rst | 28 ++++++------------ xmodule/util/builtin_assets.py | 35 +---------------------- 3 files changed, 10 insertions(+), 105 deletions(-) diff --git a/scripts/compile_sass.py b/scripts/compile_sass.py index ec1efee24d2b..14b84d003af6 100755 --- a/scripts/compile_sass.py +++ b/scripts/compile_sass.py @@ -352,18 +352,6 @@ def compile_sass_dir( repo / "lms" / "static" / "sass", ], ) - compile_sass_dir( - "Compiling built-in XBlock Sass for default LMS", - repo / "xmodule" / "assets", - repo / "lms" / "static" / "css", - includes=[ - *common_includes, - repo / "lms" / "static" / "sass" / "partials", - repo / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass", - repo / "cms" / "static" / "sass", - ], - ) if not skip_cms: compile_sass_dir( "Compiling default CMS Sass", @@ -376,18 +364,6 @@ def compile_sass_dir( repo / "cms" / "static" / "sass", ], ) - compile_sass_dir( - "Compiling built-in XBlock Sass for default CMS", - repo / "xmodule" / "assets", - repo / "cms" / "static" / "css", - includes=[ - *common_includes, - repo / "lms" / "static" / "sass" / "partials", - repo / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass", - repo / "cms" / "static" / "sass", - ], - ) click.secho(f"Done compiling default Sass!", fg="cyan", bold=True) click.echo() @@ -429,20 +405,6 @@ def compile_sass_dir( ], tolerate_missing=True, ) - compile_sass_dir( - "Compiling built-in XBlock Sass for themed LMS", - repo / "xmodule" / "assets", - theme / "lms" / "static" / "css", - includes=[ - *common_includes, - theme / "lms" / "static" / "sass" / "partials", - theme / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass" / "partials", - repo / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass", - repo / "cms" / "static" / "sass", - ], - ) if not skip_cms: compile_sass_dir( "Compiling default CMS Sass with themed partials", @@ -470,20 +432,6 @@ def compile_sass_dir( ], tolerate_missing=True, ) - compile_sass_dir( - "Compiling built-in XBlock Sass for themed CMS", - repo / "xmodule" / "assets", - theme / "cms" / "static" / "css", - includes=[ - *common_includes, - theme / "lms" / "static" / "sass" / "partials", - theme / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass" / "partials", - repo / "cms" / "static" / "sass" / "partials", - repo / "lms" / "static" / "sass", - repo / "cms" / "static" / "sass", - ], - ) click.secho(f"Done compiling Sass for theme at {theme}!", fg="cyan", bold=True) click.echo() diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst index a697915db835..feb72168f9de 100644 --- a/xmodule/assets/README.rst +++ b/xmodule/assets/README.rst @@ -27,25 +27,15 @@ However, we are proactively working towards a system where: Themable Sass (.scss) ********************* -XBlock CSS for ``student_view``, ``author_view``, and ``public_view`` is compiled from the various ``./BlockDisplay.scss`` modules, such as `AnnotatableBlockDisplay.scss`_. - -XBlock CSS for ``studio_view`` is compiled from the various ``./BlockEditor.scss`` modules, such as `AnnotatableBlockEditor.scss`_. - -Those Sass modules are mostly thin wrappers around the underscore-prefixed Sass -modules within block-type-subdirectories, such as `annotatable/_display.css`. In the -future, we may `simplify things`_ by collapsing the top-level Sass modules and -just directly compiling the block-type-subdirectories' Sass. - -The CSS is compiled into the static folders of both LMS and CMS, as well as into -the corresponding folders in any enabled themes, as part of the edx-platform build. -It is collected into the static root, and then linked to from XBlock fragments by the -``add_sass_to_fragment`` function in `builtin_assets.py`_. - -.. _AnnotatableBlockDisplay.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss -.. _AnnotatableBlockEditor.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss -.. _annotatable/_display.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/annotatable/_display.scss -.. _simplify things: https://github.com/openedx/edx-platform/issues/32621 - +Formerly, built-in XBlock CSS for ``student_view``, ``author_view``, and +``public_view`` was compiled from the various +``./BlockDisplay.scss`` modules, and ``studio_view`` CSS was +compiled from the various ``./BlockEditor.scss`` modules. + +As of November 2024, all that built-in XBlock Sass was been permanently +compiled into CSS, stored at ``../static/css-builtin-blocks/``. +The theme-overridable Sass variables are injected into CSS variables via +``../../common/static/sass/_builtin-block-variables.scss``. JavaScript (.js) **************** diff --git a/xmodule/util/builtin_assets.py b/xmodule/util/builtin_assets.py index 76e1455a1db3..cb1fca721414 100644 --- a/xmodule/util/builtin_assets.py +++ b/xmodule/util/builtin_assets.py @@ -32,7 +32,7 @@ def add_css_to_fragment(fragment, css_relative_path): if not css_absolute_path.is_file(): raise FileNotFoundError(f"css file not found: {css_absolute_path}") css_static_path = Path('css-builtin-blocks') / css_relative_path.with_suffix('.css') - css_url = get_static_file_url(str(css_static_path)) # get_static_file_url is theme-aware. + css_url = get_static_file_url(str(css_static_path)) if not css_url: raise ImproperlyConfigured( f"Did not find static CSS file {css_static_path} in staticfiles storage. Perhaps it wasn't collected?" @@ -40,39 +40,6 @@ def add_css_to_fragment(fragment, css_relative_path): fragment.add_css_url(css_url) -def add_sass_to_fragment(fragment, sass_relative_path): - """ - Given a Sass path relative to xmodule/assets, add a compiled CSS URL to the fragment. - - Raises: - * ValueError if {sass_relative_path} is absolute or does not end in '.scss'. - * FileNotFoundError if edx-platform/xmodule/assets/{sass_relative_path} is missing. - * ImproperlyConfigured if the lookup of the static CSS URL fails. This could happen - if Sass wasn't compiled, CSS wasn't collected, or the staticfiles app is misconfigured. - - Notes: - * This function is theme-aware. That is: If a theme is enabled which provides a compiled - CSS file of the same name, then that CSS file will be used instead. - """ - if not isinstance(sass_relative_path, Path): - sass_relative_path = Path(sass_relative_path) - if sass_relative_path.is_absolute(): - raise ValueError(f"sass_relative_path should be relative; is absolute: {sass_relative_path}") - if sass_relative_path.suffix != '.scss': - raise ValueError(f"sass_relative_path should be .scss file; is: {sass_relative_path}") - sass_absolute_path = Path(settings.REPO_ROOT) / "xmodule" / "assets" / sass_relative_path - if not sass_absolute_path.is_file(): - raise FileNotFoundError(f"Sass not found: {sass_absolute_path}") - css_static_path = Path('css') / sass_relative_path.with_suffix('.css') - css_url = get_static_file_url(str(css_static_path)) # get_static_file_url is theme-aware. - if not css_url: - raise ImproperlyConfigured( - f"Did not find CSS file {css_static_path} (compiled from {sass_absolute_path}) " - f"in staticfiles storage. Perhaps it wasn't collected?" - ) - fragment.add_css_url(css_url) - - def add_webpack_js_to_fragment(fragment, bundle_name): """ Add all JS webpack chunks to the supplied fragment.