Skip to content
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

chore: Add transform to handle opacity as percentage #100

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

RayRedGoose
Copy link
Contributor

Issue

Summary

PR adds transform to handle opacity as a percentage out of 100.

Release Category

Infrastructure

@RayRedGoose RayRedGoose self-assigned this Apr 1, 2024
type: 'value',
transitive: true,
matcher: filter.isBaseOpacity,
transformer: ({value}) => `${value / 100}`,
Copy link
Member

Choose a reason for hiding this comment

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

I always forget that JS is cool with doing math on string numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if it's only numbers, no letters lol

Copy link
Member

@alanbsmith alanbsmith left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, @RayRedGoose!


export const isBaseOpacity: Matcher = token => {
const [level, category] = token.path;
return level === 'base' && category === 'opacity' && parseFloat(token.value) > 1;
Copy link
Member

@alanbsmith alanbsmith Apr 1, 2024

Choose a reason for hiding this comment

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

> 1 is a very smart safeguard. 👍🏼

@RayRedGoose RayRedGoose added the automerge Automatically merge pull request label Apr 1, 2024
@alanbsmith alanbsmith merged commit feaf7ba into Workday:main Apr 1, 2024
9 checks passed
@RayRedGoose RayRedGoose deleted the add-opacity-transform branch September 9, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants