fix(tokens): remove style dictionary scripts and reuse os tokens logic #30786
Conversation
BenOsodrac
added
package: core
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BenOsodrac
requested review from
brandyscarney
and removed request for
thetaPC
thetaPC
left a comment
thetaPC
left a comment
There was a problem hiding this comment.
The foundations/README.md also needs to be updated to exlcude ionic.utility.scss since that file isn't being generated anymore.
There was a problem hiding this comment.
Revert this deletion. We use it to test on ios and md.
core/package.json
Outdated
| "jest": "^29.7.0", | ||
| "jest-cli": "^29.7.0", | ||
| "outsystems-design-tokens": "^1.3.2", | ||
| "outsystems-design-tokens": "1.3.3", |
There was a problem hiding this comment.
Why are we pinning the version?
There was a problem hiding this comment.
I think it won't matter, as Ionic will no longer consume os-tokens in the near future. But still, removed the pin for now :)
There was a problem hiding this comment.
No worries, if there's a good reason to pin then I'm not against it. Just wanted to keep record in case we should be aware of something.
BenOsodrac
changed the title
brandyscarney
left a comment
brandyscarney
left a comment
There was a problem hiding this comment.
I'm approving this based on the changes but please review my comments as some of the screenshot diffs look wrong and will likely need to be addressed. 👍
There was a problem hiding this comment.
We should consider using the existing Ionic CSS utility classes to get the ion-margin styles here (and in others).
There was a problem hiding this comment.
We should consider using the existing Ionic CSS utility classes to get the ion-padding styles here (and in others).
There was a problem hiding this comment.
The CSS utilities would be especially useful for this screenshot - you can't see the entire focused border anymore without the ion-padding styles:
`<div id="container" class="ion-padding">
<ion-chip class="ion-focused">
<ion-label>Focused</ion-label>
</ion-chip>
</div>`,
There was a problem hiding this comment.
This letter spacing is a bit weird - not sure if intentional
There was a problem hiding this comment.
Same weirdness with letter spacing here as in segment button
There was a problem hiding this comment.
Button letter-spacing is a bit weird now - is this intentional?
Issue number: internal
What is the new behavior?
This pull request updates the design token build system and its dependencies, simplifying the process and removing custom scripts in favor of using the latest features of the
outsystems-design-tokenspackage. The changes modernize how design tokens are generated and maintained, reducing custom code and leveraging upstream improvements.Key changes include:
Build Process Simplification:
core/scripts/tokens/index.mjsandcore/scripts/tokens/utils.mjs) have been removed. Token generation is now handled directly via theoutsystems-design-tokenspackage using annpxcommand in thebuild.tokensscript. This reduces maintenance overhead and keeps the build process aligned with upstream best practices. [1] [2] [3]Dependency Updates:
outsystems-design-tokensto version1.3.3andstyle-dictionaryto version5.1.1in bothpackage.jsonandpackage-lock.json. This ensures compatibility with the latest features and bug fixes and removes the need to specifystyle-dictionarydirectly as a dependency. [1] [2] [3] [4]SCSS Utility Import:
@forward "../../foundations/ionic.utility";statement was removed fromcore/src/css/ionic/utils.bundle.ionic.scss, likely because the utility SCSS is now generated and managed by the new token build process.Removed CSS tests for Ionic theme, as they relied heavily on testing the effect of the utility-classes, that are no longer created on the ionic scope.
Does this introduce a breaking change?