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

Event handlers #35

Closed
ranhsd opened this issue Mar 15, 2017 · 19 comments
Closed

Event handlers #35

ranhsd opened this issue Mar 15, 2017 · 19 comments

Comments

@ranhsd
Copy link

ranhsd commented Mar 15, 2017

Hi,
I am considering to move from storyboards to Render for part of my app UI. Currently everything works fine for me and i managed to build some nice UI with it easily. The only thing that is missing is best practices on how to handle events. in my UI i have two buttons: one for signing in and one for signing up and i wanted to know what is the recommended way to handle the on tap event with Render.
Please consider the fact that ReSwift is also big part in my architecture so when the user will click on sign in then i will dispach an action.

Thanks.

@esam091
Copy link
Contributor

esam091 commented Mar 15, 2017

I think you can use BlocksKit or something similar. The syntax is somewhat weird in Swift 3, but does the job for now.

@alexdrone
Copy link
Owner

If you are in the scope of a componentView you could use the classic target-action pattern from UIKit in the configuration closure.

ClosureKit or BlocksKit will work just fine as well.

Will soon update the documentation with some examples.

@ranhsd
Copy link
Author

ranhsd commented Mar 15, 2017

Hi, Actually from architecture point of view i think that the components should not handle actions at all that's because that i think the same component can be reused by multiple classes in the lifecycle of the app. What i did currently is to i created a delegate inside the component and my view controller registered to this delegate and i forward all actions and handle them in the view controller side instead of handling them in the component code.

What do you guys think about this approach ?

Thanks!

@hfossli
Copy link
Contributor

hfossli commented Mar 15, 2017

I second @ranhsd's point.

@alexdrone
Copy link
Owner

@ranhsd approach is the one I prefer for my applications too - but Render is not opinionated about how you'll architect your user interaction and such.

I usually make the distinction between Node fragments (pure functions that returns a hierarchy) and ComponentView subclasses.
The former are completely domain agnostic, they only know how to render the view hierarchy for the state passed as argument.

ComponentViews usually expose a delegate with a bunch of callbacks that you might want to implement in your VCs.

@hfossli
Copy link
Contributor

hfossli commented Mar 16, 2017

Let me showcase my concern.

When running this code

import Foundation
import UIKit
import Render
import BlocksKit

struct HelloWorldState: StateType {
  let name: String
}

class HelloWorldComponentView: ComponentView<HelloWorldState> {

  override init() {
    super.init()
    self.defaultOptions = [.preventViewHierarchyDiff]
  }
  
  required init?(coder aDecoder: NSCoder) {
    fatalError("Not supported")
  }

  override func construct(state: HelloWorldState?, size: CGSize = CGSize.undefined) -> NodeType {
    func button() -> NodeType {
        return Node<UIButton> {
            (view, layout, size) in
            print("added to \(view)")
            
            view.bk_addEventHandler({ sender in
                print("triggered in \(view)")
            }, for: .touchUpInside)
            
            let radius: CGFloat = CGFloat(randomInt(16, max: 128))
            view.backgroundColor = Color.green
            layout.height = radius * 2
            layout.width = radius * 2
            layout.alignSelf = .center
        }
    }
    func container() -> NodeType {
      return Node<UIImageView>(identifier: String(describing: HelloWorldComponentView.self)) {
        (view, layout, size) in

        view.backgroundColor = Color.black
        view.isUserInteractionEnabled = true
        let h = size.height == 0 ? CGFloat.max : size.height
        let w = size.width == 0 ? CGFloat.max : size.width
        layout.width = min(w, h)
        layout.height = layout.width
        layout.justifyContent = .center
      }
    }

    return container().add(children: [
      button()
    ])
  }
}

This is logged to console

added to <UIButton: 0x7fd56e618840; (...redacted...)>
added to <UIButton: 0x7fd56e618840; (...redacted...)>
added to <UIButton: 0x7fd56e618840; (...redacted...)>

When I tap the button I get

triggered in <UIButton: 0x7fd56e618840; (...redacted...)>

I really was expecting

triggered in <UIButton: 0x7fd56e618840; (...redacted...)>
triggered in <UIButton: 0x7fd56e618840; (...redacted...)>
triggered in <UIButton: 0x7fd56e618840; (...redacted...)>

Not at all what I expected from BlocksKit. It actually overwrites last registered block (if it is the same control event)! I think this is a bad architecture choice of BlocksKit, but luckily for users of Render + BlocksKit it Just Works™. :) Also totally unexpected that it works in the same manner for a UIGestureRecognizer convenience method due to requiring other recognizers to fail.

Other frameworks than BlocksKit may choose a different philosophy.

@esam091
Copy link
Contributor

esam091 commented Mar 16, 2017

I also found a problem with BlocksKit

import PlaygroundSupport

import Render
import BlocksKit

PlaygroundPage.current.needsIndefiniteExecution = true

struct AppState: StateType {
    var isOn = false
}

class MyView: ComponentView<AppState> {
    override func construct(state: AppState?, size: CGSize) -> NodeType {
        
        return Node()
            .add(children: [
                Node<UIButton>() { [unowned self] button, layout, size in
                    button.setTitle("Click me", for: .normal)
                    button.setTitleColor(.black, for: .normal)
                    
                    button.bk_(whenTapped: {
                        self.render(in: self.frame.size)
                    })
                }
            ])
    }
}

let container = UIView(frame: CGRect(x: 0, y: 0, width: 320, height: 568))
container.backgroundColor = .white

let view = MyView()
view.state = AppState()
view.render(in: container.frame.size)

container.addSubview(view)

PlaygroundPage.current.liveView = container

The time it takes to render stuffs increases the more you tap the button. Looking from the source, every time you call bk_whenTapped, it actually increases the number of gesture recognizers and that somehow increases rendering time.

@hfossli
Copy link
Contributor

hfossli commented Mar 16, 2017

Very good point.

@hfossli
Copy link
Contributor

hfossli commented Mar 16, 2017

I'm surprised to find this in UIKit (though it makes sense here knowing that the function is not changing (as opposed to blocks (where you don't know wether it is the same block or not))):

    // add target/action for particular event. you can call this multiple times and you can specify multiple target/actions for a particular event.
    // passing in nil as the target goes up the responder chain. The action may optionally include the sender and the event in that order
    // the action cannot be NULL. Note that the target is not retained.
    open func addTarget(_ target: Any?, action: Selector, for controlEvents: UIControlEvents)

In other words: this works fine

    override func construct(state: HelloWorldState?, size: CGSize = CGSize.undefined) -> NodeType {
        return Node<UIButton>() { [unowned self] button, layout, size in
            print("added to \(button)")
            button.setTitle("Click me", for: .normal)
            button.setTitleColor(.black, for: .normal)
            button.addTarget(self, action: #selector(HelloWorldComponentView.tapped(_:)), for: .touchUpInside)
        }
    }

    func tapped(_ sender: UIButton) {
        print("triggered in \(sender)")
    }

@alexdrone
Copy link
Owner

You can always use a custom create closure in the node constructor for things you don't want to be reconfigured - this can also improved the component render time.
Remember explicitly pass a ad-hoc reuse identifier when you have a custom closure.

  
override func construct(state: HelloWorldState?, size: CGSize = CGSize.undefined) -> NodeType {
    return Node<UIButton>(
      identifier: "addItemButton",
      create: {
        let button = UIButton()
        button.addTarget(self, action: #selector(Component.addItem), for: .touchUpInside)
      },
      configure: { [unowned self] button, layout, size in
        button.setTitle("Click me", for: .normal)
        button.setTitleColor(.black, for: .normal)
    })
  }

@ranhsd
Copy link
Author

ranhsd commented Mar 16, 2017

Hi,
in my code what i did is very similar to what @alexdrone did

return Node<UIButton>(
    create: {
        let button = UIButton(type: UIButtonType.custom)
        button.addTarget(self, action: #selector(self.returningUserTapHandler), for: .touchUpInside)
        return button
    },
    configure: { (button, layout, size) in
        button.setTitleColor(UIColor.white, for: .normal)
        button.backgroundColor = Color.sky
        button.setTitle("RETURNING USER", for: .normal)
        button.titleLabel?.font = Font.mediumSemiBold
        layout.height = 50
        layout.width = size.width / 2
    }
)

In additional i created the following protocol:

protocol WelcomeComponentViewDelegate : class {
    func didTapSignUpButton()
    func didTapSignInButton()
}

handle the events inside the component and then notify the delegate on each event (if was created)

@hfossli
Copy link
Contributor

hfossli commented Mar 16, 2017

Awesome!

@alexdrone
Copy link
Owner

Would be nice to add one of this examples to the Demo project :)

@hfossli
Copy link
Contributor

hfossli commented Mar 16, 2017

Can you say something about reuse in this context? Anythings we should be aware of when using the create block?

@alexdrone
Copy link
Owner

I'll add a proper section about reuse identifier in the README.
In general if you have a custom create block you need a node identifier (if you have more nodes with the same type) because the infra can't really distinguish them (I might add a warning in the logs).

As a general rule, reuse identifiers never hurt become they optimise the view reuse when the infra has to reconcile the view hierarchy.

@ranhsd
Copy link
Author

ranhsd commented Mar 17, 2017

Hi @alexdrone , i will modify the example asap when i will find some time on next week

@alexdrone
Copy link
Owner

I've added a todo list demo app where I explore some of the user interaction patterns in the context of unidirectional data flow.
Please have a look!

@ranhsd
Copy link
Author

ranhsd commented Mar 19, 2017

Hi @alexdrone , thanks for this demo it looks awesome.
Just a small question regarding it. Don't you think that it's better to use cocoapods instead of carthage here?
I tried to make this project work with cocoapods but i am getting an error on the carthage run script

/usr/local/bin/carthage copy-frameworks

If we will delete this script (and users will need to copy the frameworks that were generated by carthage on their own) we can make this project work with both cocoapods and carthage.

@alexdrone
Copy link
Owner

Done that :)

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

No branches or pull requests

4 participants