Bug for single character colors in toHex() #1

Merged
merged 3 commits into from Nov 14, 2016

Projects

None yet

3 participants

@pypmannetjies
Contributor

Hi,

I found an issue where, if a color was calculated and the result was a single character, the toHex() function would duplicate the character instead of prepending a 0.

So for example:
rgb(8,8,8) resolves to #888888 instead of #080808

I've edited the toHex() function to prepend a zero instead.

@pypmannetjies pypmannetjies Bug for single character colors in toHex()
Hi,

I found an issue where, if a color was calculated and the result was a single character, the toHex() function would duplicate the character instead of prepending a 0. 

So for example: 
rgb(8,8,8) resolves to #888888 instead of #080808

I've edited the toHex() function to prepend a zero instead.
d24b110
@kcbleeker

This looks like a good idea!

@Olical
Owner
Olical commented Nov 14, 2016

Wow, this repo is a blast from the past! Past me was a fool (just like present me!) and didn't write any tests, so this feels like a change that will break things. This was from when I was first learning to write JavaScript in a way that I could give it to other people, feels so weird to read, like I didn't even write it!

Useless comments and imperative loops everywhere. 😱 Also, single-quotes are used everywhere else. As long as you're confident that my original code was just completely broken and this fixes it, I'm happy to merge.

@Olical

Just that one tiny quote change.

color.js
@@ -90,7 +90,7 @@ var color = {
// Make sure it is always the right length
if(color[i].length === 1) {
- color[i] = color[i] + color[i];
+ color[i] = "0" + color[i];
@Olical
Olical Nov 14, 2016 Owner

Swap these to single quotes and I think this is fine. I think.

@pypmannetjies
pypmannetjies Nov 14, 2016 Contributor

Ahaha yeah I saw you wrote this 6 years ago! I'll submit another pull request later today with the min file as well.

@Olical
Olical Nov 14, 2016 Owner

Feel free to modify this PR :) you should be able to add commits to it. Gonna write a post about what it feels like when some of your earliest stuff pops up out of nowhere :D I'll probably use this as a prompt to clean this project up, might as well.

Thanks!

@pypmannetjies
Contributor

Should be good now

@Olical
Olical approved these changes Nov 14, 2016 View changes

Nice work. I'll comment here when I've finished my little cleanup post too :)

@Olical Olical merged commit 5e8d12b into Olical:master Nov 14, 2016
@pypmannetjies
Contributor

Thanks!

@pypmannetjies pypmannetjies deleted the pypmannetjies:patch-1 branch Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment