Added out-of-bounds handling for hslToRgb function with unit tests #193
Conversation
|
Thanks Matt! Will review it tomorrow (Wed).
|
| @@ -4,8 +4,25 @@ module.exports = function HSLToHex(data) { | |||
| var hslToRgb = function(h, s, l) { | |||
| var r, g, b; | |||
|
|
|||
| // normalize orientation b/w 0 and 360 degrees | |||
| h = h % 360; | |||
| if (h < 0) { | |||
There was a problem hiding this comment.
@MWers We don't use curly braces with a single statement. Please remove them to keep our style consistent.
|
Beside that one comment everything is fine. 👍 |
|
No problem, 'tis fixed! |
| @@ -4,8 +4,23 @@ module.exports = function HSLToHex(data) { | |||
| var hslToRgb = function(h, s, l) { | |||
| var r, g, b; | |||
|
|
|||
| // normalize hue orientation b/w 0 and 360 degrees | |||
| h = h % 360; | |||
| if (h < 0) h += 360; | |||
There was a problem hiding this comment.
Sorry, wasn't specific enough - we prefer the following style:
if (h < 0)
h += 360;
Two more things:
- There are other cases below where braces should be gone.
- Please rebase all commits into one once you are done!
There was a problem hiding this comment.
Added a missing semicolon - make sure to run npm run check before squashing too.
|
Fixed and (hopefully properly) rebased! ;) |
|
@MWers Looks great - merging right away! |
Adds out-of-bounds handling for HSL colors.
|
@MWers 2.0.4 is out with your fix! |
|
Great, thanks!! |
I've added out-of-bounds handling for the
hslToRgbfunction. It normalizes the hue orientation between 0 and 360 degrees and ensures that saturation and lightness are between 0 and 100. I based tests of this functionality on the Webkit CSS tests here:http://trac.webkit.org/export/98524/trunk/LayoutTests/fast/css/hsl-color.html