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

Would like to add a flag to hide the labels above the slider controls… #21

Merged
merged 1 commit into from
Jan 11, 2016
Merged

Would like to add a flag to hide the labels above the slider controls… #21

merged 1 commit into from
Jan 11, 2016

Conversation

chefnobody
Copy link

…. Why? Two main reasons:

  1. I think this makes for a cleaner API. The work-around mentioned in the header seems overly complicated and doesn't work well with Swift. It might be possible with Swift, but I was unable to downcast an NSNull or AnyObject to NSNumberFormatter and get back an NSNull in Obj-C. I was only ever able to get a nil which the other sort of unclear use-case in the API. If you set this property to nil you get no formatter style. Anyway ... I just wanted to turn off the labels. Seems to me a flag would be a much cleaner way to go about it.
  2. I didn't need labels animated and following the thumb controls for my purposes. I would assume others don't either.

What I've done:

  • Added a new IBInspectable flag named hideLabels which by default is set to NO. Setting it to YES or true will hide them. I've made sure that updating this property while building the UI will correctly update the Storyboard/XIB. Updated the comment in the header.
  • Maintained backward compatibility with the aforementioned label hiding rules so older consumers of this API don't break. I would also be in favor of removing the old behavior and making this a breaking change ... but that's probably not my call :-)/
  • Updated the Storyboard to include an example without the labels. Though I didn't bother fixing AutoLayout problems. Perhaps I should?
  • Updated the readme.

…. Why? Two main reasons:

1. I think this makes for a cleaner API. The work-around mentioned in the header seems overly complicated and doesn't work well with Swift. It might be possible with Swift, but I was unable to downcast an `NSNull` or `AnyObject` to `NSNumberFormatter` and get back an `NSNull` in Obj-C. I was only ever able to get a `nil` which the other sort of unclear use-case in the API. If you set this property to `nil` you get no formatter style. Anyway ... I just wanted to turn off the labels. Seems to me a flag would be a much cleaner way to go about it.
2. I didn't need labels animated and following the thumb controls for my purposes. I would assume others don't either.

What I've done:
- Added a new `IBInspectable` flag named `hideLabels` which by default is set to `NO`. Setting it to `YES` or `true` will hide them. I've made sure that updating this property while building the UI will correctly update the Storyboard/XIB. Updated the comment in the header.
- Maintained backward compatibility with the aforementioned label hiding rules so older consumers of this API don't break. I would also be in favor of removing the old behavior and making this a breaking change ... but that's probably not my call :-)/
- Updated the Storyboard to include an example without the labels. Though I didn't bother fixing AutoLayout problems. Perhaps I should?
- Updated the readme.
@TomThorpe
Copy link
Owner

Hi chefnobody, 
Thanks for the thought you've clearly put into this. I'm swamped with work at the moment but will try and look at this soon. On initial glance I agree with your comments though :-)
Tom

@TomThorpe
Copy link
Owner

Hi @chefnobody,

Sorry for my brief reply earlier. I've had a quick look at the code and it looks great. On your point about the number formatter you are spot on - what you've done is much better than before so thanks for updating that and fixing my hack!

Merging :-)

Tom

@TomThorpe TomThorpe closed this Jan 11, 2016
@TomThorpe TomThorpe reopened this Jan 11, 2016
TomThorpe added a commit that referenced this pull request Jan 11, 2016
Would like to add a flag to hide the labels above the slider controls…
@TomThorpe TomThorpe merged commit 9797856 into TomThorpe:master Jan 11, 2016
@chefnobody
Copy link
Author

Cheers Tom! Thanks!

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

2 participants