[BD-46] feat: added support Paragon design tokens #351

PKulkoRaccoonGang · 2023-06-15T18:31:18Z

Important

Update frontend-platform to a version compatible with Paragon 23.
Needs openedx/frontend-platform#689 to be merged.

Issue: openedx/paragon#2348

openedx-webhooks · 2023-06-15T18:31:22Z

Thanks for the pull request, @PKulkoRaccoonGang!

When this pull request is ready, tag your edX technical lead.

adamstankiewicz · 2023-07-14T12:09:21Z

example/index.scss

+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/core.min.css");
+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/light.min.css");
+@import "@edx/paragon/styles/scss/core/core";
+@import "@edx/paragon/styles/css/themes/light/index.css";


[clarification] As is, I believe these would be importing duplicate styles? For example, @import "@edx/paragon/styles/css/themes/light/index.css"; is largely the same import as @import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/light.min.css"); above.

If we're importing the core.css and light.css files via example/index.scss, we should likely rely on the local imports from @edx/paragon. Take a look at the draft migration documentation for an example of what I imagine MFEs would need to do to import the compiled Paragon CSS locally: https://openedx.atlassian.net/wiki/spaces/BPL/pages/3770744958/Migrating+MFEs+to+Paragon+design+tokens+and+CSS+variables#Using-locally-installed-Paragon-CSS

Alternatively, we could also consider configuring the example MFE to utilize the @edx/frontend-platform approach to injecting the Paragon CSS from external CDN url like jsDelivr once openedx/frontend-platform#440 merges.

These imports have been changed, now it looks like this:

@use "@openedx/paragon/dist/core.min.css" as paragonCore; @use "@openedx/paragon/dist/light.min.css" as paragonLight;

Alternatively, we could also consider configuring the example MFE to utilize the @edx/frontend-platform approach to injecting the Paragon CSS from external CDN url like jsDelivr once openedx/frontend-platform#440 merges.

We will get a working version for testing here: openedx/frontend-app-discussions#723

adamstankiewicz · 2023-07-14T12:17:11Z

src/index.scss

+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/core.min.css");
+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/light.min.css");
+@import "@edx/paragon/styles/scss/core/core";
+@import "@edx/paragon/styles/css/themes/light/index.css";


I think we should consider whether there are any performance concerns around coupling Paragon's styles with these shared components. For example, if both these component includes all of Paragon's CSS and the consuming MFE includes all of Paragon's CSS, will the user essentially need to download all of Paragon's CSS twice?

I'm wondering if it would make sense for the shared components to be de-coupled from Paragon styles, with the assumption that these components will always be used within an application that provides Paragon styles externally from the component. A potential concern with this proposed approach may be around versioning; that is, is there a risk that developers could run into version discrepancies between what this component expects/uses and what version of Paragon's CSS is provided by the consuming/hosting MFE?

If we do leave Paragon styles coupled with the shared component, it may be a good to exercise to quantify/benchmark the performance implications of both approaches?

What do you think?

Personally, I feel like we should de-couple Paragon styles from these components. The biggest problem with leaving Paragon's styles here is that we're dealing with CSS now - if we import Paragon's CSS variables here, then they will override all CSS variables from MFE's Paragon and this will affect the whole MFE (it will happen because header / footer styles are usually imported last in the MFE). This will basically mean that header component is in charge of the theme for the whole platform.

And even if we switch import order in the MFE, it will just mean that MFE will override all CSS used by header component.

I think if we're migrating to CSS, then MFEs should be a single source of truth for the styles in the platform. People should also understand that they cannot override CSS variables in their styles because that will likely affect this variable across all styles.

As for the problem with version discrepancies... The first thing we could do is to advise engineers to only use global tokens from Paragon in SCSS files in header / footer components as they are less likely to change in Paragon. But the main problem is with components I guess, because they use CSS classes from Paragon which are a lot likely to change between versions, not sure what to do here. The only long-term solution that comes to mind is to de-couple header / footer components even further from Paragon and just do not use Paragon's component's here at all. And create new components that will re-use only Paragon's CSS variables so that Paragon would still control the theme of the header / footer.

Another idea is to move header and footer components into Paragon so they always share the version? I might be crazy though haha

It seems that in 2025 we have already decided to abandon the imports of styles provided by Paragon and have completely trusted the single source of styles - MFE.

A potential concern with this proposed approach may be around versioning; that is, is there a risk that developers could run into version discrepancies between what this component expects/uses and what version of Paragon's CSS is provided by the consuming/hosting MFE?

Such a problem can occur. Perhaps with the advent of Webpack Module Federation, we will be able to eliminate the version control issues for our microfrontends and manage/update all dependencies from a single place. At the moment, the only way, as it seems to me, to identify and fix potential issues is to visually check the correctness of the header display. Fortunately, it is just a small component.

adamstankiewicz · 2023-07-14T13:07:34Z

example/index.scss

-@import "@edx/brand/paragon/overrides";
+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/core.min.css");
+@import url("https://cdn.jsdelivr.net/npm/@edx/brand-edx.org@alpha/dist/light.min.css");
+@import "@edx/paragon/styles/scss/core/core";


Should this be importing the core.css rather than SCSS?

Diana corrected it and now we have this version: 4ef75fe

@use "@openedx/paragon/dist/core.min.css" as paragonCore; @use "@openedx/paragon/dist/light.min.css" as paragonLight; @import "@edx/frontend-component-header/index";

adamstankiewicz · 2023-07-21T13:20:44Z

package.json

@@ -55,7 +55,7 @@
    "redux-saga": "1.2.3"
  },
  "dependencies": {
-    "@edx/paragon": "20.44.0",
+    "@edx/paragon": "21.0.0-alpha.38",


Consider moving this to a peer dependency.

Moved and updated - "@openedx/paragon": ">= 23.0.0",

This PR purpose is to test/demo parago design tokens simliar to this one for the profile openedx/frontend-app-profile/pull/764 it override the following depns as seen in package.json - paragon alpha - openedx/frontend-build/pull/365 - openedx/frontend-platform/pull/440 - openedx/frontend-component-header/pull/351 - openedx/frontend-component-footer/pull/303 Conclousion so far: - There is an extra library that learning depends on which needs to support paragon; `frontend-lib-learning-assistant` and also `frontend-lib-special-exams` - It would be great to have a gudie or a document to look at, while doing the conversion that would **map variable from who it was used/named before to the name in design tokens** - I was stuck in the end with compliation error, that wepack couldn't find `Modal` exported from paragon.

Mashal-m · 2023-11-17T07:35:40Z

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

PKulkoRaccoonGang · 2023-11-17T22:04:35Z

@Mashal-m Hello! At the moment, all pull requests opened in the edx and openex repositories are awaiting further action on the use of Paragon design tokens in the MFE. Don't pay any attention to them for now.

Full list of pull requests that should just be open for now:

brian-smith-tcril

Overall this is looking great! I'm feeling really good about being able to do this as a non-breaking change!

I left a few comments in there, nothing scary - just a few changes where I felt some edge cases might tip the scales in the "possibly breaking" direction.

src/index.scss

package.json

src/Menu/menu.scss

src/index.scss

src/studio-header/StudioHeader.scss

brian-smith-tcril

I think this PR is really close to ready to land!

Things I'd like to do before merging:

One last round of testing on 22 and 23 based MFEs
- I'm happy to do this as local testing on Monday
Remove dist/restore .gitignore
Get some input on [BD-46] feat: added support Paragon design tokens #351 (comment) - I'm still thinking "having >= 22.0.0 < 24.0.0 in this PR feels like the most reasonable option" - but it'd be nice to hear some thoughts from others before landing this.

src/index.scss

brian-smith-tcril · 2025-03-17T15:56:21Z

@dcoa I have verified this works locally on latest master of frontend-app-learning (Paragon 22) by using module.config.js!

I also tried testing this against openedx/frontend-app-learning#1588 (Paragon 23) locally but I encountered an issue where no styles were loading. It's possible that problem was with my dev environment as opposed to the code - is that frontend-app-learning PR working locally for you with Paragon 23? If not no worries, I can test this against openedx/frontend-app-discussions#723 instead.

dcoa · 2025-03-17T21:52:59Z

@dcoa I have verified this works locally on latest master of frontend-app-learning (Paragon 22) by using module.config.js!

I also tried testing this against openedx/frontend-app-learning#1588 (Paragon 23) locally but I encountered an issue where no styles were loading. It's possible that problem was with my dev environment as opposed to the code - is that frontend-app-learning PR working locally for you with Paragon 23? If not no worries, I can test this against openedx/frontend-app-discussions#723 instead.

@brian-smith-tcril , I haven't added the empty object in the MFEs to fallback the styles to the local installed (I will do that).

brian-smith-tcril · 2025-03-18T14:32:57Z

@dcoa I got it working using Paragon 23 in frontend-app-learning with the following changes to openedx/frontend-app-learning#1588

diff --git a/package.json b/package.json
index 8350534d..09aceeb0 100644
--- a/package.json
+++ b/package.json
@@ -37,10 +37,11 @@
   "dependencies": {
     "@edx/brand": "npm:@openedx/brand-openedx@^1.2.2",
     "@edx/browserslist-config": "1.4.0",
+    "@edx/frontend-component-footer": "^14.3.0",
     "@edx/frontend-component-header": "^5.8.0",
     "@edx/frontend-lib-learning-assistant": "^2.14.2",
-    "@edx/frontend-lib-special-exams": "^3.1.3",
-    "@edx/frontend-platform": "^8.0.0",
+    "@edx/frontend-lib-special-exams": "^3.4.0",
+    "@edx/frontend-platform": "^8.3.1",
     "@edx/openedx-atlas": "^0.6.0",
     "@edx/react-unit-test-utils": "3.0.0",
     "@fortawesome/free-brands-svg-icons": "5.15.4",
@@ -48,7 +49,7 @@
     "@fortawesome/free-solid-svg-icons": "5.15.4",
     "@fortawesome/react-fontawesome": "^0.1.4",
     "@openedx/frontend-build": "^14.2.0",
-    "@openedx/frontend-plugin-framework": "^1.2.1",
+    "@openedx/frontend-plugin-framework": "^1.6.0",
     "@openedx/frontend-slot-footer": "^1.0.2",
     "@openedx/paragon": "^23.3.0",
     "@popperjs/core": "2.11.8",
diff --git a/src/index.scss b/src/index.scss
index 12c045b0..c4d11c3a 100755
--- a/src/index.scss
+++ b/src/index.scss
@@ -1,7 +1,9 @@
+@use "../node_modules/@openedx/paragon/dist/core.min.css" as paragonCore;
+@use "../node_modules/@openedx/paragon/dist/light.min.css" as paragonLight;
 @use "@openedx/paragon/styles/css/core/custom-media-breakpoints" as paragonCustomMediaBreakpoints;
 
 @import "~@edx/frontend-component-footer/dist/footer";
-@import "~@edx/frontend-component-header/dist/index";
+@import "../../frontend-component-header/dist/index";
 
 
 #root {

Then I ran

npm install --legacy-peer-deps

And updated module.config.js to have

{ moduleName: '@edx/frontend-component-header', dir: '../frontend-component-header/', dist: 'dist' }

in localModules (my local checkout of this frontend-component-header PR is a sibling of my local frontend-app-learning checkout - I pulled the latest version of this PR and ran npm ci, npm run build)

brian-smith-tcril

Thanks for working through all the changes required to be able to land this as non-breaking! This really came together beautifully!

brian-smith-tcril · 2025-03-18T14:37:26Z

example/index.scss

+@use "@openedx/paragon/dist/core.min.css" as paragonCore;
+@use "@openedx/paragon/dist/light.min.css" as paragonLight;


I got errors when I had this in frontend-app-learning's index.scss

ERROR in ./src/index.scss (./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[1].oneOf[1].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[1].oneOf[1].use[2]!./node_modules/resolve-url-loader/index.js!./node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[1].oneOf[1].use[4]!./src/index.scss) Module build failed (from ./node_modules/sass-loader/dist/cjs.js): Can't find stylesheet to import. ╷ 3 │ @use "@openedx/paragon/dist/core.min.css" as paragonCore;

I was able to work around it by using

@use "../node_modules/@openedx/paragon/dist/core.min.css" as paragonCore; @use "../node_modules/@openedx/paragon/dist/light.min.css" as paragonLight;

which feels less than ideal. My assumption is that can be fixed by some webpack config changes. I consider this non-blocking but worth noting.

adamstankiewicz · 2025-03-18T14:56:57Z

src/studio-header/StudioHeader.scss

@@ -33,13 +33,13 @@ $white: #FFFFFF;

    @media only screen and (min-width: 769px) {
      padding: .5rem;
-      padding-right: $spacer;
+      padding-right: var(--pgn-spacing-spacer-base, $spacer);
      border-right: 1px solid #E5E5E5;


[nit, non-blocking]: hardcoded HEX value; is it worth swapping to a token/CSS variable? This color doesn't appear to an official Paragon color (presumably, it should be one of the officially supported gray levels)

openedx-webhooks added the blended PR is managed through 2U's blended developmnt program label Jun 15, 2023

PKulkoRaccoonGang mentioned this pull request Jun 15, 2023

Understand and address design tokens & CSS variable related issues in @edx/frontend-component-* repos openedx/paragon#2348

Open

PKulkoRaccoonGang marked this pull request as draft June 15, 2023 18:38

PKulkoRaccoonGang force-pushed the Peter_Kulko/support-design-tokens branch 3 times, most recently from 80f93ec to 17aee00 Compare June 22, 2023 18:36

monteri approved these changes Jun 23, 2023

View reviewed changes

PKulkoRaccoonGang requested a review from adamstankiewicz June 29, 2023 06:34

adamstankiewicz changed the title ~~[BD-46] feat: added support Paragon design tokens~~ [BD-46] feat!: added support Paragon design tokens Jul 14, 2023

adamstankiewicz reviewed Jul 14, 2023

View reviewed changes

adamstankiewicz reviewed Jul 21, 2023

View reviewed changes

PKulkoRaccoonGang requested a review from monteri August 9, 2023 12:19

monteri approved these changes Aug 9, 2023

View reviewed changes

PKulkoRaccoonGang marked this pull request as ready for review August 9, 2023 13:11

PKulkoRaccoonGang force-pushed the Peter_Kulko/support-design-tokens branch from 180a7fb to c264b1b Compare August 9, 2023 13:27

PKulkoRaccoonGang changed the title ~~[BD-46] feat!: added support Paragon design tokens~~ [BD-46] feat: added support Paragon design tokens Aug 9, 2023

PKulkoRaccoonGang force-pushed the Peter_Kulko/support-design-tokens branch from c264b1b to 8a7607e Compare August 9, 2023 13:33

PKulkoRaccoonGang requested a review from monteri August 9, 2023 14:07

PKulkoRaccoonGang force-pushed the Peter_Kulko/support-design-tokens branch from 8a7607e to dc84f97 Compare August 18, 2023 10:52

ghassanmas mentioned this pull request Sep 6, 2023

[do not merge] DEMO: design tokens and CSS variables openedx/frontend-app-profile#764

Draft

ghassanmas mentioned this pull request Oct 6, 2023

[DRAFT] feat: add support to paragon design tokens openedx/frontend-app-learning#1203

Closed

PKulkoRaccoonGang marked this pull request as draft December 18, 2023 09:25

dcoa force-pushed the Peter_Kulko/support-design-tokens branch from 07b1c4f to 053cf7e Compare March 14, 2025 11:09

brian-smith-tcril reviewed Mar 14, 2025

View reviewed changes

dcoa force-pushed the Peter_Kulko/support-design-tokens branch 4 times, most recently from 6496409 to 35b22d6 Compare March 15, 2025 00:03

brian-smith-tcril reviewed Mar 15, 2025

View reviewed changes

src/index.scss Show resolved Hide resolved

src/index.scss Show resolved Hide resolved

src/index.scss Show resolved Hide resolved

src/index.scss Show resolved Hide resolved

PKulkoRaccoonGang and others added 14 commits March 18, 2025 11:52

feat: added support Paragon design tokens

9e95158

refactor: styles refactoring

8373dc4

refactor: removed paragon imports

2e3f393

refactor: moving the Paragon and Platform in peer and dev dependencies

dcae599

feat: add studio header styles and remove unused styles

17fb1d1

fix: update example app and dependencies

b51c82a

fix: update padding in couse title for learning header

c6e7e87

refactor: added dist folder

24b65b6

chore: update peerDeps

d066c85

test: update dist folder

534ed7a

chore: updated dist after rebase

83e0fbb

chore: update frontend-platform and frontend-plugin-framework

11ce896

fix: update to no breaking change

0ab3216

fix: implement feedback

27b8411

dcoa force-pushed the Peter_Kulko/support-design-tokens branch from 35b22d6 to 27b8411 Compare March 18, 2025 00:56

revert: remove dist forlder

57ae6a2

brian-smith-tcril approved these changes Mar 18, 2025

View reviewed changes

adamstankiewicz approved these changes Mar 18, 2025

View reviewed changes

brian-smith-tcril mentioned this pull request Mar 18, 2025

Hardcoded hex value sweep #579

Open

brian-smith-tcril merged commit 7970561 into openedx:master Mar 18, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-46] feat: added support Paragon design tokens #351

[BD-46] feat: added support Paragon design tokens #351

PKulkoRaccoonGang commented Jun 15, 2023 •

edited

Loading

openedx-webhooks commented Jun 15, 2023 •

edited

Loading

adamstankiewicz Jul 14, 2023

PKulkoRaccoonGang Feb 27, 2025

adamstankiewicz Jul 14, 2023

viktorrusakov Jul 21, 2023

PKulkoRaccoonGang Feb 27, 2025

adamstankiewicz Jul 14, 2023

PKulkoRaccoonGang Feb 27, 2025

adamstankiewicz Jul 21, 2023

PKulkoRaccoonGang Feb 27, 2025

Mashal-m commented Nov 17, 2023

PKulkoRaccoonGang commented Nov 17, 2023 •

edited

Loading

brian-smith-tcril left a comment

brian-smith-tcril left a comment

brian-smith-tcril commented Mar 17, 2025

dcoa commented Mar 17, 2025

brian-smith-tcril commented Mar 18, 2025

brian-smith-tcril left a comment

brian-smith-tcril Mar 18, 2025

adamstankiewicz Mar 18, 2025

		@use "@openedx/paragon/dist/core.min.css" as paragonCore;
		@use "@openedx/paragon/dist/light.min.css" as paragonLight;

[BD-46] feat: added support Paragon design tokens #351

[BD-46] feat: added support Paragon design tokens #351

Conversation

PKulkoRaccoonGang commented Jun 15, 2023 • edited Loading

openedx-webhooks commented Jun 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mashal-m commented Nov 17, 2023

PKulkoRaccoonGang commented Nov 17, 2023 • edited Loading

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril commented Mar 17, 2025

dcoa commented Mar 17, 2025

brian-smith-tcril commented Mar 18, 2025

brian-smith-tcril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PKulkoRaccoonGang commented Jun 15, 2023 •

edited

Loading

openedx-webhooks commented Jun 15, 2023 •

edited

Loading

PKulkoRaccoonGang commented Nov 17, 2023 •

edited

Loading