-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fixes & additional updates #53
Conversation
/// protocol to represents the button rather than hardcode it to classes, | ||
/// allowing for any `UIControl` to use this method. | ||
/// | ||
var tapped: TapAction<Base>? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd welcome a new name here, as pressed
is already used, and they semantically are pretty much the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
…es corrupt its internal state
98be74c
to
1a6e03f
Compare
self.applyClosure = { action.apply($0 as! Action.Input).map { $0 }.mapError { $0 } } | ||
self.events = action.events.map { $0.map { $0 }.mapError { $0 } } | ||
self.values = action.values.map { $0 } | ||
self.errors = action.errors.map { $0 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we write
self.values = action.values
self.errors = action.errors
is .map { $0 } required ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harishsaini it is, to convert to Any
or Error
through type inference. I could have added as Any
in the map
s and as Error
in the mapError
, but I figured it wasn't necessary as it's a common-ish pattern in Swift
24ed824
to
1061417
Compare
Codecov Report
@@ Coverage Diff @@
## develop #53 +/- ##
=========================================
Coverage ? 7.34%
=========================================
Files ? 60
Lines ? 1729
Branches ? 0
=========================================
Hits ? 127
Misses ? 1602
Partials ? 0 Continue to review full report at Codecov.
|
6fc6b8a
to
6963623
Compare
6963623
to
29be1b6
Compare
Goals ⚽
This PR:
insets
parameter toaddAndFitSubview()
, which should be self-explanatoryremoveArrangedSubviews()
'sremoveFromHierachy
parameter default totrue
OrderedSet
, and make it conform toSetAlgebra
tapped
helper to link anyAction
to anyUIControl
AnyIdentifiable
&AnyAction
for type-erasedIdentifiable
&ReactiveActionProtocol
respectivelyOverridingAction
, a newAction
that if executed when already executing, will cancel the current one and start anewBackwards-compatibility:
All changes are additive and doesn't break backward compatibility.
Testing Details 🔍
Unit tests were added for
OrderedSet
.