jlandon / Alexandria

A library of Swift extensions to turbocharge your iOS development.
https://jlandon.github.io/Alexandria
MIT License
65 stars 8 forks source link

Add action closures for all UIControl subclasses #24

Closed apcorc closed 8 years ago

apcorc commented 8 years ago

screen shot 2016-06-23 at 9 20 44 am

jlandon commented 8 years ago

Just gonna throw this out there, but the code could be greatly simplified if we didn't return the control in the closure. The reason multiple protocols and generics are necessary is simply because we're returning the object. If we were to remove this, the code becomes much simpler, and we don't really lose any functionality. If someone were to want the control when the closure is called, they can easily do so like this:

button.addTarget(controlEvents: .TouchUpInside) {
   button.enabled = false
   // do other stuff with `button`
}

Thoughts?

apcorc commented 8 years ago

@jlandon I thought about that, and you're right it's very much simplified, I just like the ability to golf the control at $0 to get things like value which is especially important in controls other than UIButton which this PR is designed to support. Also if we remove the control, we'd need to keep the old implementation for UIButton and deprecate it due to code in use now which expects the control to be returned.

apcorc commented 8 years ago

After looking into it more we've come down to an option: We can do as @jlandon suggested and not return the control in the closure and support all UIControl subclasses INCLUDING custom UIControl subclasses that you define in your own projects outside what Alexandria knows about. Or we can continue passing the control back as a parameter in the closure but only support UIKit and Alexandria custom controls that are defined ahead of time. My idea of just adding the extension to support a custom control didn't work as self.action is undefined at that scope and can't be made public because of the use of Self. Or we can do both and only pass back the control if we know about it maybe? I'm interested in ya'll thoughts

hsoi commented 8 years ago

https://www.youtube.com/watch?v=CxK_nA2iVXw

hsoi commented 8 years ago

My Neutralness aside....

Given that Alexandria is public to the world, I think it makes better sense to serve that audience. So being able to support custom subclasses outside of Alexandria's knowledge will probably serve best in the long run.

jlandon commented 8 years ago

Yeah, I feel like the behavior should be the same for all UIControl subclasses, so if there's not a way to extend the behavior to built-in UIKit controls, I think we should go with the closure that doesn't return the control

jlandon commented 8 years ago

Now that we've done away with the generic, I'd say we just extend UIControl directly and remove the Actionable protocol

hsoi commented 8 years ago

I'll agree there, because what's in the protocol is inherently UIControl-specific, so I'm not sure what's gained.

If there is something @apcorc please enlighten.

apcorc commented 8 years ago

Ah yea I do remember talking about that before. Should be fine, just forgot about it.

jlandon commented 8 years ago

👍