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 support for macOS #68

Merged
merged 15 commits into from Feb 10, 2019
Merged

Add support for macOS #68

merged 15 commits into from Feb 10, 2019

Conversation

@boyvanamstel
Copy link
Contributor

@boyvanamstel boyvanamstel commented Jan 20, 2019

This adds support for macOS. It includes a Mac framework and example project. Both are prefixed with NS instead of UI. Code is shared between both platforms though.

To make the project feel at home on both platforms I've taken the liberty to rename the UIImageColors struct and UIImageColorsQuality enum to ImageColors and ImageColorsQuality respectively. I've left UIImageColors intact in every other area to not mess with the branding of the repo.

I've updated the version to 2.1.0 as this alters the API just a bit: ImageColors is returned as an optional to take into account that the process could fail. To finalize a release a tag should be added to the repo.

preview-macos

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Jan 22, 2019

Would love to get this merged. Let me know if there's anything I can do to help make that happen. ☺️

@jathu
Copy link
Owner

@jathu jathu commented Jan 23, 2019

@boyvanamstel Thanks for the PR. We've previously had PRs that tried to add OS X support but we closed it (#19). However, I like the fact that code is shared here and is less intrusive. I'm going to give this a thorough review over the weekend and will give you an update.

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Jan 24, 2019

@jathu Excellent. Let me know if you have any questions or requests!

I wasn't aware of Panic's project. While I don't see a benefit necessarily for a project that's just 'not Objective-C', I think my changes are quite unobtrusive and they would make this the goto maintained iOS and macOS library for vibrant color picking. Which I think would be a win. 👍

Copy link
Owner

@jathu jathu left a comment

This looks nice and is a good change. Couple of nits. I don't think having a macOS example is necessary, I think it makes the whole repo bloated. I might even remove the iOS example in the near future. Once you make those changes, I can merge it in 👌

UIImageColors/Sources/UIImageColors.swift Outdated Show resolved Hide resolved
UIImageColors/Sources/UIImageColors.swift Outdated Show resolved Hide resolved
README.md Outdated

## Installation

You can either directly copy [UIImageColors.swift](Sources/UIImageColors.swift) into your project *or* you can use CocoaPods: [UIImageColors](https://cocoapods.org/pods/UIImageColors).
### Manually
Copy link
Owner

@jathu jathu Jan 27, 2019

Manual

README.md Outdated

![preview](preview.png)
![iOS preview](preview-ios.png)
![macOS preview](preview-macos.jpg)
Copy link
Owner

@jathu jathu Jan 27, 2019

I think we can remove this now

Copy link
Contributor Author

@boyvanamstel boyvanamstel Jan 28, 2019

Yea, definitely. I had to leave yesterday while making changes. Will finish this up today.

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Jan 28, 2019

This looks nice and is a good change. Couple of nits. I don't think having a macOS example is necessary, I think it makes the whole repo bloated. I might even remove the iOS example in the near future. Once you make those changes, I can merge it in 👌

Thanks! I've made the changes you mentioned.

Maybe I'll make a new repo with the macOS example project myself then. One thing I liked particularly about the Mac example is that I could drag in custom images and see how the algorithm performed.

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Jan 31, 2019

@jathu Let me know if this is good to go. :)

@jathu
Copy link
Owner

@jathu jathu commented Jan 31, 2019

@boyvanamstel this looks great. I'm going to merge it in this weekend 👌

@jathu
Copy link
Owner

@jathu jathu commented Feb 2, 2019

Hey @boyvanamstel I'm on the fence about renaming the structs to just ImageColors. Although UIImageColors doesn't make sense on macOS, I think the vast majority of people would use this for iOS so it would be alright to keep the name UIImageColors.

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Feb 3, 2019

What would be the downside of using ImageColors though?

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Feb 3, 2019

I've switched back to UIImageColors and UIImageColorsQuality if that makes you more comfortable.

@jathu jathu merged commit a1338fc into jathu:master Feb 10, 2019
@jathu
Copy link
Owner

@jathu jathu commented Feb 10, 2019

Thanks @boyvanamstel !! 💯

@boyvanamstel
Copy link
Contributor Author

@boyvanamstel boyvanamstel commented Feb 11, 2019

You're welcome @jathu. Thanks for a great repo! 👍

@boyvanamstel boyvanamstel deleted the macos branch Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants