Update CSS token per new design system theming capability #16
Conversation
fajar-apri-alaska
force-pushed
the
fajarapri/updateCSSToken/#15
branch
2 times, most recently
from
05d67bc to
ce74805
Compare
- fix the postCSS script to only use newest WC-standard - remove unused npm packages - keep the old legacy tokens for now - fix the test script Changes to be committed: modified: demo/index.html modified: demo/style.scss modified: package-lock.json modified: package.json modified: scripts/postCss.js deleted: scripts/removeNonRemPlugin.js deleted: src/style-fixed.css.map deleted: src/style-fixed.map deleted: src/style-fixed.scss modified: src/style.css.map modified: src/style.scss
fajar-apri-alaska
force-pushed
the
fajarapri/updateCSSToken/#15
branch
from
ce74805 to
59539a7
Compare
| <script src="https://cdn.jsdelivr.net/npm/marked/marked.min.js"></script> | ||
| <script src="https://cdn.jsdelivr.net/npm/prismjs/prism.js"></script> | ||
| <script> | ||
| // import 'https://unpkg.com/marked@latest/marked.min.js'; | ||
| // import 'https://unpkg.com/prismjs@latest/prism.js'; |
There was a problem hiding this comment.
@jason-capsule42 and @fajar-apri-alaska not sure why this was passed though? Why were lines 38 and 39 commented out and 35 and 36 added? This is not how we have things laid out in the generator.
There was a problem hiding this comment.
I think this one was my bad, I should just add <script type=module> instead of did above changes. I forgot that this changes is not mandatory, just the marked.parse part that is.
| "@alaskaairux/design-tokens": "^3.15.5", | ||
| "@aurodesignsystem/design-tokens": "^4.2.0", |
There was a problem hiding this comment.
If the new tokens were added and the code updated to support the --ds namespace, why is the legacy library still installed?
There was a problem hiding this comment.
oh for this one I follow this hyperlink feedback, a need to keep the legacy tokens around for the time being.
AlaskaAirlines/auro-hyperlink#185 (comment)
FYI I did this for the other currently open PR's, so if this is not needed, then I should remove those as well in the other components.
There was a problem hiding this comment.
Oh... that may be a misunderstanding. We do not need to keep the package dependency if both tokens and WCSS is updated and there is no other internal dependency to the element that needs this.
But there are external peer dependencies that need the legacy tokens, so all we need to do is keep that on the ./demo/index.html page.
| await expect(el.shadowRoot.innerHTML).contains(`--insetSm:var(--inset)`); | ||
| await expect(el.shadowRoot.innerHTML).contains(`--insetMd:var(--insetSm)`); | ||
| await expect(el.shadowRoot.innerHTML).contains(`--insetLg:var(--insetMd)`); | ||
| await expect(el.shadowRoot.innerHTML).contains(`--inset: var(--auro-size-none)`); |
There was a problem hiding this comment.
Shouldn't this have been updated to use a --ds value?
There was a problem hiding this comment.
I am not sure about this, it does refer to this render function though, should I make the changes on this also?
|
I will create another PR that will address above feedbacks since this one is already merged. |
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Alaska Airlines Pull Request
Fixes: #15
Summary:
Type of change:
Please delete options that are not relevant.
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team