Add bindable property to wrap a control's value #119
Conversation
| return associatedObject(host, key: key) { host in | ||
| let property = MutableProperty(initial(host)) | ||
|
|
||
| setUp?(property) |
There was a problem hiding this comment.
setUp should be as well @NoEscape
There was a problem hiding this comment.
It cannot be @noescape unless we change it to non-optional but it that case we need to provide a default implementation. What approach should I follow?
There was a problem hiding this comment.
I would prefer with it and and default { _ in }. This would be easier for the consumer, since he wouldn't have to reference self in the closure. There is also some minor optimization with it.
dmcrodrigues
force-pushed
the
dr/add-uicontrol-value-changed-property
branch
from
3dea651
to
d8d9dff
Compare
| /// events to detect changes and keep the value up-to-date. | ||
| // | ||
| @warn_unused_result(message="Did you forget to use the property?") | ||
| class func rex_valueChanged<Host: UIControl, T>(host: Host, getter: Host -> T, setter: (Host, T) -> ()) -> MutableProperty<T> { |
There was a problem hiding this comment.
I think the name rex_valueChanged is misleading.
Especially considering rex_controlEvents above, I first assumed that this was like a pre-filtered version of rex_controlEvents, but then I saw thats not really what it does.
I'm not sure what would be a good name, rex_controlEventUpdatedProperty seems a bit clumsy.
There was a problem hiding this comment.
I agree that the name is not entirely correct mainly because this is a property with a state so the Changed part does not make much sense. I would suggest rex_value which I think is more accurate since is a bindable property for a value of a UIControl.
The name will clash with UISlider's value (in terms of naming only) but this property is not intended to be public, it's only a convenient method to be (re-)used internally. But I'm totally open for other suggestions.
There was a problem hiding this comment.
Since its internal, I think rex_value should be fine
dmcrodrigues
force-pushed
the
dr/add-uicontrol-value-changed-property
branch
from
d8d9dff
to
e9fe238
Compare
|
@dmcrodrigues quick one, how is this related to #115? |
dmcrodrigues
force-pushed
the
dr/add-uicontrol-value-changed-property
branch
from
e9fe238
to
5123869
Compare
|
@RuiAAPeres sorry I haven't seen your comment. #115 introduces a bindable property for |
The introduced `setUp` is extremely helpful to setup any signals that may affect the property once.
This property is common to every `UIControl` and can be used by providing a getter and a setter for the value wrapped. This property uses `UIControlEvents.ValueChanged` and `UIControlEvents.EditingChanged` events to detect changes and keep the value up-to-date. This kind of logic can be reused instead of being defined and used for each control.
This prevents repetition of events in cases where the property is accessed more than once (each access adds a new `rac_signalForSelector` for the dismiss of the view controller).
…be of optional type
dmcrodrigues
force-pushed
the
dr/add-uicontrol-value-changed-property
branch
from
5123869
to
98d4932
Compare
What's included in this PR?
associatedPropertywith asetUpclosure to perform some configuration like setting up any signals that may affect the property once.rex_valueChanged, to wrap a control's value. The property usesUIControlEvents.ValueChangedandUIControlEvents.EditingChangedevents to detect changes and keep the value up-to-date. This prevents code duplication in each control to monitor changes.-sendActionsForControlEvents:like UIDatePicker (test was disabled) andUISwitch.setUpto install the signal thatrex_dismissAnimateddepends upon. This prevents repetition of events in cases where the property is accessed more than once (each access adds a newrac_signalForSelectorfor the dismiss of the view controller).If this goes forward it will help to easily provide bindable properties for other controls like
UISegmentedControl,UISlider, ...