-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Issue-2464: Add color luminosity options #2566
Issue-2464: Add color luminosity options #2566
Conversation
So we can generate lighter and darker colors as requested in the issue: faker-ruby#2464. Next step is to convert a light/dark hsl to hex.
So we can easily specify when we want light or dark colors, since hsl values have a very intuitive way to specify light and dark colors. See: faker-ruby#2464
New param allows user to pass in :light for light hex color and :dark for dark hex colors. Additional specs confirm that this works. See: faker-ruby#2464
lib/faker/default/color.rb
Outdated
def hex_color | ||
format('#%06x', (rand * 0xffffff)) | ||
def hex_color(lightness = nil) | ||
lightness_lookup = { light: 0.8, dark: 0.2 } |
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.
Might become a constant?
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.
A constant for this would be good
This isn't the prettiest code in the world but it's a code snippet I use to determine if a color should get white or black text based on whether or not the background is dark or light: def contrast_color(hex)
yiq_threshold = 128
# TODO: This doesn't account for 3 character hex codes.
red, green, blue = hex.scan(/../).map{ |c| c.to_i(16) }
yiq = ((red * 299) + (green * 587) + (blue * 114)) / 1000
yiq > yiq_threshold ? "#000" : "#fff"
end It's a well known algorithm https://24ways.org/2010/calculating-color-contrast/ , here's a JS demo: http://jsfiddle.net/decx/RRt3q/. It could be useful to help test this PR because a dark color should always produce contrasted white text. |
The interesting thing is that not all light colors contrast well with black for example. So perhaps the question here is if we want a color that contrasts with white or black? |
Text size does play a role here. What size were you testing it on? If you use a contrast checker such as https://webaim.org/resources/contrastchecker/, having black text with a Personally on both my monitors (IPS displays) the black text is much easier to read. White text fails the contrast test checker. |
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.
Just a few minor changes and LGTM
lib/faker/default/color.rb
Outdated
def hex_color | ||
format('#%06x', (rand * 0xffffff)) | ||
def hex_color(lightness = nil) | ||
lightness_lookup = { light: 0.8, dark: 0.2 } |
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.
A constant for this would be good
This is just a thought but what do you think about introducing a way to let users customize the lightness? For example, let's say I want to generate dark colors but Something like being able to do |
Adding specs to confirm this works, and also prevents specifying invalid hsl colors. @see faker-ruby#2566 (comment)
So it cash be used to pass values (such as hue and saturation) to hsl_color. See: faker-ruby#2566 (comment)
To make it more general. @see faker-ruby#2566 (comment)
So clients are able to generate very specific colors when needed. See: faker-ruby#2566 (comment)
Hey @Zeragamba and @nickjj I have pushed some updates to address your feedback. Please let me know what you think or if you have concerns with the implementation. Thank you! |
Based on the documented example usage this looks great, thanks for making it so customizable. This will let folks pick any type of variety category they prefer. There's also neat options that we can do now like: |
Tests be passin, I be sayin LGTM, and we be mergin. Thanks for the feature! |
* Add ability to override hsl_color lightness So we can generate lighter and darker colors as requested in the issue: faker-ruby#2464. Next step is to convert a light/dark hsl to hex. * Use hsl_color and hsl_to_hex to generate hex_color So we can easily specify when we want light or dark colors, since hsl values have a very intuitive way to specify light and dark colors. See: faker-ruby#2464 * Add ability to specify light or dark hex color New param allows user to pass in :light for light hex color and :dark for dark hex colors. Additional specs confirm that this works. See: faker-ruby#2464 * Update version to next As per: https://github.com/faker-ruby/faker/blob/master/CONTRIBUTING.md#documentation * Fix rubocop linting issue * Allow specific hue and saturation values for hsl_color Adding specs to confirm this works, and also prevents specifying invalid hsl colors. @see faker-ruby#2566 (comment) * Add LIGHTNESS_LOOKUP constant @see faker-ruby#2566 (comment) * Introduct hsl_hash to hex_color So it cash be used to pass values (such as hue and saturation) to hsl_color. See: faker-ruby#2566 (comment) * Rename lightness to args To make it more general. @see faker-ruby#2566 (comment) * Allow hue, saturation or lightness for hex colors So clients are able to generate very specific colors when needed. See: faker-ruby#2566 (comment) * Fix documentation typo
Summary
Closes: #2464
We added 3 new config options.
light
ordark
a hex color.hue
,saturation
orlightness
when generating a hex color.hue
,saturation
orlightness
when generating an HSL color.The change to the HSL color generator enables us to easily generate a light / dark color since the lightness (In HSL) directly affect the color's luminosity.
We added a private method (not directly tested) that converts and HSL value into a HEX color value.
The conversion algorithm was heavily inspired by:
The original issue suggested we implement it as follows:
Faker::Color.hex_color(luminosity: 'dark')
Faker::Color.hex_color(luminosity: 'light')
Instead I implemented with a slightly less verbose syntax, but I am open to changing it.
Faker::Color.hex_color(:dark)
-> generates a color with lightness set to 20%Faker::Color.hex_color(:light)
-> generates a color with lightness set to 80%A side effect of this change is that you can now also (optionally) provide a specific HSL values when generating a hex or hsl color.
Faker::Color.hex_color(hue: 100, saturation: 0.52, lightness: 0.3)
will generate the specified color.Faker::Color.hsl_color(hue: 100, saturation: 0.52, lightness: 0.3)
will generate the specified color.Other Information
If you would like to manually verify that the generated color is light/dark.
puts @tester.hex_color(:light)