Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adds support for loading external theme CSS for MFEs #440

Closed
wants to merge 59 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
3cde02d
feat: add ability to dynamically load theme overrides
viktorrusakov Feb 2, 2023
f41dcb2
feat: adds support for loading external theme CSS for MFEs
adamstankiewicz Feb 15, 2023
2b19931
fix: handle missing theme config
adamstankiewicz Feb 15, 2023
347957b
fix: add env vars to env files
adamstankiewicz Feb 15, 2023
ef52e41
fix: remove unused code
adamstankiewicz Feb 15, 2023
2849bfd
chore: wip commit
adamstankiewicz Feb 24, 2023
a298bf5
fix: prefer runtime config for paragon theme
adamstankiewicz Feb 27, 2023
697e43b
fix: rebase on master and resolve package conflicts
adamstankiewicz Feb 27, 2023
fbfd722
fix: revert to original webpack.dev.config config
adamstankiewicz Feb 27, 2023
a2b3f99
chore: clean up unnecessary comment line
adamstankiewicz Feb 27, 2023
b477c8d
fix: grammar
adamstankiewicz May 20, 2023
916c1b3
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz May 20, 2023
4b2038c
fix: remove duplicate import
adamstankiewicz May 20, 2023
26fbfac
chore: refresh package-lock.json to lockfileVersion 3
adamstankiewicz May 20, 2023
b259760
fix: clean up link nodes in document head when no longer needed
adamstankiewicz May 20, 2023
155bf03
fix: fallback to locally installed core and light theme css urls
adamstankiewicz May 27, 2023
e5ee81e
docs: update to docs
adamstankiewicz May 27, 2023
813169d
chore: update docs about
adamstankiewicz May 27, 2023
a714d49
fix: expose setThemeVariant
adamstankiewicz May 27, 2023
1e13ac3
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz May 27, 2023
e2b0df9
docs: update docs
adamstankiewicz May 27, 2023
ea99e3c
fix: rebase with master and update based on PARAGON changes
adamstankiewicz May 28, 2023
00fd9c2
chore: remove support for env vars config for paragon dynamic theming
adamstankiewicz May 29, 2023
0da2cfa
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz May 29, 2023
f24c336
chore: clean up package-lock
adamstankiewicz May 29, 2023
84b34ee
fix: updates
adamstankiewicz May 29, 2023
f9aa947
fix: one more update
adamstankiewicz May 29, 2023
fb70be9
fix: refresh package-lock.json
adamstankiewicz May 29, 2023
e0768ff
fix: refresh package-lock.json pt2
adamstankiewicz May 29, 2023
4c5f358
fix: updates
adamstankiewicz May 29, 2023
c957eb5
fix: update package-lock.json
adamstankiewicz May 29, 2023
2c386ac
fix: make it theme variant agnostic
adamstankiewicz May 31, 2023
340e259
docs: update howto theming guide
adamstankiewicz May 31, 2023
9b5bfa5
fix: ensure app loads without styles if the PARAGON_THEME_URLS and fa…
adamstankiewicz May 31, 2023
59401f5
fix: ensure fallback theme links are removed if they also error
adamstankiewicz May 31, 2023
46cb39a
docs: add link to mfe runtime config api adr
adamstankiewicz May 31, 2023
ebbe03a
fix: don't attempt to load paragon css urls if PARAGON_THEME_URLS is …
adamstankiewicz Jun 1, 2023
494ad62
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz Jun 1, 2023
74f9f8f
fix: brand overrides
adamstankiewicz Jun 5, 2023
e3e5fe5
docs: fix code example
adamstankiewicz Jun 5, 2023
902e8f4
docs: add missing comma
adamstankiewicz Jun 5, 2023
0c73b5b
docs: update how to
adamstankiewicz Jun 5, 2023
b380c18
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz Jul 15, 2023
67a0a1d
feat: support dark mode
adamstankiewicz Jul 15, 2023
c30ae3f
chore: update package-lock.json
adamstankiewicz Jul 15, 2023
e2a115e
chore: update package-lock.json take 2
adamstankiewicz Jul 15, 2023
33a8377
chore: remove console.log statements
adamstankiewicz Jul 15, 2023
45c727e
fix: ignore system preference change when theme variant set in locals…
adamstankiewicz Jul 16, 2023
f6d633c
chore: add tests for updates to AppProvider
adamstankiewicz Jul 16, 2023
d65acff
chore: update react-intl to pass peer dependencies after pinning all …
adamstankiewicz Jul 16, 2023
efdf60c
chore: split hooks.js up into separate files and begin some related t…
adamstankiewicz Jul 16, 2023
e96de6b
test: add testing to useParagonTheme hooks (#514)
dcoa Jul 21, 2023
8f39517
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz Jul 21, 2023
cf10278
chore: update package-lock.json
adamstankiewicz Jul 21, 2023
8f0edb4
fix: update links in head and *isLoaded to true (#534)
monteri Dec 9, 2023
084a4ed
Merge branch 'master' into ags/inject-theme-css
adamstankiewicz Dec 9, 2023
b5f1588
test: add test to useParagonTheme hook and paragon utils (#525)
dcoa Dec 9, 2023
2b2772e
Merge branch 'ags/inject-theme-css' of github.com:openedx/frontend-pl…
adamstankiewicz Dec 9, 2023
3345dc0
fix: resolve lockfileVersion conflict after merge with master
adamstankiewicz Dec 9, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ STUDIO_BASE_URL=http://localhost:18010
MARKETING_SITE_BASE_URL=http://localhost:18000
ORDER_HISTORY_URL=http://localhost:1996/orders
REFRESH_ACCESS_TOKEN_ENDPOINT=http://localhost:18000/login_refresh
SEGMENT_KEY=''
SEGMENT_KEY=
SITE_NAME=localhost
USER_INFO_COOKIE_NAME=edx-user-info
LOGO_URL=https://edx-cdn.org/v3/default/logo.svg
Expand All @@ -27,4 +27,6 @@ FAVICON_URL=https://edx-cdn.org/v3/default/favicon.ico
IGNORED_ERROR_REGEX=
MFE_CONFIG_API_URL=
APP_ID=
SUPPORT_URL=https://support.edx.org
SUPPORT_URL=https://support.edx.org
PARAGON_THEME_CORE_URL=
PARAGON_THEME_VARIANTS_LIGHT_URL=
2 changes: 2 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ IGNORED_ERROR_REGEX=
MFE_CONFIG_API_URL=
APP_ID=
SUPPORT_URL=https://support.edx.org
PARAGON_THEME_CORE_URL=
PARAGON_THEME_VARIANTS_LIGHT_URL=
40 changes: 40 additions & 0 deletions docs/how_tos/theming.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Theming support with Paragon

Status: Draft

This document serves as a guide to using `@edx/frontend-platform` to support MFE theming with Paragon using theme CSS loaded externally (e.g., from a CDN). By serving CSS from externally, consuming applications of Paragon no longer need to be responsible for compiling the theme SCSS to CSS themselves and instead use a pre-compiled CSS file instead. In doing so, this allows making changes to the Paragon theme without needing to necessarily re-build and re-deploy all consuming applications. We would also get a meaningful gain in performance as loading the compiled theme CSS from an external CDN means micro-frontends (MFEs) can include cached styles instead of needing to load essentially duplicate theme styles as users navigate across different MFEs.
adamstankiewicz marked this conversation as resolved.
Show resolved Hide resolved

## Theme URL configuration

Paragon supports 2 mechanisms for configuring the Paragon theme URLs:
* Environment variable configuration
* Runtime configuration

The Paragon theming extension to dynamically load external theme CSS prefers the theme configuration in the runtime config over the environment variable configuration.

### Environment variable configuration

The standard way to configure MFEs is to use environment variables specific to the application environment they are running in. For example, during local development, environment variables are defined and loaded via the `.env.development` file.

Two new environment variables are exposed to configure the Paragon theme URLs:
* `PARAGON_THEME_CORE_URL`. This URL represents the foundational theme styles provided by Paragon's `core.css` file.
* `PARAGON_THEME_VARIANTS_LIGHT_URL`. This URL represents the light theme variant specific styles provided by Paragon's `light.css` file.

### Runtime configuration

`@edx/frontend-platform` additionally supports loading application configuration from an API at runtime rather than environment variables. For example, in `edx-platform`, there is an API endpoint for MFE runtime configuration at `http://localhost:18000/api/mfe_config/v1`. The application configuration may be setup via Django settings as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: To me this paragraph sort of implies that the MFE config API in edx-platform is one example of how this can be done, rather than the sole way frontend-platform can get this info. (I mean, I guess someone could configure the frontend to point at some other service, but none exists!)

A link to the MFE config API doc here would probably be helpful.

Copy link
Member Author

@adamstankiewicz adamstankiewicz May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: To me this paragraph sort of implies that the MFE config API in edx-platform is one example of how this can be done, rather than the sole way frontend-platform can get this info. (I mean, I guess someone could configure the frontend to point at some other service, but none exists!)

Agreed. Re-worded, and will push up a fix shortly.

A link to the MFE config API doc here would probably be helpful.

@davidjoy Is there a suitable link to include here? I'm not aware of one. I know I started to create this (currently 2U-internal) document describing the different configuration mechanisms because I couldn't find any other documentation about the MFE runtime config API.

That being said, yes, a link to some more detailed, public docs here would be helpful :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the language to include this above suggested link.


```
ENABLE_MFE_CONFIG_API = True
MFE_CONFIG = {}
MFE_CONFIG_OVERRIDES = {
"profile": {
"PARAGON_THEME_URLS": {
'core': 'https://cdn.jsdelivr.net/npm/@edx/[email protected]/dist/paragon.css',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to support variable substitutions here? i.e. the URL could be:

https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/paragon.css

This way with each upgrade you don't need to update this value here in a completely different place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last version of Paragon Alpha creates a folder instead of a file (core, and theme variant)

  • @edx/paragon
    • styles
      • css
        • core
          • custom-media-breakpoints.css
          • index.css
          • variables.css
        • themes
          • light
            • index.css
            • utility-classes.css
            • variables.css

I was wondering if this implementation should take that into consideration or should be resolved by the jsdelivr capacity to load more than one file per request https://www.jsdelivr.com/documentation#id-combine-multiple-files?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to support variable substitutions here? i.e. the URL could be: https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/paragon.css

@xitij2000 Can you expand a bit more on how you think the variable substitution would work here? Where would $paragonVersion come from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcoa That the output you're showing is from the npm run build-tokens command. We have another command node build-scss.js (or via npm run build) that compiles all of Paragon's SCSS and these generated CSS files into the expected output files in the dist folder:

  • core.css
  • core.min.css
  • core.css.map
  • light.css
  • light.min.css
  • light.css.map

These compiled CSS files during this build step are the ones intended to hosted on a CDN, where this @edx/frontend-platform solution would pull from.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to support variable substitutions here? i.e. the URL could be: https://cdn.jsdelivr.net/npm/@edx/paragon@$paragonVersion/dist/paragon.css

@xitij2000 Can you expand a bit more on how you think the variable substitution would work here? Where would $paragonVersion come from?

The idea with variable substitution is that if you upgrade the version of paragon in the MFE it should automatically inject its version of paragon into this URL instead of requiring you to update the MFE configuration is site-configuration, or in settings.

The version of Paragon to be filled in here is known at build time since it's in package.json so it can just be a static variable auto-discovered from the package.json file.

Copy link
Member Author

@adamstankiewicz adamstankiewicz May 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] I pushed up a commit using the above linked version of @edx/frontend-build to demonstrate the usage of the PARAGON_VERSION global variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming I am not incorrect, this would lead to style shift, right? because the flow as I understnad would as

  • The HTML of the is MFE is served
  • Browser start fetching resoucrs app.js .app.css ...etc
  • Now concurrently browser would
    • Execuse the app.js of which it fetch the dynamic config api
    • Start rendering the screen
  • Now if dynamic config from previous step would chane the style, there would be stlyle shift i.e. user would see a theme style and then suddenly the theme/style would change.

Am I right, with the timeline of events above?, either cases I think the use PARAGON_VERSION is valuable, but we probably shall ask if sudden style change is okay and if not how to resolve it...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I am referring to is exactly this https://css-tricks.com/content-jumping-avoid/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading into the thread, I think my comment above, is related even if we don't define PARAGON_VERSION. i.e even if just define PARAGON_THEME_URLS, wouldn't the MFE at first load the deafult theme that was used/set when the MFE is built, before it get a chance to update it using via the dynamic config API?.

Copy link
Member Author

@adamstankiewicz adamstankiewicz May 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghassanmas I believe the intent here is that the MFE would no longer import any of Paragon's theme SCSS, in favor of having @edx/frontend-platform inject the compiled theme CSS via AppProvider at runtime, or otherwise fallback to the CSS included in the locally installed Paragon verison.

In this case, the MFE isn't shown until the externally loaded CSS files are injected/loaded by the browser, similar to how the MFE isn't rendered until the request to fetch the dynamic MFE config API is resolved.

See the below graphic for the flow in this proposed implementation for injecting/loading the Paragon theme URLs (I'll plan on adding a similar graphic to the ADR in this PR as well):

image

'variants': {
'light': 'https://cdn.jsdelivr.net/npm/@edx/[email protected]/scss/core/css/variables.css',
},
},
},
}
```
3 changes: 2 additions & 1 deletion example/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import {
import { APP_INIT_ERROR, APP_READY, initialize } from '@edx/frontend-platform';
import { subscribe } from '@edx/frontend-platform/pubSub';

import './index.scss';
import ExamplePage from './ExamplePage';
import AuthenticatedPage from './AuthenticatedPage';

import './index.scss';

subscribe(APP_READY, () => {
ReactDOM.render(
<AppProvider>
Expand Down
Loading