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: Implement contrast-color function #808

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

Conversation

LinusU
Copy link

@LinusU LinusU commented Sep 7, 2024

Hello! 👋

This is my first time contributing, so I'm sure that I got a lot of things wrong. But I'm very happy to receive feedback and iterate on this!

This adds support for the contrast-color function. I've loosely based my changes on #105 which adds color-mix (huge thank you @devongovett 🙏).

I've followed the current draft specification:
https://drafts.csswg.org/css-color-6/#colorcontrast

@LinusU LinusU mentioned this pull request Sep 7, 2024
6 tasks
@devongovett
Copy link
Member

Thanks for working on this! 🥳

One thing I think we should clarify is the status of the spec. I noticed that it also appears in css-color-5 with a very different syntax. I'd like to understand the state of things and what changes are expected, and perhaps we can implement it behind a draft flag initially.

@LinusU
Copy link
Author

LinusU commented Sep 11, 2024

and perhaps we can implement it behind a draft flag initially.

Yeah, this is probably a very good idea!

It's hard to find information on what exactly is going on with this and I actually implemented yet another syntax first from MDN 😅

For some background on why I'm adding this, my use case is that I'm converting a very old legacy PHP + Less app to Rust. I'm using askama to replace the PHP templating, and since I wanted to move to just Rust based tools I needed something that could replace Less. I could probably do it with only askama templating, but I figured that it would be pretty sweet if I could just write regular CSS and have it compiled to work for all browsers. I've gotten almost everything working (e.g. lighten/darken can be implemented with hsl(from {{ color }} h s calc(l + 10%)), etc.), but I haven't gotten contrast(...) to work yet.

So with that said, for me the syntax isn't really that important, but I need the ability to specify the fallback colors, which was added in level 6. But I'd be happy to add support for the max keyword as well, to make it fully compatible with level 5 as well. In fact, the only thing that needs to be done is to optionally allow "max" at the end, but it won't affect functionality since level 5 spec says that #000 and #fff is allowed colors even without the max keyword, and in level 6 it was changed so that it should be that unless specific colors was specified and the max keyword seems to have been removed.

I wonder if maybe the best thing would be to support the level 5 syntax without any flags, and then have a "level6" cargo feature flag that additionally adds the "tbd-bg wcag2" syntax with a list of colors?

That way we can document that the level6 feature is not finalized and could change in minor versions to align with the spec. (although I don't think it would be a problem to keep supporting tbd-bg / tbd-fg after the names are finalized)

There is also some relevant discussions here:

@LinusU
Copy link
Author

LinusU commented Sep 11, 2024

I've added two commits to adress the discussed:

  • dcf2d65 adds support for the max keyword to be compatible with Level 5 syntax
  • a405f5f moves the tbd-bg wcag2 syntax behind the level6 feature flag

@LinusU
Copy link
Author

LinusU commented Oct 6, 2024

@devongovett what do you think of the added feature flag? Is this something that could work?

Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in review.

// https://github.com/w3c/csswg-drafts/issues/7937
#[cfg(feature = "level6")]
"tbd-bg" | "tbd-fg" => {
input.expect_ident_matching("wcag2")?;
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is optional in the spec.

If no color candidates have been provided, may be omitted, in which case a UA-chosen algorithm is used.

If the target contrast level is omitted, the color candidate with the greatest contrast is returned. Otherwise, the returned color is the first color candidate that meets or exceeds that level, defaulting to white or black if none qualify.

Also looks like it can be a function:

Arguments to a functional notation indicate the target contrast level.

= wcag2 | wcag2([ | [ aa | aaa ] && large? ])

},

// https://github.com/w3c/csswg-drafts/issues/7937
#[cfg(feature = "level6")]
Copy link
Member

Choose a reason for hiding this comment

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

Rather than a cargo flag, we should use the drafts option for this. Then at runtime you could decide, and we can expose this to the JS API. We could accept the spec-level as the value. For example:

{
  drafts: {
    contrastColor: 6
  }
}

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