feat: adds support for loading external theme CSS for MFEs

[adamstankiewicz]

Description:

Implementation POC/prototype for ADR on loading a common, external stylesheet for MFEs.

• Introduces useParagonTheme in AppProvider to load/inject <link> elements for the core theme CSS and any theme variant CSS into the HTML document.
• Exposes the app theme state and a way for applications to expose functionality to toggle between theme variants to consumers via AppContext.
• Supports a dark theme variant (even though Paragon itself does not yet have a dark theme variant).
• Supports a few mechanisms for determining which theme variant to make "active" and visible to the user:
  • When an application sets a theme variant programmatically (e.g., by exposing a button to the user to switch between light and dark modes), the selected theme variant is stored in localStorage so it persists/loads across sessions and page refreshes.
  • When no theme variant is selected in localStorage, checks if the user's System Preferences has a preference for dark mode via a @media query for prefers-color-scheme: dark. If dark color scheme is preferred, the default configured dark theme variant is made visible, if one exists.
    • When the prefers-color-scheme: dark query changes (i.e., user updates their System Preferences), loads the appropriate default theme variant corresponding to the system preference in real time.
  • When either the dark mode theme variant doesn't exist or the default light theme is actively visible, finds the default configured light theme variant.

See theming.md for documentation and more details.

Merge checklist:

• [ ] Consider running your code modifications in the included example app within frontend-platform. This can be done by running npm start and opening http://localhost:8080.
• [ ] Consider testing your code modifications in another local micro-frontend using local aliases configured via the module.config.js file in frontend-build.
• [ ] Verify your commit title/body conforms to the conventional commits format (e.g., fix, feat) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.

Post merge:

• [ ] After the build finishes for the merged commit, verify the new release has been pushed to NPM.

[codecov[bot]]

Codecov Report

Attention: 107 lines in your changes are missing coverage. Please review.

Comparison is base (3a93d60) 83.22% compared to head (cf10278) 79.05%.

:exclamation: Current head cf10278 differs from pull request most recent head 3345dc0. Consider uploading reports for the commit 3345dc0 to get more accurate results

Files	Patch %	Lines
src/react/hooks/paragon/useParagonTheme.js	3.12%	23 Missing and 8 partials :warning:
src/react/hooks/paragon/useParagonThemeVariants.js	78.64%	20 Missing and 2 partials :warning:
src/react/hooks/paragon/utils.js	24.13%	16 Missing and 6 partials :warning:
src/react/hooks/paragon/useParagonThemeCore.js	82.19%	12 Missing and 1 partial :warning:
src/react/reducers.js	23.07%	9 Missing and 1 partial :warning:
src/react/hooks/paragon/useParagonThemeUrls.js	73.33%	6 Missing and 2 partials :warning:
src/react/hooks/useAppEvent.js	80.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #440      +/-   ##
==========================================
- Coverage   83.22%   79.05%   -4.18%     
==========================================
  Files          40       48       +8     
  Lines        1073     1351     +278     
  Branches      197      283      +86     
==========================================
+ Hits          893     1068     +175     
- Misses        168      251      +83     
- Partials       12       32      +20     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[regisb]

I don't know enough about MFEs to comment about the details of this change, but what I understand is that it will require just two settings to customise the CSS, which is great.

One thing, though: this new extension mechanism will only be as good as its documentation. So I urge you to spend some time to document it as best you can. Actually, I suggest that you write the docs before finalizing the implementation, in README-driven-development style. For instance, you should describe about Tutor users can create and integrate new themes.

Reading the docs will allow us to comment on this PR in a much more informed way. Also, writing the docs will help you realize whether the new extension mechanism integrates well with the workflows of platform administrators and Open edX developers.

[ghassanmas]

I have a couple of question,

• If loading the theme/style from cdn, would that be enough for the app (to run UX/UI wise) without including the default app.*css file generated on each bundle. i.e. does the theme has enough css or just stuff to override? I am referring exactly this file

• Will each MFE has to own file or one file is enough for all the MFEs for example app learning has its own scss file https://github.com/openedx/frontend-app-learning/blob/master/src/index.scss

[arbrandes]

@adamstankiewicz, some stuff that came up out of a high-level discussion with tCRIL engineering:

• Support and longevity: we should consider the case where somebody is still running Palm after 5 years. The required stylesheets must still be hosted somewhere, and the implication is also that they will have to be versioned.
• While we can envision the advantages of having themes be built separately and thus potentially reusable across versions, maybe this should be an optional behavior: say, if APP_THEME_CORE_URL is set, the stylesheet is fetched accordingly, but if not, then the app would expect to find it in the webpack bundle. (This also addresses the issue of longevity.)
• Performance: there's nothing to say this proposal wouldn't be performant if done well, but we should keep UX in mind. With the dynamic config API currently in place, it could mean making the user wait for two requests in sequence (not counting the very first GET), any of which could fail: first, for the configuration itself, and after this one is complete, for the stylesheets. And this is not some theoretical possibility: it would be the default behavior for most Tutor deployments.

These aren't necessarily formal objections by tCRIL, by the way. Just adding some stuff to keep in mind. :)

[adamstankiewicz]

@arbrandes Thanks for sharing the input!

Support and longevity: we should consider the case where somebody is still running Palm after 5 years. The required stylesheets must still be hosted somewhere, and the implication is also that they will have to be versioned.

If we did end up using/recommending jsDelivr as an off-the-shelf free CDN supporting versions already aligned with NPM/Github releases, they guarantee any file from any version of the NPM package will remain accessible forever. They even support version fallback (say, a file is deleted in v1.0.2 that was previously there in v1.0.1, if you try to access that now-deleted file on v1.0.2, jsDelivr is smart enough to fallback to the version of the file in v1.0.1.

While we can envision the advantages of having themes be built separately and thus potentially reusable across versions, maybe this should be an optional behavior: say, if APP_THEME_CORE_URL is set, the stylesheet is fetched accordingly, but if not, then the app would expect to find it in the webpack bundle. (This also addresses the issue of longevity.)

I was actually just chatting with @brian-smith-tcril about this a bit, too 😄 generally, though, +1 to the input and I'd extend it to say that we might be able to fallback to the installed CSS within the webpack bundle should the external CSS fail to load for whatever reason so at least the app renders with some styles, even if those styles may be from a slightly older version than what might be upstream in the CDN, for example.

I believe this latter bit around falling back to locally installed CSS addresses at the third feedback item as well regarding performance? If the external CSS files are on a CDN(e.g., jsDelivr), the first few requests to a particular version of the theme may take a bit longer but subsequent requests to the same versions/files in the future would be cached and nearly instantaneous to load. If it errors, falling back to slightly out of date CSS would at least still make the application functional.

See https://www.jsdelivr.com/documentation for more details on how jsDelivr is production-ready (e.g., if Cloudflare goes down, jsDelivr instantly reverts to another CDN provider instead, has support for China, etc.).

[adamstankiewicz]

@ghassanmas's questions above were largely answered in the Frontend Working Group meeting earlier today. To recap though:

If loading the theme/style from cdn, would that be enough for the app (to run UX/UI wise) without including the default app.*css file generated on each bundle. i.e. does the theme has enough css or just stuff to override? I am referring https://github.com/openedx/frontend-build/pull/266#issuecomment-1428690893

The core theme CSS (core.css) intends to be an all-encompassing theme containing all of the Paragon styles necessary the foundational parts of the Open edX theme including layout, typography, component styles, etc. The light theme variant CSS (light.css) - in the future, possibly an additional dark.css for dark mode support - would define just the CSS variables necessary for the light theme variant (the default). These CSS variables are used within core.css.

Assuming applications don't have their own custom styles, applications indeed could not have any custom SCSS/CSS files at all, only relying on the external theme CSS.

If applications do have their own custom styles (hopefully not overriding Paragon styles!), @edx/frontend-build (via Webpack) will bundle the application's CSS to .css file(s) and inject it after the external theme CSS.

By doing so, the application .css has higher priority over the theme CSS files and will (generally) take precedence.

Will each MFE has to own file or one file is enough for all the MFEs for example app learning has its own scss file https://github.com/openedx/frontend-app-learning/blob/master/src/index.scss

The externally hosted, already-compiled theme CSS will need to versioning according to semantic versioning and aligned to NPM/Github releases. In our current MFE architecture, MFEs would use the latest release for the major version compatible with the installed @edx/paragon version in the MFE. For example, if the frontend-app-profile MFE has @edx/paragon@21 installed in its package.json file, the MFE should also be using the latest v21 of the external theme CSS to ensure compatibility.

[adamstankiewicz]

Reading the docs will allow us to comment on this PR in a much more informed way. Also, writing the docs will help you realize whether the new extension mechanism integrates well with the workflows of platform administrators and Open edX developers.

@regisb Noted! Yes, documentation is definitely a critical component of this project to move from current state of theming to runtime theming via design tokens. While there is no specific documentation to this frontend-platform implementation quite yet, the existing ADR proposing a related approach helped give some motivation/context for this PR. That said, I've taken an action item to:

• Write a brief ADR to go alongside this implementation in the same PR, likely borrowing certain aspects of the existing ADR linked above, describing the motivation/context, decision, etc.
• Include a "how to" doc in the repo with more specifics about "How do I use this theming extension?" for consumers.

Also worth noting there is a separate documentation effort needed for how to do the Paragon theming with design tokens and CSS variables 😄

[felipemontoya]

I can't comment on the implementation, but the overall approach, and the concept of the core and light css variable bundles seem very appropriate to me. Thanks for getting this done.

[regisb]

Thanks for your answer @adamstankiewicz! I think that the extension docs should live close to the rest of the developer docs: https://docs.openedx.org/en/latest/developers/index.html

[xitij2000]

The core theme CSS (core.css) intends to be an all-encompassing theme containing all of the Paragon styles necessary the foundational parts of the Open edX theme including layout, typography, component styles, etc. The light theme variant CSS (light.css) - in the future, possibly an additional dark.css for dark mode support - would define just the CSS variables necessary for the light theme variant (the default). These CSS variables are used within core.css.

Considering that it's now possible to have richer JSON-based config thanks to the config API. (i.e. the config API can return a JSON for any setting). Perhaps this code can be made more generic to load any kind of theme?

i.e. the config api can return a structure like:

{
  "core": "https...",
  "variants":  {
     "light": "https...",
     "dark": "https...",
     "high-contrast": "https..."
  }
}

Which is the structure being built here:

https://github.com/openedx/frontend-platform/pull/440/files#diff-e986d374444ced0f5162cc6cb8b3dac6cbd228f339851f2a99a4aec545559d87R71-R78

If the config API isn't enabled, then it can continue to fall back to the APP_THEME_LIGHT_URL.

[adamstankiewicz]

@xitij2000 Great suggestion! Yeah, agreed this should prefer any runtime config settings, and otherwise fall back to the regular ol' environment variables. I can't imagine it'd be too much of a lift to support the runtime config use case, to use an already created object structure representing the theme core/variant URLs, if it exists.

I'm not immediately sure what that implementation might look like as I'm not all that familiar with the config API outside of how frontend-platform calls it; is this something you might want to take a stab at (feel free to put some pseudocode or otherwise together as a PR to this one!)?

Out of curiosity... I'm not actually sure, is the runtime config used in production by anyone at this point (AFAIK it's not being used by 2U/edX yet)?

[felipemontoya]

We are already using the runtime config in prod for one medium sized instance with ~70 multitenant sites. So far so good with its usage.

[xitij2000]

I'm not immediately sure what that implementation might look like as I'm not all that familiar with the config API outside of how frontend-platform calls it; is this something you might want to take a stab at (feel free to put some pseudocode or otherwise together as a PR to this one!)?

Sure! I'll put together something based on this PR and link it here when ready.

[adamstankiewicz]

Sure! I'll put together something based on this PR and link it here when ready.

@xitij2000 I ended up putting together a possible solution to prefer config from the MFE config API; otherwise, fallback to environment variables. I just pushed up the latest changes to this PR. The runtime config vs. environment variable config approaches are described in a new docs/how_tos/theming.md document.

[dcoa]

I made some testing about this implementation and paragon alpha and I'm excited about it. I would like to ask about the next steps of this implementation, this draft pretends to become an open PR or do you have in mind a better solution that we can check?

Thanks for all the work around theming.

[andrey-canon]

Hi @adamstankiewicz, I was testing this implementation and I found an error, when I set both variables PARAGON_THEME_VARIANTS_LIGHT_URL and PARAGON_THEME_CORE_URL the mfe doesn't loads

Both variables

One variable

[adamstankiewicz]

@andrey-canon Appreciate you doing some additional QA on this branch! Do you happen to see any relevant JavaScript errors in the console or your terminal at build time? When I run this branch against frontend-app-profile using the CDN urls for the @edx/paragon@alpha release, it seems to be working as expected. See screenshot with both the core.css and light.css URLs injected into the <head>.

[felipemontoya]

Coming back from the contributors meeting to bump this.

I wanted to say this a great addition to the frontend. I have not been able to give a proper review because my frontend skills are not quite up to the task. However we have been using this feature to theme different instances in installations running nutmeg and olive. We backported the feature and the paragon design tokens to both releases. We are not looking into backporting to palm. I'd be very happy to make this land in master so that we can build more theming capabilities on top of it.

@dcoa please could you give this a new look with the last version changes and provide an official checkmark from our side if everything is working correctly?

[adamstankiewicz]

@felipemontoya Appreciate the ping, and it was great catching up a bit with you earlier this week to understand the eduNext use cases in more depth!

As mentioned, the design tokens project is close to being ready-for-release, with the exception of this PR and the associated PR in frontend-build, as well as some quality of life improvements targeted at improving the migration process for consumers across the Open edX platform (37+ repos), such as documentation and any opportunities for CLI tooling. I gave an estimate of perhaps conservatively 2-3 months 🤞

As for this work in particular, I spent the afternoon updating it to account for the feedback from a recent Paragon Working Group where we discussed this PR to add support for a defaults configuration property in PARAGON_THEME_URLS.

The only thing I believe that's remaining for this PR at this point are tests 👀 TBD when I will be able to get to those. If others have some capacity and want to contribute to this PR, tests might be a good place to start; feel free to open a PR against this one!

---

I've also added some additional features to this PR since we spoke:

Disclaimer: the "dark" mode in the examples below are effectively set to "no theme variant" since Paragon doesn't have an official dark mode (yet).

• Sets users' theme variant selection (e.g., light vs. dark) in localStorage so the user's choice persists across sessions and page refreshes.

paragon-dark-mode-localstorage-persist.webm

• If user has not made an explicit choice (i.e., no value stored in localStorage), checks if the user's system preference prefers a dark color scheme, and if so, loads the dark theme variant (if configured). Responds to changes in the system preferences in real time.

paragon-dark-mode-system-preferences.webm

• Otherwise, falls back to the configured default light theme variant.

{
    "core": {
        "url": "https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/core.min.css"
    },
    "defaults": {
        "light": "light",
    },
    "variants": {
        "light": {
            "url": "https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/light.min.css",
        }
    }
}

• Adds a data-paragon-theme-variant="light|dark" attribute to the <html> element, which gives consuming applications the ability to know the current theme variant via their own application CSS and JS (e.g., in a CSS file, where React Context is not available).

[dcoa]

I was testing system preference configuration and it works when default and brandOverride are defined but when I use only default values like:

"PARAGON_THEME_URLS": {
                "core": {
                    "url": "https://cdn.jsdelivr.net/npm/@edx/[email protected]/dist/core.min.css"
                },
                "defaults": {
                    "dark": "dark",
                    "light": "light"
                },
                "variants": {
                    "dark": {
                        "url": "https://cdn.jsdelivr.net/npm/@edx/[email protected]/dist/dark.min.css""
                    },
                    "light": {
                        "url": "https://cdn.jsdelivr.net/npm/@edx/[email protected]/dist/light.min.css"
                    }
                }
            }

can not change the theme properly:

https://github.com/openedx/frontend-platform/assets/66016493/3caa1e8c-b926-4644-ad63-9eede11de130

[Mashal-m]

Hey @adamstankiewicz , What is the current status of this PR, is it ready to review and merge? Could you please resolve conflicts?

[xitij2000]

@adamstankiewicz Is there any way we can help get this closer to merging? We're currently using this in conjunction with patched versions of MFEs and it's working pretty well.

[arbrandes]

This was discussed today in an FWG meeting, It sounds like this will be taken forward as part of a Design Token Axim Funded Contribution.

[adamstankiewicz]

Closed in favor of https://github.com/openedx/frontend-build/pull/546, which picked up where this PR left off :)