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

PR'ing that widen function I mentioned a while ago. :) #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

erlandsona
Copy link

No description provided.

Copy link
Owner

@tesk9 tesk9 left a comment

Choose a reason for hiding this comment

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

Heya, thanks for the PR!

The changes to checkContrast will make this a major version change, and generally I avoid those. I also generally avoid type aliases in function type annotations in packages because I think the compiler error messages are nicer without the alias.

I would love to see some tests against widen before merging as well.

@erlandsona
Copy link
Author

Fair point... that said, I ended up going even deeper down the rabbit hole which fundamentally shifted my approach to this whole "widen"ing thing.
But it makes the widen function more predictable.
That is, I realized that working in sRGB / HSL makes reasoning about accessible contrast a bit of a nightmare and that LCH just "fixes" it.
So I pulled the JS from https://css.land/lch/ and translated most of the JS to elm given most of it was pure functions anyways.

Basing my theme-ing primitives on LCH means I can adjust the Lightness of any hue according to this 10-step system.

Which means widen is just +/- 40 Lightness (for AA) and 50 (for AAA) for any hue of any color.

Ultimately I thought I could use elm-color-extra's Accessible module to do the lch conversion but it didn't have functions for converting True LCH back into the sRGB space which is why I had to adapt the css.land/lch functions.

ALL THAT said, not sure whether to make a new elm package just for LCH or add it to elm-color-extra or adapt the functions to work with this repo...

I'd love more eyes on the work and maybe some guidance / rubber 🦆'ing, I'm pretty much always available on Slack at @erlandsona if you're interested or want to talk more?

Feel free to do whatever with this.

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 this pull request may close these issues.

2 participants