search

Sephiroth87 / ODRefreshControl

A pull down to refresh control like the one in Apple's iOS6 Mail App
MIT License
2.15k stars 385 forks source link

Three small changes #45

Closed zadr closed 11 years ago

zadr commented 11 years ago
  1. Static Library build target. Also updates the Demo app to use a static library instead of adding the files to the project directly. This shouldn't affect existing uses (cocoapod or otherwise), since the file locations weren't changed.
  2. Use string constants instead of their values. This one's likely just for correctness, but, make a change to use NSKeyValueChangeNewKey instead of assuming that @"new" will be the key name.
  3. Don't send UIControlEventValueChanged actions until the animation finishes. This fixes a problem that came up during testing where data would come in before the animation finished, leading to weird animation timings when hiding a refresh control.
Sephiroth87 commented 11 years ago

Probably a dumb question, but is there any advantage in having a static library?

zadr commented 11 years ago

Static library targets allows for ODRefreshControl to have its own set of build flags without interfering with external projects that use it (easier to mix ARC and non-ARC, for example).

Sephiroth87 commented 11 years ago

Well, it can already have it's own flags set in the build phase tab no?

(PS: not arguing, just want to know if it's useful :) the others 2 commit are definitely ok :+1: )

zadr commented 11 years ago

While you can add flags to individual files in the project menu, it is rather annoying/difficult to do so (you have to know the GCC/Clang flag to add, make sure the order is right, etc) and requires effort from the consumer of the library.

I think its much better to have a nice dropdown menu or checkbox, and set flags once and be able to know for sure that it will build the same for everyone, everywhere.

Also, this allows for the opposite behavior: for people to have a specific set of compiler flags that apply to their project, but not third party code. (Example: Clang has an option to warn on direct ivar access. Some projects may have a style that mandates to never access ivars directly. In that case, it would make sense to turn this warning on. But, doing so would lead to dozens of warnings from ODRefreshControl if it is included in the project directly. A static library to link against avoids this problem.)

zadr commented 11 years ago

A static library target would also allow ODRefreshControl to have project-wide settings for spaces instead of tabs, to avoid contributors having to either make commits like 0ed3f8a, or having to rebase stuff in specifically to fix up whitespace ;)