Repository navigation
Horizontally, Vertically & All with margin parameter - #111
Conversation
|
|
||
| @discardableResult | ||
| internal func top(_ value: CGFloat, _ context: Context) -> PinLayout { | ||
| setTop(value, context) |
There was a problem hiding this comment.
You don't need to add this method if you call setTop(value, context), setBottom(value, context) ... in func all(_ value: CGFloat),horizontally(...) and vertically(...)
There was a problem hiding this comment.
Ah, ok you can keep it that way, I use this pattern somewhere else 😊
There was a problem hiding this comment.
But could you update the method func top(_ value: CGFloat) -> PinLayout in PinLayoutImpl.swift to:
@discardableResult
func top(_ value: CGFloat) -> PinLayout {
return top(value, { return "top(\(value))" })
}
|
|
||
| @discardableResult | ||
| internal func top(_ percent: Percent, _ context: Context) -> PinLayout { | ||
| guard let layoutSuperviewRect = layoutSuperviewRect(context) else { return self } |
There was a problem hiding this comment.
Could you update the method func top(_ percent: Percent)-> PinLayout in PinLayoutImpl.swift to:
func top(_ percent: Percent) -> PinLayout {
return top(percent, { return "top(\(percent.description))" })
}
|
|
||
| @discardableResult | ||
| internal func bottom(_ percent: Percent, _ context: Context) -> PinLayout { | ||
| guard let layoutSuperviewRect = layoutSuperviewRect(context) else { return self } |
There was a problem hiding this comment.
Could you update the method func bottom(_ percent: Percent) -> PinLayout in PinLayoutImpl.swift to:
func bottom(_ percent: Percent) -> PinLayout {
return bottom(percent, { return "bottom(\(percent.description))" })
}
| @discardableResult | ||
| func all(_ value: CGFloat) -> PinLayout { | ||
| func context() -> String { return "all(\(value))" } | ||
| top(value, context) |
There was a problem hiding this comment.
Could you do as in all(), i.e. set a different context string for all internal calls (ex: { "all(\(value)) top coordinate" })
| @discardableResult | ||
| func horizontally(_ value: CGFloat) -> PinLayout { | ||
| func context() -> String { return "horizontally(\(value))" } | ||
| left(value, context) |
There was a problem hiding this comment.
Could you do as in all(), i.e. set a different context string for all internal calls (ex: { "horizontally(\(value)) left coordinate" })
| @discardableResult | ||
| func horizontally(_ percent: Percent) -> PinLayout { | ||
| func context() -> String { return "horizontally(\(percent.description))" } | ||
| left(percent, context) |
There was a problem hiding this comment.
Could you do as in all(), i.e. set a different context string for all internal calls (ex: { "horizontally(\(value)) left coordinate" })
| @discardableResult | ||
| func vertically(_ value: CGFloat) -> PinLayout { | ||
| func context() -> String { return "vertically(\(value))" } | ||
| top(value, context) |
There was a problem hiding this comment.
Could you do as in all(), i.e. set a different context string for all internal calls (ex: { "vertically(\(value)) top coordinate" })
| @discardableResult | ||
| func vertically(_ percent: Percent) -> PinLayout { | ||
| func context() -> String { return "vertically(\(percent.description))" } | ||
| top(percent, context) |
There was a problem hiding this comment.
Could you do as in all(), i.e. set a different context string for all internal calls (ex: { "vertically(\(value)) top coordinate" })
| } | ||
| it("should warn") { | ||
| aView.pin.left(10).horizontally(20) | ||
| expect(Pin.lastWarningText).to(contain(["horizontally(20.0)", "won't be applied", "already been set to 10"])) |
There was a problem hiding this comment.
Once you have updated the method context, thanks to updating expectations, here it should contain left.
| expect(Pin.lastWarningText).to(contain(["vertically() bottom coordinate", "won't be applied", "already been set to 20"])) | ||
| } | ||
| it("should warn") { | ||
| aView.pin.top(10).vertically(20) |
There was a problem hiding this comment.
Once you have updated the method context, thanks to updating expectations, here it should contain top.
lucdion
left a comment
There was a problem hiding this comment.
Excellent, once you have fixed the little issues 👍
Codecov Report
@@ Coverage Diff @@
## master #111 +/- ##
==========================================
- Coverage 90.94% 90.91% -0.03%
==========================================
Files 34 34
Lines 6627 6718 +91
==========================================
+ Hits 6027 6108 +81
- Misses 600 610 +10
Continue to review full report at Codecov.
|
|
|
||
| @discardableResult | ||
| func vertically(_ value: CGFloat) -> PinLayout { | ||
| top(value, { return "vertically(\(value)) top" }) |
There was a problem hiding this comment.
Just missing the "coordinate" word: "vertically((value)) top coordinate".
Also, for the bottom.
|
|
||
| @discardableResult | ||
| func vertically(_ percent: Percent) -> PinLayout { | ||
| top(percent, { return "vertically(\(percent.description)) top" }) |
There was a problem hiding this comment.
Just missing the "coordinate" word: "vertically((value)) top coordinate".
Also, for the bottom.
No description provided.