Skip to content

feat(tokens): create variables fallbacks based on its alias token #30404

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

Merged
merged 5 commits into from
May 14, 2025

Conversation

BenOsodrac
Copy link
Contributor

@BenOsodrac BenOsodrac commented May 9, 2025

Issue number: internal


What is the current behavior?

Currently, even if a token used an alias token as its value, when we're generating the variables we wouldn't respect this connection and were simply adding the fixed final value.

What is the new behavior?

  • Now when a token uses an alias token, we keep that relationship by making sure the alias variable is used as fallback.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@BenOsodrac BenOsodrac added type: feature request a new feature, enhancement, or improvement package: core @ionic/core package labels May 9, 2025
Copy link

vercel bot commented May 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2025 2:13pm

@BenOsodrac BenOsodrac marked this pull request as ready for review May 9, 2025 16:20
@BenOsodrac BenOsodrac requested a review from a team as a code owner May 9, 2025 16:20
@BenOsodrac BenOsodrac requested review from ShaneK and thetaPC and removed request for ShaneK May 9, 2025 16:20
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor suggestion but not a blocker

Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have a question (repeatedly asked but the same essential question) about the fallback for the rgb variables.

$ion-border-radius-800: var(--ion-border-radius-800, $ion-scale-800);
$ion-border-radius-1000: var(--ion-border-radius-1000, $ion-scale-1000);
$ion-border-radius-full: var(--ion-border-radius-full, 999px);
$ion-border-radius-025: var(--ion-border-radius-025, $ion-scale-050);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of unrelated to the PR but why is this one not after border-radius-0? And also why does it jump from 0 -> 25 -> 100

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe its due to the order on the actual JSON, as these were added later by the UX team

@BenOsodrac
Copy link
Contributor Author

I just have a question (repeatedly asked but the same essential question) about the fallback for the rgb variables.

I'm preventing the generation of fallbacks for rgb variables. Mainly as it would add a little bit more code to achieve that and also rgb are only generated for internal use, they don't come from actual rgb tokens, so I thought it wouldn't bring as much value, as its lees likely devs will customize these.

But we can always return to this in the future, if we see it makes sense to add.

@BenOsodrac BenOsodrac requested a review from brandyscarney May 14, 2025 09:50
@BenOsodrac BenOsodrac merged commit f7a2d34 into next May 14, 2025
48 checks passed
@BenOsodrac BenOsodrac deleted the ROU-11898 branch May 14, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants