Skip to content

Ensure we migrate theme(spacing.1) to var(--spacing-1) correctly#14723

Closed
RobinMalfait wants to merge 1 commit into
nextfrom
robin/ensure_we_migrate_theme_spacing.1_correctly
Closed

Ensure we migrate theme(spacing.1) to var(--spacing-1) correctly#14723
RobinMalfait wants to merge 1 commit into
nextfrom
robin/ensure_we_migrate_theme_spacing.1_correctly

Conversation

@RobinMalfait

Copy link
Copy Markdown
Member

This PR fixes an issue where theme(…) calls that contain a .1 weren't correctly converted to var(--spacing-1). The reason for this is that .1 has some special meaning in cases like fontSize.xs.1.lineHeight where it should be converted to --font-size-xs--line-height, not --font-size-xs-1-line-height.

To solve this, we make sure to only apply the -- check if the 1 occurs somewhere in the middle instead of at the very end.

With this change, the following migrations will happen correctly:

- [--value:theme(spacing.1)]
+ [--value:var(--spacing-1)]
- [--value:theme(fontSize.xs.1.lineHeight)]
+ [--value:var(--font-size-xs--line-height)]

@RobinMalfait RobinMalfait requested a review from a team as a code owner October 18, 2024 19:54

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @RobinMalfait and the rest of your teammates on Graphite Graphite

@RobinMalfait RobinMalfait changed the base branch from robin/simplify_candidate_printing to next October 18, 2024 19:55
`theme(spacing.1)` wasn't converted because we generated `--spacing-`
and that doesn't exist therefore we kept it as-is.
@RobinMalfait RobinMalfait force-pushed the robin/ensure_we_migrate_theme_spacing.1_correctly branch from 73d0024 to aa19114 Compare October 18, 2024 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant