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(Util): add HSV and HSL color format support #9756

Closed
wants to merge 1 commit into from

Conversation

codeblitz97
Copy link

In this pull request, I've made significant enhancements to the resolveColor function, empowering it to now handle HSL (Hue, Saturation, Lightness) and HSV (Hue, Saturation, Value) color representations. This addition not only broadens the spectrum of colors we can work with but also opens up exciting creative possibilities.

As someone who's passionate about colors, I truly believe that this enhancement will add a new layer of versatility to our projects. Whether you're aiming for a vibrant and eye-catching design or a more subtle and elegant palette, the flexibility of using HSL and HSV can take your color schemes to the next level.

To showcase the power of these additions, I've included usage examples below:

Using HSV on an embed:

let embed = new EmbedBuilder().setColor({ h: 30, s: 100, v: 100 }); // Achieve that vivid orange using HSV

Or experimenting with HSL:

let embed = new EmbedBuilder().setColor({ h: 30, s: 100, l: 50 }); // Capture the essence of orange using HSL

I'm confident that this enhancement will enhance our color-handling capabilities and elevate the visual aesthetics of our projects.

Looking forward to your feedback and suggestions as we continue to make strides in delivering exceptional experiences through our codebase. Thanks for considering these changes!

@vercel
Copy link

vercel bot commented Aug 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Aug 8, 2023 5:37am
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Aug 8, 2023 5:37am

@github-actions
Copy link

github-actions bot commented Aug 8, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 62
🟢 Accessibility 97
🟢 Best practices 100
🟠 SEO 75
🔴 PWA 30

Lighthouse ran on https://discord-js-git-fork-codeblitz97-main-discordjs.vercel.app/

@kyranet
Copy link
Member

kyranet commented Aug 8, 2023

I'm opposed to supporting this, for starters, users could expect the array overload (always RGB) to be HSL or HSV.

However, later we would face the issue where people want to use full names (hue, saturation, and luminance) over the short names (h, s, l).

And that's not to mention the users who would like to use other colour formats, e.g. CIELUV, CIELAB, HSLuv, LCh (which is a cylindrical transformation of the two first), and so on. At that point, it's better to just use a dedicated color library such as colord, which supports all the listed formats and even more.

@vladfrangu
Copy link
Member

I definitely concur with @kyranet's main point. I don't think this should be added to .setColor (not in builders, nor in discord.js), and instead utility methods should be used

@Jiralite Jiralite added this to the discord.js 14.13 milestone Aug 8, 2023
@codeblitz97
Copy link
Author

You all think this should not be added?

@iCrawl
Copy link
Member

iCrawl commented Aug 8, 2023

I'm not entirely against it. I think it has its place.

But as mentioned before already, there is a library that basically does this already 😅
So it's a bit of a hard sell here.

@codeblitz97
Copy link
Author

So I'll just close the PR?

@codeblitz97 codeblitz97 closed this Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants