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

feat: color module #801

Merged
merged 82 commits into from May 22, 2022
Merged

Conversation

harsohailB
Copy link
Contributor

@harsohailB harsohailB commented Apr 7, 2022

Resolves:

#286

Key Changes:

  • Migrate color API from commerce to its own color module
  • Deprecate color method in commerce module (since v7 until v8)
  • Add random RGB generation and tests (Red Green Blue)
  • Add random RGBA generation and tests (Red Green Blue Alpha)
  • Add random CMYK generation and tests (Cyan, Magenta, Yellow, and Key (black))
  • Add random HSL generation and tests (Hue, Saturation, Lightness)
  • Add random HSLA generation and tests (Hue, Saturation, Lightness, Alpha)
  • Add random HWB generation and tests (Hue, Whiteness, Blackness)
  • Add random LAB generation and tests (Lightness, followed by the A-axis (green to red) and B-axis (blue to yellow))
  • Add random LCH generation and tests (Lightness, Chroma, and Hue)
  • Migrate existing tests from commerce.spec.ts to color.spec.ts

Add support for color gamuts: (see those supported by CSS media queries here)

  • sRGB
  • display-p3
  • rec2020
  • Add random space generation and tests (color space list here)

Add CSS support: (perhaps in options have { format: 'css' })

  • RGB - rgb(255, 0, 0) (covers sRGB gamut for CSS)
  • RGBA - rgba(255, 0, 0, 0.5)
  • HSL - hsl(0deg 100% 80%)
  • HSLA - hsl(0deg 100% 50% / 0.5)
  • HWB - hwb(194 0% 0%)
  • CMYK - cmyk(100%, 0%, 0%, 0%)
  • LAB - lab(29.2345% 39.3825 20.0664)
  • LCH - lch(52.2345% 72.2 56.2)
  • display-p3 - color(display-p3 0 1 0)

Design Decisions: (WIP)

  • Combine hex and numeric methods and provide the option in args
  • Include binary as a return format

CSS Colors Reference

Modern CSS Colors

@import-brain import-brain added c: feature Request for new feature p: 1-normal Nothing urgent labels Apr 8, 2022
@import-brain import-brain added this to the v6.2 - New small features milestone Apr 8, 2022
@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #801 (61c2bc2) into main (bd4d3db) will decrease coverage by 0.00%.
The diff coverage is 98.04%.

@@            Coverage Diff             @@
##             main     #801      +/-   ##
==========================================
- Coverage   99.65%   99.65%   -0.01%     
==========================================
  Files        1988     2007      +19     
  Lines      211108   212073     +965     
  Branches      909      972      +63     
==========================================
+ Hits       210384   211333     +949     
- Misses        705      720      +15     
- Partials       19       20       +1     
Impacted Files Coverage Δ
src/definitions/color.ts 0.00% <0.00%> (ø)
src/definitions/commerce.ts 0.00% <ø> (ø)
src/definitions/definitions.ts 0.00% <0.00%> (ø)
src/definitions/index.ts 0.00% <0.00%> (ø)
src/locales/ar/color/human.ts 100.00% <ø> (ø)
src/locales/ar/commerce/index.ts 100.00% <ø> (ø)
src/locales/az/color/human.ts 100.00% <ø> (ø)
src/locales/az/commerce/index.ts 100.00% <ø> (ø)
src/locales/el/color/human.ts 100.00% <ø> (ø)
src/locales/el/commerce/index.ts 100.00% <ø> (ø)
... and 68 more

@prisis
Copy link
Member

prisis commented Apr 8, 2022

Thank you very much for your Pull Request :) we a discussing in what version of faker it would make sense to put this feature into, but it looks like it will go into faker v7. We will update you on the process :)

@Shinigami92 Shinigami92 linked an issue Apr 8, 2022 that may be closed by this pull request
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll review it, when we start with v7.

src/locales/ar/commerce/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should think about if we want to go more in a direction of supporting CSS stuff or more like generating values for e.g. three.js.

For CSS I would also like to see something like get a color gamut display-p3

But for three.js I would like to have e.g. possibility to generate colors for a specific channel or stuff like that: https://threejs.org/docs/#api/en/math/Color

And maybe there is also something else non-css/non-three.js

src/color.ts Outdated Show resolved Hide resolved
src/color.ts Outdated Show resolved Hide resolved
src/color.ts Outdated Show resolved Hide resolved
src/color.ts Outdated Show resolved Hide resolved
src/color.ts Outdated Show resolved Hide resolved
src/commerce.ts Outdated Show resolved Hide resolved
@harsohailB
Copy link
Contributor Author

We should think about if we want to go more in a direction of supporting CSS stuff or more like generating values for e.g. three.js.

For CSS I would also like to see something like get a color gamut display-p3

But for three.js I would like to have e.g. possibility to generate colors for a specific channel or stuff like that: https://threejs.org/docs/#api/en/math/Color

And maybe there is also something else non-css/non-three.js

@Shinigami92 I will look into this further and update the PR description with new tasks.

@Shinigami92 Shinigami92 added the s: accepted Accepted feature / Confirmed bug label May 19, 2022
@harsohailB
Copy link
Contributor Author

@harsohailB I will jump into this PR and fix everything until it's green

Thank you! Sorry been a little busy with school and work. I will try to contribute where ever I can still.

Shinigami92
Shinigami92 previously approved these changes May 21, 2022
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please copy the impl signature so it will be picked up as the last api signature.
AFAICT you don't have to add the jsdocs to the impl signature.

src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
@Shinigami92 Shinigami92 self-assigned this May 21, 2022
Shinigami92
Shinigami92 previously approved these changes May 21, 2022
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Outdated Show resolved Hide resolved
src/modules/color/index.ts Show resolved Hide resolved
@Shinigami92 Shinigami92 requested a review from ST-DDT May 22, 2022 11:00
@ST-DDT ST-DDT requested review from a team May 22, 2022 13:21
@Shinigami92 Shinigami92 merged commit bee6054 into faker-js:main May 22, 2022
matthewmayer added a commit to matthewmayer/faker that referenced this pull request May 13, 2023
looks like the hu definition file was missed in faker-js#801 when migrating from faker.commerce.color to faker.color.human
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add more random color generators
8 participants