Conversation
|
|
jordanpichler
left a comment
jordanpichler
left a comment
There was a problem hiding this comment.
Some minor style comments 🙂
| guard | ||
| let scrollView = presentable?.panScrollable, | ||
| !scrollView.isScrolling | ||
| !scrollView.isScrolling && presentable?.shouldConfigureScrollViewInsets == true |
There was a problem hiding this comment.
To keep the coding style consistent, I'd suggest
- replacing the
&&operator with a,like in the previous statement - remove the
== trueasshouldConfigureScrollViewInsetsis a boolean - giving this it's own line and not appending to the previous for better legibility
| */ | ||
| func haltScrolling(_ scrollView: UIScrollView) { | ||
| scrollView.setContentOffset(CGPoint(x: 0, y: scrollViewYOffset), animated: false) | ||
| guard presentable?.shouldHaltScroll != false else { return } |
There was a problem hiding this comment.
again, no need to check if something is not false.
My suggestion:
if presentable?.shouldHaltScroll { return }
which reads a bit easier 😉
| scrollView.showsVerticalScrollIndicator = true | ||
| scrollViewXOffset = max(scrollView.contentOffset.x, 0) | ||
|
|
||
| if presentable?.shouldConfigureScrollViewInsets == true { |
There was a problem hiding this comment.
if presentable?.shouldConfigureScrollViewInsets without the == true
| ``` | ||
| extension YourViewController: PanModalPresentable { | ||
| func shouldRoundTopCorners: Bool { return false } | ||
| func shouldRoundTopCorners: Bool { return false } |
There was a problem hiding this comment.
Your IDE may have done this, but these whitespaces probably are deliberate and should remain in this comment 😄
| */ | ||
| var longFormHeight: PanModalHeight { get } | ||
|
|
||
There was a problem hiding this comment.
Looks like your IDE filled in whitespaces for no reason here. Double check if you Xcode trims these.
Xcode -> Preferences -> Text Editing -> Editing -> While Editing -> CheckAutomatically trim trailing whitespace and Including whitespace-only lines
3 participants
Summary
Currently PanModal only supports vertical scroll behavior, but there are cases where horizontal support may be needed, for example a horizontally scrolling set of images. This PR hopes to add that support to this library.
Requirements (place an
xin each[ ])I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.
I've written tests to cover the new code and functionality included in this PR.
Note: I'm not sure if this requires additional tests from what I could see.