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(rgba): Fade color with rgba module #168

Merged
merged 1 commit into from
Jun 21, 2017
Merged

feat(rgba): Fade color with rgba module #168

merged 1 commit into from
Jun 21, 2017

Conversation

bhough
Copy link
Contributor

@bhough bhough commented May 10, 2017

rgba now can accept a hex or named CSS color value and an alpha value to fade a color.

Addresses request made in #104

@codecov
Copy link

codecov bot commented May 10, 2017

Codecov Report

Merging #168 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #168   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          64     64           
  Lines         347    350    +3     
  Branches       97     98    +1     
=====================================
+ Hits          347    350    +3
Impacted Files Coverage Δ
src/color/parseToRgb.js 100% <ø> (ø) ⬆️
src/color/rgba.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 752f41b...213e2a0. Read the comment docs.

@bhough bhough requested review from nikgraf and mxstbr May 10, 2017 14:40
@nikgraf
Copy link
Member

nikgraf commented May 10, 2017

Still not a fan, transparentize(0.3, '#fff') is a much more descriptive API :)

Nevertheless the PR looks good. One open question: Should we allow strings then rgb as well? If not that seems to be pretty inconsistent.

@bhough
Copy link
Contributor Author

bhough commented May 11, 2017

@nikgraf I've been kinda torn on this one myself. There are a couple things at play here.

  1. it seems like the consensus is that users don't seem to want to do the math and would rather have the alpha value just be a pass in for the alpha level as opposed to the amount you want to transparentize by.

  2. because the goal of the project is still to replace sass/less and their libraries, users seem to want rgba to work the same way.

Which still leaves us with the question of, is there a solution that accomplishes these things without making us feel bad about including it in the library?

Can you give me an example of what you mean when you say :

Should we allow strings then rgb as well?

@nikgraf
Copy link
Member

nikgraf commented Jun 21, 2017

Hmm, I can't remember what I meant by strings, but the PR looks good to me.

If we haven't found a better solution after that time I think the best course of action is to merge it :)

Can we merge this now or should it be 2.0? Afaik it's not a breaking change.

@bhough bhough changed the base branch from master to version-2 June 21, 2017 10:23
@bhough bhough changed the base branch from version-2 to master June 21, 2017 10:37
@bhough
Copy link
Contributor Author

bhough commented Jun 21, 2017

This is not a breaking change, so I'm fine with merging it.

rgba now can accept a hex or named CSS color value and an alpha value to fade a color
@bhough bhough merged commit c5ccb3b into master Jun 21, 2017
@bhough bhough deleted the rgba-enhancement branch June 21, 2017 10:55
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