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(css): add color-mix() tool #184

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Conversation

OnkarRuikar
Copy link
Contributor

@OnkarRuikar OnkarRuikar commented Jun 25, 2024

The PR adds a tool to experiment with the CSS color-mix() function. Following is the screenshot of the tool:
1

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner June 25, 2024 11:34
@OnkarRuikar OnkarRuikar requested review from pepelsbey and removed request for a team June 25, 2024 11:34
@bsmth bsmth self-requested a review June 27, 2024 12:49
@bsmth
Copy link
Member

bsmth commented Jul 1, 2024

Thanks, @OnkarRuikar. Looking good! I made some suggestions on a branch, do you want to have a look here:

https://github.com/OnkarRuikar/css-examples/compare/css_color_mix...bsmth:css-examples:css_color_mix_brian?expand=1

  • monospace font for readability of the <label>s.
  • Change event listeners to use input so we have faster feedback
  • Flex for alignment
  • Some margin to give the calc / output some breathing room
@bsmth
Copy link
Member

bsmth commented Jul 1, 2024

And just general question; why do we want to include Pickr? Not a blocker, just curious 🤔

@OnkarRuikar
Copy link
Contributor Author

I've applied the changes.

And just a general question; why do we want to include Pickr?

Because HTML color input doesn't support alpha(transparency) channel: whatwg/html#3400 (comment) .

@bsmth
Copy link
Member

bsmth commented Jul 1, 2024

I've applied the changes.

Nice, taking a look again!

And just a general question; why do we want to include Pickr?

Because HTML color input doesn't support alpha(transparency) channel: whatwg/html#3400 (comment) .

Ah-ha, thanks for the hint 👍🏻

Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

LGTM, only last thing to think about is demonstrating that the percentage doesn't have to be a whole number;

<input id="percentage-one" type="range" name="percent1" step="0.1" />

What do you think? I'm fine with merging as-is!

@OnkarRuikar
Copy link
Contributor Author

LGTM, only last thing to think about is demonstrating that the percentage doesn't have to be a whole number;

🤓 done.

@bsmth
Copy link
Member

bsmth commented Jul 1, 2024

Merging now, thank you!

@bsmth bsmth merged commit e0c8a42 into mdn:main Jul 1, 2024
1 check passed
@OnkarRuikar OnkarRuikar deleted the css_color_mix branch July 1, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants