-
Notifications
You must be signed in to change notification settings - Fork 63
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: Using Paragon CSS External Hosting (JavaScript-based configuration) #726
base: master
Are you sure you want to change the base?
feat: Using Paragon CSS External Hosting (JavaScript-based configuration) #726
Conversation
refactor: code refactoring feat: necessary dependencies added refactor: corrected lint refactor: corrected sh refactor: added logs refactor: added temp frontend-platform dist refactor: updated run-build-for-gh-deps.sh refactor: corrected run-build-for-gh-deps.sh refactor: fixing the problem refactor: changed run build command feat: added header refactor: added dark theme and footer refactor: code refactoring refactor: code refactoring fix: fixed CommentsView button feat: added brand import feat: removed brand
Thanks for the pull request, @PKulkoRaccoonGang! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Sandbox deployment successful 🚀 |
Hi @brian-smith-tcril, it's because of the styles loaded by the dark theme if you change your device theme to light, the default paragon styles will be loaded |
Thank @PKulkoRaccoonGang, all the design token work is fantastic and looks good. 🥳 I noticed some changes that are not related to the design tokens but It's affecting the styles. These lines (from PR #697) override font-size 14px and 16px affecting header and footer, not only the main area This is how the dropdown is looking This is how supported to be (redwood) We can increase the specification of the rule to avoid affecting the header and footer, also we can use Also, this change made the submit button resize, which looks weird to me, I mean, the component usually has a weight resize when executing the summit (because the length of the text changes and an icon is added). However, due to the text reduction (but the icon maintains the original one) a height resize is added. scrnli_7_6_2024_12-00-00.PM.webm |
This is again not directly related to design tokens implementation, I'm not 100% sure if it's related to Paragon (that's why I highlight it) or the discussions implementation but the tabs course menu is not displaying the "more" button for responsive support during the first load, I have to resize the screen to make it work. scrnli_7_6_2024_12-46-06.PM.webmAnd again the changes from PR #697 are affecting the "more" button and the dropdown text size |
@brian-smith-tcril @dcoa thanks! @brian-smith-tcril we have prepared Paragon design tokens, which MFE Discussions consumes as the openedx theme. I added the brand-edx.org theme as a Is the brand-edx.org theme mandatory to release updated MFEs in Sumac?
For testing, I prepared an additional sandbox, which is based on the master branch of the current repository (without changes related to design tokens). Unfortunately, the problem you described relates to already existing changes that do not directly relate to design tokens. I suggest creating a corresponding issue in this repository.
I propose not to make changes/corrections related to the original styles of MFE Discussions. This approach will make it possible to provide for review only the diff that is associated with the adding functionality of the Paragon design tokens. @dcoa @brian-smith-tcril what are your thoughts? |
Thanks @PKulkoRaccoonGang, I agree with you. In terms of the design tokens implementation, I didn't find an issue. All looks as expected 🥳. |
Ah, yes, I have my system default set to dark mode so that makes sense. I'm curious as to how this would behave if we changed
to be
I'd like to ensure that people (like me!) that have dark mode as a system default don't experience broken themes. It's not a problem for a website to not have a dark theme, but it is a problem if |
Given |
@brian-smith-tcril, dark theme is optional we can use a definition that is module.exports = {
PARAGON_THEME_URLS: {
core: {
urls: {
default: 'https://cdn.jsdelivr.net/npm/@openedx/paragon@alpha/dist/core.min.css',
},
},
defaults: {
light: 'light',
},
variants: {
light: {
urls: {
default: 'https://cdn.jsdelivr.net/npm/@openedx/paragon@alpha/dist/light.min.css',
},
},
},
}; |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #726 +/- ##
=======================================
Coverage 92.88% 92.88%
=======================================
Files 160 160
Lines 3329 3329
Branches 888 898 +10
=======================================
Hits 3092 3092
+ Misses 219 218 -1
- Partials 18 19 +1 ☔ View full report in Codecov by Sentry. |
Sandbox deployment successful 🚀 |
Sandbox deployment successful 🚀 |
Sandbox deployment failed 💥 |
@PKulkoRaccoonGang - can this be closed, or is it still in progress? |
@mphilbrick211 this PR should stay open. The plan is to land it in a new long living branch for design tokens once it is updated to use a published version of Paragon that supports design tokens (which is getting really close!) |
Description
The JavaScript-based configuration approach allows the user to add Paragon CSS from external hosting.
Info
Related PRs
ParagonWebpackPlugin
to support design tokens frontend-build#546