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 methods to make images using a DynamicGradient #38

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

Conversation

gbitaudeau
Copy link

No description provided.

@yannickl
Copy link
Owner

yannickl commented Aug 1, 2017

Hi Guillaume,

I'm not a big fan of the gradient image API because it introduces inconsistent APIs between iOS, tvOS, watchOS and macOS platforms. Moreover, your tests fail on travis.

DynamicColor is about the color manipulation, not visualisation. Either we create a sub-project with a DynamicGradientView that match the NSGradient macOS counterpart, or we create a new project.

However the isOpaque property is a great idea. You can pull those updates to start. I would just update the gradient one with something like that:

var isOpaque: Bool {
  return colors.reduce(true) { $0.0 && $0.1.isOpaque }
}

@gbitaudeau
Copy link
Author

Hi Yannick,

Tests work on Xcode, it's weird that they fail on travis ... One problem I had on Xcode is that ".png" were compress during compilation, to avoid that I changed their extension using ".ref", but I don't know if it works on travis.

I understand your point on APIs inconsistent between iOs and the other platforms, but I don't well know macOs counterpart, so I think I will not find the time to make it, sorry.

At the beginning of my pool request, I have a need: make a color using UIColor.init(patternImage: UIImage) to be able to put gardient in some backgrounds of my UIViews. Do we consider that colors with pattern are not real colors and haven't got their place in DynamicColor ?

I will make another request for isOpaque property.

Thanks for your time.

@codecov-io
Copy link

Codecov Report

Merging #38 into master will decrease coverage by 4.82%.
The diff coverage is 77.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage     100%   95.17%   -4.83%     
==========================================
  Files          13       14       +1     
  Lines         390      497     +107     
==========================================
+ Hits          390      473      +83     
- Misses          0       24      +24
Flag Coverage Δ
#ios 95.17% <77.57%> (-4.83%) ⬇️
#osx 75.65% <0%> (-20.76%) ⬇️
Impacted Files Coverage Δ
Sources/DynamicColor.swift 100% <100%> (ø) ⬆️
Sources/CGFloat.swift 62.5% <62.5%> (ø)
Sources/DynamicGradient.swift 84.09% <78.35%> (-15.91%) ⬇️

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 baac8bd...db4006c. Read the comment docs.

@yannickl
Copy link
Owner

yannickl commented Aug 5, 2017

Travis uses Xcode to execute the test suite, so it should work in the same way. I should investigate that when I'll have time.

I think I'll create a sub-project with a DynamicGradientView with the same API as NSGradient and we will use your code.

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.

None yet

3 participants