From a0d2e5cb971e9d412cb8c0bfd650ce587480f998 Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Tue, 5 Jul 2022 16:06:39 +0530 Subject: [PATCH] docs: ADR that proposes a common stylesheet for all MFEs. Adds an ADR that proposes creating a single common stylesheet that all MFEs load from a central place. --- ...0007-compile-common-theme-for-all-mfes.rst | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 docs/decisions/0007-compile-common-theme-for-all-mfes.rst diff --git a/docs/decisions/0007-compile-common-theme-for-all-mfes.rst b/docs/decisions/0007-compile-common-theme-for-all-mfes.rst new file mode 100644 index 000000000..2e8fb8e98 --- /dev/null +++ b/docs/decisions/0007-compile-common-theme-for-all-mfes.rst @@ -0,0 +1,116 @@ +0007 Compile Common Theme for all MFEs +###################################### + + +Status +****** + +Draft + +Context +******* + +Currently each Open edX MFE includes a small snippet of SCSS code that is common +to all of them, which is the following four lines: + +.. code-block:: SCSS + + @import "~@edx/brand/paragon/fonts"; + @import "~@edx/brand/paragon/variables"; + @import "~@edx/paragon/scss/core/core"; + @import "~@edx/brand/paragon/overrides"; + +These four lines, import the Paragon and bootstrap code and apply +branding-specific overrides on top. They contain the bulk of the SCSS code +generated for MFEs. Following this are usually imports for the frontend header +and footer components, imports from the MFE own components and MFE-specific +SCSS. + +This effectively means that all MFEs share a large chunk of common styling code +with a comparatively much smaller set of styles applied over that. + +There is scope here to properly split the MFE stylesheet into multiple parts, +one being the common code as represented by the above imports, and another for +MFE-specific code, and build them separately. + +One thing that prevents this is that MFEs need these SCSS imports to get +variables. In a number of cases these can be replaced with either +Paragon/Bootstrap classes, or CSS variables. A separate ADR in `Paragon +`_ covers the situations where +this isn't possible yet. + +Decision +******** + +Common cores styling that is to be applied to all MFEs should be part of a +separate stylesheet and built separately from the MFE-specific stylesheet. + +Both these stylesheets will be included in each MFE, however the common +stylesheet can be loaded from a common source for all MFEs.This will allowing +for deploying the common theme stylesheet without redeploying the MFEs. + +The mechanism for loading this common stylesheet will need to be different from +the mechanism for loading the MFE stylesheet. A JavaScript theme loader will +contain the location of the SCSS file and will add this stylesheet to the +document. + +The common theme package will be similar to the branding package, but can be +deployed independently of MFEs. It will produce an CSS stylesheet, and a +JavaScript loader that will load this stylesheet. When deploying MFEs, they can +be provided a configuration variable that contains the location where this +theme loader is deployed. + +The frontend-platform repo can host code that simplifies using themes. It can +check if a theme loader is configured, and if so, dynamically load the theme +from there. Otherwise, it can continue using the existing mechanism of building +and deploying the theme for each MFE for backwards-compatibility. It can also +provide hooks for loading the theme and potentially/eventually also selecting +a theme. + +Consequences +************ + +With a single common stylesheet for all MFE, they should load quicker since the +stylesheet can be cached once for all MFEs. It also makes theme rebuilds much +quicker, and eliminates the need for rebuilding each MFE for minor change to +the theme. + +By continuing to support the existing theming mechanism, the current mechanism +for theming can continue working for those who need it. + +Since there is a JavaScript-based loader involved, there is potential for +flashing of unstyled content during loading, however this can be avoided by +making the theme loader part of the init process so it runs before the app is +considered ready. + +Rejected Alternatives +********************* + +- Loading the common CSS directly instead of with a JavaScript based loader + Adding a loader eliminates caching issues. Currently the CSS files include + the hash of the CSS code in the file name, so that with each redeployment the + file names change, ensuring that a file is cached as long as it's used but not + loaded any more once the file contents change. + + Since the common theme needs to have a fixed name, we can't rely on changing + the file name when the theme changes. If we disable caching for this file we + lose the benefits of caching and having a common theme. + + The CSS loader can be a very small JavaScript file that simply provides a + standard API to load the stylesheet, and we can ensure that this file is + loaded fresh on each reload so we always get the latest code. This loader will + know the location of the SCSS file, and as it does now it can be a file with + the hash in its name, so it's cached as long as it doesn't change. + +- Have the theme loading code in each MFE + This doesn't change much about the rest of the mechanism of this ADR, but it + causes a lot of code duplication across MFEs. Just like ``frontend-platform `` + handles authentication for all MFEs, it can also handle dealing with the + theme loader. + + +References +********** + +- `ADR to introduce CSS variables to parargon + `_