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

Adds SLSwitch #85

Merged
merged 6 commits into from
Oct 1, 2013
Merged

Adds SLSwitch #85

merged 6 commits into from
Oct 1, 2013

Conversation

j-mutter
Copy link
Contributor

This adds an SLSwitch class which enables wraps UIASwitch's setValue method, and provides a property to access the current value of the switch as a bool instead of a string.

#import "SLButton.h"

#define ON YES
#define OFF NO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm loathe to dump symbols with names as generic as "ON" and "OFF" into the global namespace. At the same time, it is nice to layer "off/on" semantics onto value. What do you think about getting rid of setValue: and just making on readwrite? setOn:YES would be pretty clear imo--that's how UISwitch is set up, actually.

@wearhere
Copy link
Contributor

Hey thanks for the request @j-mutter. I really appreciate you including documentation and tests. I've made a few comments. Also, Travis doesn't yet have Xcode 5 installed, unfortunately, so the build will fail like so unless you set SLSwitchTestViewController.xib to "open in Xcode 4.6" in Xcode's right sidebar:

screen shot 2013-09-30 at 4 38 51 pm

Thanks!

@j-mutter
Copy link
Contributor Author

j-mutter commented Oct 1, 2013

@wearhere Done. I had been using setValue instead of setOn because that's how it's handled in UIASwitch, but matching UISwitch also makes sense. And not sure how I completely spaced on the lack of matchesObject:, but that's sorted as well.

@wearhere
Copy link
Contributor

wearhere commented Oct 1, 2013

Looks great! Thanks again.

wearhere added a commit that referenced this pull request Oct 1, 2013
@wearhere wearhere merged commit 718182b into inkling:master Oct 1, 2013
@j-mutter j-mutter deleted the feature/adds-slswitch branch April 21, 2014 14:02
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