-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
feat: Add expanded support for CSS Colors #34600
feat: Add expanded support for CSS Colors #34600
Conversation
Base commit: 37d0a25 |
Base commit: 37d0a25 |
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.
We should add tests for rgba(255 255 255 / 0.5)
syntax, and for space-separated hsl
, hsla
, etc.
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.
See comment from @necolas
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Sure, I've added a new regex so we can support the |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was successfully merged by @gabrieldonadel in ac1fe3b. When will my fix make it into a release? | Upcoming Releases |
Summary: ## Summary This PR adds expanded support for CSS Colors, as requested on facebook#34425. It updates the current regex used to match the functional notation for colors, e.g `rgb()` to accept space as a valid separator between values as specified on [CSS Color Module Level 4](https://www.w3.org/TR/css-color-4/) definition. This also adds support for the `hwb` notation. ## Changelog [General] [Added] - Add expanded support for CSS Colors Pull Request resolved: facebook#34600 Test Plan: We can test different color scenarios through the new test cases added to the already existing normalizeColor-test.js Reviewed By: necolas Differential Revision: D39269360 Pulled By: cipolleschi fbshipit-source-id: 449158d17256bbab8bd9fd0da29245660225fb92
Summary: In the last diff I mixed and matched `<legacy-rgb-syntax>` and `<modern-rgb-syntax>` a bit to keep compatiblity with `normalze-color`. Spec noncompliant values have only been allowed since facebook#34600 with the main issue being that legacy syntax rgb functions are allowed to use the `/` based alpha syntax, and commas can be mixed with whitespace. This seems like an exceedingly rare real-world scenario (there are currently zero usages of slash syntax in RKJSModules validated by `rgb\([^\)]*/`), so I'm going to instead just follow the spec for more sanity. Another bit that I missed was that modern RGB functions allow individual components to be `<percentage>` or `<number>` compared to legacy functions which only allow the full function to accept one or the other (`normalize-color` doesn't support `<percentage>` at all), so I fixed that as well. I started sharing a little bit more of the logic here, to make things more readable when adding more functions. Changelog: [Internal] Differential Revision: D68468275
Summary
This PR adds expanded support for CSS Colors, as requested on #34425. It updates the current regex used to match the functional notation for colors, e.g
rgb()
to accept space as a valid separator between values as specified on CSS Color Module Level 4 definition. This also adds support for thehwb
notation.Changelog
[General] [Added] - Add expanded support for CSS Colors
Test Plan
We can test different color scenarios through the new test cases added to the already existing normalizeColor-test.js