Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

v2 questions #48

Closed
bre7 opened this issue Sep 21, 2015 · 19 comments
Closed

v2 questions #48

bre7 opened this issue Sep 21, 2015 · 19 comments
Labels

Comments

@bre7
Copy link
Collaborator

bre7 commented Sep 21, 2015

  1. Typo in the definition ? It's in the #pragma mark - Class Methods section
  2. In UIViewController+Chameleon.h the method flatify() is empty (Line 97)
  3. In findColorsOfImage(...), won't the NSArray and NSCountedSet have the same colors ?
@bre7 bre7 changed the title colorWithMinimumSaturation is an instance method instead of class v2 questions Sep 21, 2015
@vicc
Copy link
Owner

vicc commented Sep 21, 2015

@bre7

  1. In what file is the typo in? Ohh you're referring to the Instance Method in that section. Got it! Will fix it in the next version thanks!
  2. I ended up removing support for the flatify method. It wasn't as perfect as I wanted it to be, and with the new support for themes, I figured it wouldn't be as useful.
  3. Not exactly. The NSCountedSet holds a color for every single pixel. I then do some processing (ensure they all have a minimum saturation tolerance) and sort them by most occuring to least occuring. Eventually only 5 will end up in the NSArray.

@vicc vicc closed this as completed Sep 21, 2015
@vicc vicc reopened this Sep 21, 2015
@bre7
Copy link
Collaborator Author

bre7 commented Sep 21, 2015

I think 1 was in one of the categories named Private.

Tests incoming also. I'll start with your code as a baseline for now.

@bre7
Copy link
Collaborator Author

bre7 commented Sep 22, 2015

if (abs(r - g < 0.03f && fabs(r - b) < 0.03f)) {

abs of a bool ?

@vicc
Copy link
Owner

vicc commented Sep 22, 2015

Where is that line located?

@vicc
Copy link
Owner

vicc commented Sep 22, 2015

Oh right. abs isn't a bool. What that line is doing is checking if a certain color is distinct enough from another. Specifically, it is checking if the difference of several values is at least 0.03 (tolerance). If it is, it determines that the two colors are distinct, otherwise it returns no. It does this several times checking for different things each time.

@bre7
Copy link
Collaborator Author

bre7 commented Sep 23, 2015

  1. Shouldn't UIColor.blueColor().flatten() be the same as FlatBlue() or FlatBlueDark() ? Or should nearestColor return another thing ? Or is flatten supposed to compute a flat version of the color, without taking into account the FlatColors array ?
  2. One other thing, shouldn't flatten() call UIColor(flatVersionOf: color, withAlpha: color.alpha) instead of UIColor(flatVersionOf: color, withAlpha: 1.0) ?

@vicc
Copy link
Owner

vicc commented Sep 23, 2015

  1. Yes and no. What it does is it tries to find the closest "Flat Color" to the provided color. 'Flattening' blue has always been a bit of a problem since the human eye tends to favor it differently than computers, so it usually returns a purplish flat for that specific blue. It worked exactly the same as the deprecatedflatVersionOf: method.
  2. Ahh you're right! Nice catch!

@bre7
Copy link
Collaborator Author

bre7 commented Sep 23, 2015

  1. Why have a separate parameter for alpha instead of using color's alpha, in + (UIColor *)colorWithComplementaryFlatColorOf:(UIColor *)color withAlpha:(CGFloat)alpha
  2. Why does complementaryFlatColor(..) calculate the color using the gradient image ? Nvm, got it
  3. gradientImage is supposed to be an optional asset set by the dev, right ?
  4. if (abs(r - g < 0.03f && fabs(r - b) < 0.03f)) again. I'm still not sure why abs(boolValue) is working since abs expects an Int (or other numeric type...abs(0) = 0 in which case why even use abs ?)

@vicc vicc added the Question label Sep 25, 2015
@vicc
Copy link
Owner

vicc commented Sep 25, 2015

  1. To answer your first question, I figured it would be easier for developers who also wanted to change the alpha of the new color, to do so in one call. I've found myself having to do that a few times, and so this definitely made that easier. However I'm sure you've seen it, but there's also a simple version of that method: + (UIColor *)colorWithComplementaryFlatColorOf:(UIColor *)color
  2. Glad you figured it out! 😃
  3. Actually gradientImage is a property added to UIImage to hold the gradient a user creates when using colorWithGradient... It's not meant to be directly set by the user (More like a storage area for Chameleon). If anything it should be readonly though. I'll update it.
  4. Let me take a look and I'll get back to you on this!

@bre7 bre7 mentioned this issue Sep 26, 2015
@vicc
Copy link
Owner

vicc commented Sep 27, 2015

@bre7 Seeing how much you've helped out in the past few weeks, I've added you as a collaborator to make things easier! Thanks for all your help 💯 😃

@bre7
Copy link
Collaborator Author

bre7 commented Sep 27, 2015

😉😁 ty. I've enjoyed working with this framework quite a lot

I usually don't merge anything (unless it's minor) until the owner has a chance to check it out.

@vicc
Copy link
Owner

vicc commented Sep 27, 2015

No worries! I trust your input. 👍

@jlama
Copy link

jlama commented Oct 13, 2015

Shouldn't if (abs(r - g < 0.03f && fabs(r - b) < 0.03f)) actually be if (fabs(r - g) < 0.03f && fabs(r - b) < 0.03f) ?

@vicc
Copy link
Owner

vicc commented Nov 25, 2015

@jlama At first glance it looks like your suggestion should be used, but apparently Xcode throws the following warning if we do that:

/Users/...UIColor+ChameleonPrivate.m:103:13: Using floating point absolute value function 'fabs' when argument is of integer type

That is why we're using abs instead of fabs.

@vicc vicc closed this as completed Nov 25, 2015
@jlama
Copy link

jlama commented Nov 26, 2015

Xcode throws that warning because the whole expression inside fabs() always evaluates to a boolean, due to the parentheses position, and does not make any sense.

What I wanted to point out is the missing closing ) after abs(r - g
if (abs(r - g < 0.03f && fabs(r - b) < 0.03f))
vs
if (fabs(r - g) < 0.03f && fabs(r - b) < 0.03f)

@vicc
Copy link
Owner

vicc commented Nov 26, 2015

Ahh I see. Thanks @jlama. It seems that as of 2.0.4 it's been fixed. Appreciate it though.

@vicc
Copy link
Owner

vicc commented Nov 26, 2015

Oh wow you're right. It should be fixed in 2.0.5.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants