-
Notifications
You must be signed in to change notification settings - Fork 148
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
[FEATURE] Support arithmetic operators in CSS functions #607
Conversation
This is the enhancement from #390.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good all in all. Only one nitpick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for picking this up! ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, no, stop the press: We still need a changelog entry for this new feature.
I've added strict types declaration to the new (test) source file. @sabberworm, can you give this a quick look to confirm you think it's OK for now. Longer term, I think it would be cleaner to introduce a There is calling for a release with this in ASAP, at least as far as CSS3, hence I think this interim solution will suffice, as full CSS4 support may take some time and effort. |
- Correct nomenclature in datum name for numeric part of `length`.
Indeed we do. It's lack was a deliberate alertness test ;) Also corrected nomenclature of the numeric part of a length value (MDN calls it a 'number' not a 'value'). |
`calc` was already supported.
I've updated this again. I noted that arithmetic in |
Just re-spotted that #389 seeks to do just that. Though it is a PR on top of a PR, so may be somewhat awkward to unravel. |
Yes, absolutely. I'd also like to check our class naming and structure against the terms and concepts used in the the official specification. |
I'm finding a very close if not exact match there. Kudos to @sabberworm. But, yes, please do review. |
Looking at that, the new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
… but not as part of this PR. :-) |
Closes #619. The test passes, whereas without the change from #607, it does not. Co-authored-by: Jake Hotson <[email protected]>
This is the enhancement from #390. `calc` was already supported.
This is the enhancement from #390. `calc` was already supported.
This is the enhancement from #390.