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

Add vars for yiq color contrast function #24426

Merged
merged 1 commit into from
Oct 22, 2017
Merged

Add vars for yiq color contrast function #24426

merged 1 commit into from
Oct 22, 2017

Conversation

mdo
Copy link
Member

@mdo mdo commented Oct 19, 2017

Adds two vars instead of the hardcoded colors we had previously. This supersedes #23440, getting us close to what that implementation provides without all the additional Sass map merges.

@danijelGombac
Copy link

danijelGombac commented Oct 19, 2017

@mdo

I have a question about yiq color contrast function. Why this function don't' use darken, lighten function to got the contrast?

Like:

$color-yiq-dark-amount: 60% !default;
$color-yiq-light-amount: 60% !default;

@function color-yiq($color, $dark-amount: $color-yiq-dark-amount, $light-amount: $color-yiq-light-amount) {
  $r: red($color);
  $g: green($color);
  $b: blue($color);

  $yiq: (($r * 299) + ($g * 587) + ($b * 114)) / 1000;

  @if ($yiq >= 150) {
    @return darken($color, $dark-amount);
  } @else {
    @return lighten($color, $light-amount);
  }
}

@mdo
Copy link
Member Author

mdo commented Oct 20, 2017

The function determines brightness for us and makes a call on whether a #fff or #111 color passed color contrast against the background. A fixed percentage doesn't work here.

@danijelGombac
Copy link

@mdo

I understand how it works, but there isn't fixed percentage. Here is a demo if you have time.

https://www.sassmeister.com/gist/efc3b9eafddadb17dd4c0b0873fd47e5

@mdo mdo merged commit bf2fee9 into v4-dev Oct 22, 2017
@mdo mdo mentioned this pull request Oct 22, 2017
@XhmikosR XhmikosR deleted the yiq-vars branch October 22, 2017 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants