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 support for space-separated RGB #58

Merged
merged 5 commits into from
Nov 26, 2021
Merged

Conversation

JJC1138
Copy link
Contributor

@JJC1138 JJC1138 commented Nov 5, 2021

This is almost exactly the same as @htunnicliff's excellent PR #55, but just for the RGB case instead of HSL. It adds support for values like:

rgb(255 255 255)
rgb(255 255 255 / 1)

The motivation for this was that Chrome's developer tools now uses that form of notation for RGB colors, and I got bitten by blindly copying and pasting one into my CSS 😅

@clytras
Copy link
Contributor

clytras commented Nov 25, 2021

Thank you @JJC1138 for the awesome work!

Does the RE support alpha percentage like rgb(0 255 38 / 7%)?

@Qix- please merge and release, we need this!

@JJC1138
Copy link
Contributor Author

JJC1138 commented Nov 25, 2021

Does the RE support alpha percentage like rgb(0 255 38 / 7%)?

No, I'm afraid not. I thought that was disallowed by the spec, but I see was wrong 😣 Sorry about that. I'm not using the library in my project anymore so I don't have an immediate need to work on that myself. You're very welcome to fork my branch to add support, of course.

@clytras
Copy link
Contributor

clytras commented Nov 25, 2021

@JJC1138 I just fix it to work with alpha percentage and added more tests. Everything passes and it works.

Please check this PR JJC1138#1 and merge.

@JJC1138
Copy link
Contributor Author

JJC1138 commented Nov 25, 2021

Done. Thanks very much!

@Qix- Qix- force-pushed the rgb-space-notation branch 2 times, most recently from ccbea79 to a6c9763 Compare November 26, 2021 07:26
@Qix- Qix- force-pushed the rgb-space-notation branch from a6c9763 to a2ea739 Compare November 26, 2021 07:28
@Qix- Qix- merged commit f5fe5a8 into Qix-:master Nov 26, 2021
@Qix-
Copy link
Owner

Qix- commented Nov 26, 2021

Thanks :)

@Qix-
Copy link
Owner

Qix- commented Nov 26, 2021

Published as 1.7.0.

@clytras
Copy link
Contributor

clytras commented Nov 26, 2021

You're very welcome @Qix- , thank you for making this!

Now that I'm looking at it again, I think I should have used parseFloat for the percentage instead of parseInt. The spec says that it's a number, not an integer, thus it may have floating point numbers. What do you think?

@Qix-
Copy link
Owner

Qix- commented Nov 26, 2021

PR welcome. :)

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.

3 participants