Skip to content

Conversation

@ashfurrow
Copy link
Member

Mergers #70.

I was initially confused about how to use this, and the Demo came in handy! I'm definitely a fan of this approach, great work @stefanomondino!

A remaining question: should the action parameter in bindTo() functions be optional?

We should also update our readme once this is merged 😉

@ashfurrow
Copy link
Member Author

@stefanomondino you can review my individual changes here: https://github.com/RxSwiftCommunity/Action/pull/72/files/747917a73ec992da38e329ec94d7cb7bebc491fc..c62ca31ffded0817b3989b0cefc6ebdd312072f1 Everything I did is up for discussion, so don't be shy!

@stefanomondino
Copy link
Member

@ashfurrow I was thinking about cases when you want to "unbind" the Action from the button, that's why I've declared the action as optional. Felt better than passing something like Action {_ in return .empty()}.
If it was just for me, I would have declared a second property on each control with a new type, something like var controlAction:ControlAction? , where ControlAction is a NSObject subclass that takes an Action and a input transform closure (cloning ReactiveCocoa's CocoaAction), but I'm afraid it can generate too much confusion.

@ashfurrow ashfurrow mentioned this pull request Jan 5, 2017
@ashfurrow
Copy link
Member Author

Cool, @stefanomondino if you could take one more look and merge if it's looks okay, that'd be 👌

@stefanomondino stefanomondino merged commit 4daa9d3 into master Jan 6, 2017
@stefanomondino stefanomondino deleted the stefanomondino-bindToAction branch January 6, 2017 19:04
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.

3 participants