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

Add specs for passing non-deg units to hwb's hue #1806

Merged
merged 2 commits into from
Jul 19, 2022
Merged

Add specs for passing non-deg units to hwb's hue #1806

merged 2 commits into from
Jul 19, 2022

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Jul 19, 2022

See #1607

[skip dart-sass]

@nex3 nex3 requested a review from Goodwine July 19, 2022 00:15
Soon, it will instead be correctly converted to 57.2957795131deg.

To preserve current behavior: $hue * 1deg/1rad
To migrate to new behavior: 0deg + $hue
Copy link
Member

Choose a reason for hiding this comment

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

Does 0dev + $hue mean that users will have to always do this until dart-sass 2.0.0?
I wonder if we should give another alternative like doing the calculation to deg and saying

To migrate to new behavior, either:
 - force-cast to deg units with: 0deg + $hue
 - manually update to deg units: 57.3 deg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we release Phase 2 non-deg angles will be automatically converted to degrees. 0deg + $hue is just a stop-gap measure to force the conversion before Phase 2 is released.

I'm always wary about suggesting literal values in these, because I expect a lot of the time the argument comes from something that is itself dynamic and so can't be reliably replaced by a single value.

@nex3 nex3 merged commit 0e69886 into main Jul 19, 2022
@nex3 nex3 deleted the hwb-hue-unit branch July 19, 2022 00:57
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.

None yet

2 participants