-
Notifications
You must be signed in to change notification settings - Fork 23
Compile using Paragon 23 with the dark theme included in the light #43
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
Compile using Paragon 23 with the dark theme included in the light #43
Conversation
themes/dark/_utilities.scss
Outdated
| @if type-of($variant) == "number" { | ||
| $color-level: $variant; | ||
| } | ||
| @import "~@openedx/paragon/styles/scss/core/core.scss"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should just import the specific mixins we use here. Also, would core.scss be a better place to import this in case we end up needing these mixins in any other style sheet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’re right, I got the wrong file. I somehow thought it was a Paragon mixin defined there, but it seems to come from ~bootstrap/scss/buttons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve replaced this import with @import "~@openedx/paragon/dist/Button/mixins" in core.scss.
| "build-scss": "paragon build-scss --corePath ./paragon/core.scss --themesPath ./paragon/build/themes", | ||
| "build": "make build", | ||
| "build:watch": "nodemon --ignore dist -x \"make build\"", | ||
| "serve": "paragon serve-theme-css -h 0.0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why do we need the serve script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For testing, to serve the built CSS files so that the MFE can load them from MFE_CONFIG["PARAGON_THEME_URLS"].
|
Thanks, Olivier! I appreciate the PR. I'll build it locally to test the changes before we proceed. |
…hadow combined variables.
…d missing imports.
fe47a46 to
f0e9839
Compare
…import Paragon core anymore.
|
Tested these changes on my local instance. They seem to work fine. Let's merge this. |
Sorry for not providing a description earlier, I had to run to catch my train, and I understand there’s a deadline for the Ulmo release process, so I wanted my work to be available as soon as possible.
This PR gets the brand package to compile with Paragon 23, allowing to use it with the
MFE_CONFIG["PARAGON_THEME_URLS"]setting, which allows to change the MFE theme without rebuilding the Docker image.It’s based on @ahmed-arb’s
ahmed-arb/ulmo-upgradebranch, and inspired by hisahmed-arb/test-design-tokensbranch, which helped me get started.I think it’s not technically speaking “using design tokens” (I can’t tell for sure because I had no experience with Paragon or design tokens until a week ago,) because it’s still the same SCSS customisation as before, with SCSS variables for colours replaced by CSS variables. However, these CSS variables are defined in a token file, so it should simplify theming.
Since Indigo’s dark theme is based on the them toggle setting the
indigo-dark-themeclass on<body>(as opposed to Paragon 23’s variants CSS switching, which I wouldn’t know how to achieve with the current toggle switch,) and the dark theme SCSS being very different from the light (and also my partial understanding of theme variants, which seem to need redefining A LOT of things as soon as they’re not calledlight, because everything that Paragon’s light theme handily defines is lost) this version provides only one variant calledlight, but which switched to dark mode when theindigo-dark-themeclass is set on<body>. To achieve that, I added adarkbranch in thecolor.jsonfile, that I use to redefine the main CSS variables inbody.indigo-dark-theme, just like it was done in SCSS in the previous version. That’s why I call it “integrating the dark theme hackily.”I hope this helps. I would welcome feedback, as I’d be interested to learn to do make this more properly in the future, with design tokens and theme variants.