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

.hex() silently drops alpha info. #243

Closed
maranomynet opened this issue Feb 20, 2022 · 1 comment · Fixed by #244
Closed

.hex() silently drops alpha info. #243

maranomynet opened this issue Feb 20, 2022 · 1 comment · Fixed by #244

Comments

@maranomynet
Copy link
Contributor

maranomynet commented Feb 20, 2022

Yes, I have searched the closed issues and read what's there.


Problem: Color.prototype.hex() breaks the principle of least surprise.

The behavior is never documented (not even in a test).

It's inconsistent with the existing behavior of .rgb().toString(), .hsl.toString() et. al.
All those methods intelligently switch between rgb and rgba, hsl and hsla etc. when appropriate.
(Note: Currently the only way to force a "rgb()" string is to first run .alpha(1))

I see a few ways to mend this, and I'm willing to make a PR if you're willing to accept any of them:

  1. Documentation:
    1. Add clearly worded note in the README warning of this behavior.
    2. Add a test that tests for this behavior.
  2. Add a lossless .toHex() method that switches between .hex() and .hexa()
    1. Add tests for the new method.
  3. Make .hex() more similar to .rgb().string():
    1. Change .hex() to automatically emit the alpha channel when valpha < 1.
    2. Update the tests for .hex()
    3. Either deprecate .hexa() or add a new lossy .hex6() method.

Methods 1 and 2 easily go together.

Method 3 could qualify as a non-breaking change, as the current behaviour has never been tested or documented.

@Qix-
Copy link
Owner

Qix- commented Feb 20, 2022

PR for docs and tests welcome. Anything more than that is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants