feat(getLuminance): Add getLuminance function #163
Conversation
Codecov Report
@@ Coverage Diff @@
## master #163 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 68 69 +1
Lines 386 388 +2
Branches 102 102
=====================================
+ Hits 386 388 +2
Continue to review full report at Codecov.
|
|
Thanks for the PR @jcquinlan. I definitely think we should at least add some sort of guard type functionality http://lesscss.org/features/#mixin-guards-feature that allows us to check the lightness or darkness of a color. There are also things we can do related to contrast here that would also be useful: https://codepen.io/giana/project/full/ZWbGzD But I agree, |
|
Awesome to see this in a PR! I was thinking about it, and do you think it should be called |
|
@souporserious Hey sorry I ripped off your idea! I was bored at a conference and wanted to do some coding, I hope you don't mind! As per the suggestion of @bhough it might be a good idea to use this as a stepping stone to creating some of the guards and more full fleshed-out brightness tools that exist elsewhere. |
|
Oh no, not at all! I'm glad someone found the time to implement it 🙂 |
|
Other than the conflicting files, is there anything in particular that is holding this up? |
|
@wtgtybhertgeghgtwtg it needs several things:
|
There was a problem hiding this comment.
Couple of other misc. comments:
-
You will also need to add this to https://github.com/styled-components/polished/blob/master/src/index.js so it can be properly imported using destructured imports. (I always forget to do this).
-
I would love if we can replace the formula in readableColor with this helper for consistency's sake: https://github.com/styled-components/polished/blob/master/src/color/readableColor.js
Otherwise looking good, and definitely want to get this merged in as soon as it is ready.
| // @flow | ||
| import getLuminance from '../getLuminance' | ||
|
|
||
| describe('getLuminance', () => { |
There was a problem hiding this comment.
Would like to see the tests for all color spaces if possible (rgb, hsl, hsla, and a named CSS color) if we could.
src/color/getLuminance.js
Outdated
| return (0.2126 * r) + (0.7152 * g) + (0.0722 * b) | ||
| } | ||
|
|
||
| export default curry(getLuminance) |
There was a problem hiding this comment.
This will need to be updated based on #204
- export default curry(getLuminance)
+ // Don’t inline this variable into export because Rollup will remove the /*#__PURE__*/ comment
+ const curriedGetLuminance = /*#__PURE__*/ curry(getLuminance) // eslint-disable-line spaced-comment
+ export default curriedGetLuminance
src/color/getLuminance.js
Outdated
| import curry from '../internalHelpers/_curry' | ||
|
|
||
| /** | ||
| * Returns a number representing the luminance of a color. |
There was a problem hiding this comment.
Maybe clarify that this is a percentage (float)?
README.md
Outdated
| @@ -146,6 +146,7 @@ In the documentation you will see examples using [object spread properties](http | |||
| <li><a href="http://polished.js.org/docs/#rgbacolor">RgbaColor</a></li> | |||
| <li><a href="http://polished.js.org/docs/#rgbcolor">RgbColor</a></li> | |||
| <li><a href="http://polished.js.org/docs/#timingfunction">TimingFunction</a></li> | |||
| <li><a href="http://polished.js.org/docs/#getluminance">getLuminance</a></li> | |||
There was a problem hiding this comment.
This is getting put in the wrong spot, due to it missing from documentation.json. If you add it, this should get inserted into the proper place in the readme and the documentation.
|
@jcquinlan I was just starting to look at this myself, so I already merged the rebase with master into your fork 😎 |
Closes #148
At least partially, as it allows the user to get the value, but doesn't have an accompanying setter to change the luminance. It seemed like this was already being handled by other methods.
I feel like to really make this useful, another method would need to allow a user to compare the contrast ratio of two colors and return a boolean stating whether or not it adheres to the W3C specs.