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.

Sign up for GitHub

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

💲[Native Checkout] "Peek" functionality using hidden scroll view #665

Merged
merged 19 commits into from
May 10, 2019
Merged

💲[Native Checkout] "Peek" functionality using hidden scroll view #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9406300
New files
ifbarrera Apr 18, 2019
b0aa974
removing scroll view stuff for now
ifbarrera Apr 19, 2019
85db760
Tests
ifbarrera Apr 22, 2019
48a3e1f
Cleaning up insets
ifbarrera Apr 22, 2019
5fcf780
More cleanup
ifbarrera Apr 22, 2019
21216ac
Swiftlint
ifbarrera Apr 22, 2019
2e4ddc2
Adding “deprecated” to more file names
ifbarrera Apr 22, 2019
8b7f848
Removing duplicated file
ifbarrera Apr 22, 2019
a405611
Increasing hit area of close button
ifbarrera Apr 22, 2019
7aa0272
Hidden scroll view
ifbarrera Apr 23, 2019
6c92d7d
Hidden scroll view working
ifbarrera Apr 23, 2019
992f1e4
Refactoring & cleanup
ifbarrera Apr 23, 2019
21ba760
Use collection view’s content size
ifbarrera Apr 23, 2019
88ef739
PR updates from comments
ifbarrera May 6, 2019
a31a1ae
Merge branch 'feature-native-checkout' of https://github.com/kickstar…
ifbarrera May 6, 2019
d60f4aa
Fix autolayout warnings on device rotation
ifbarrera May 6, 2019
1037763
Cleanup
ifbarrera May 6, 2019
e5d3515
More cleanup
ifbarrera May 7, 2019
c9eb6ec
More PR feedback
ifbarrera May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 129 additions & 17 deletions Kickstarter-iOS/Views/Controllers/RewardsCollectionViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ import Library
import KsApi
import Prelude

final class RewardsCollectionViewController: UICollectionViewController {
private enum Layout {
enum Card {
static let width: CGFloat = 249
}
}

final class RewardsCollectionViewController: UICollectionViewController {
// MARK: - Properties

private let dataSource = RewardsCollectionViewDataSource()
Expand All @@ -19,16 +24,15 @@ final class RewardsCollectionViewController: UICollectionViewController {
private let layout: UICollectionViewFlowLayout = {
UICollectionViewFlowLayout()
|> \.minimumLineSpacing .~ Styles.grid(3)
|> \.sectionInset .~ .init(all: Styles.grid(6))
|> \.minimumInteritemSpacing .~ 0
|> \.sectionInset .~ .init(topBottom: Styles.grid(6))
|> \.scrollDirection .~ .horizontal
}()

private var flowLayout: UICollectionViewFlowLayout? {
return self.collectionView.collectionViewLayout as? UICollectionViewFlowLayout
}

private let peekAmountInset = Styles.grid(3)

static func instantiate(with project: Project, refTag: RefTag?) -> RewardsCollectionViewController {
let rewardsCollectionVC = RewardsCollectionViewController()
rewardsCollectionVC.viewModel.inputs.configure(with: project, refTag: refTag)
Expand Down Expand Up @@ -66,6 +70,8 @@ final class RewardsCollectionViewController: UICollectionViewController {

self.collectionView.register(RewardCell.self)

self.configureHiddenScrollView()

self.viewModel.inputs.viewDidLoad()
}

Expand All @@ -74,18 +80,13 @@ final class RewardsCollectionViewController: UICollectionViewController {

guard let layout = self.flowLayout else { return }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an ideal scenario...this guard is repeated twice in the flow...first time here...second time in updateHiddenScrollViewBoundsIfNeeded. In order to make each function independent of the state of the view controller could we do this

// Callsite
guard let layout = self.flowLayout else { return }

// or maybe even also injecting layout to a new function and extracting this to `updateItemSize(for: layout)`
layout.itemSize = self.calculateItemSize(from: layout, using: self.collectionView)

self.updateHiddenScrollViewBoundsIfNeeded(for: layout)

// Updated function input
private func updateHiddenScrollViewBoundsIfNeeded(for layout: UICollectionViewFlowLayout) {
  // Then we don't need to guard here
}

// Possibly new function
private func updateItemSize(for layout: UICollectionViewFlowLayout) {
  // Then we don't need to guard here
}

or even something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, will update the updateHiddenScrollViewBoundsIfNeeded to accept layout as an argument 👍


let sectionInsets = layout.sectionInset
let topBottomInsets = sectionInsets.top + sectionInsets.bottom
let collectionViewSize = self.collectionView.frame.size

let itemHeight = self.collectionView.contentSize.height - topBottomInsets
var itemWidth = collectionViewSize.width - sectionInsets.left - 2 * peekAmountInset
self.updateHiddenScrollViewBoundsIfNeeded(for: layout)
}

if [.landscapeLeft, .landscapeRight].contains(UIDevice.current.orientation) {
itemWidth = collectionViewSize.width / 3 - sectionInsets.left - 2 * peekAmountInset
}
override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) {
super.viewWillTransition(to: size, with: coordinator)

layout.itemSize = CGSize(width: itemWidth, height: itemHeight)
self.flowLayout?.invalidateLayout()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed that no matter what the current horizontal offset rotating the device always resets it to 0 therefore scrolls back to the very first reward. There should be a way to either cache current page and upon rotation restore the scroll offset (maybe inside the coordinator completion handler)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to implement this separately as an enhancement. I'll create a card 👍

}

override func bindStyles() {
Expand All @@ -96,6 +97,9 @@ final class RewardsCollectionViewController: UICollectionViewController {

_ = self.collectionView
|> collectionViewStyle

_ = self.collectionView.panGestureRecognizer
|> \.isEnabled .~ false
}

override func bindViewModel() {
Expand All @@ -105,7 +109,89 @@ final class RewardsCollectionViewController: UICollectionViewController {
.observeForUI()
.observeValues { [weak self] rewards in
self?.dataSource.load(rewards: rewards)
self?.collectionView.reloadData()
}
}

// MARK: - Private Helpers

private func configureHiddenScrollView() {
_ = self.hiddenPagingScrollView
|> \.delegate .~ self

_ = (self.hiddenPagingScrollView, self.view)
|> ksr_insertSubviewInParent(at: 0)

self.collectionView.addGestureRecognizer(self.hiddenPagingScrollView.panGestureRecognizer)
}

private func updateHiddenScrollViewBoundsIfNeeded(for layout: UICollectionViewFlowLayout) {
let (contentSize, pageSize, contentInsetLeftRight) = self.hiddenScrollViewData(from: layout,
using: self.collectionView)
let needsUpdate = self.collectionView.contentInset.left != contentInsetLeftRight
|| self.hiddenPagingScrollView.contentSize != contentSize

// Check if orientation or frame has changed
guard needsUpdate else {
return
}

_ = self.hiddenPagingScrollView
|> \.frame .~ self.collectionView.frame
|> \.bounds .~ CGRect(x: 0, y: 0, width: pageSize.width, height: pageSize.height)
|> \.contentSize .~ CGSize(width: contentSize.width, height: contentSize.height)

let (top, bottom) = self.collectionView.contentInset.topBottom

_ = self.collectionView
|> \.contentInset .~ .init(top: top,
left: contentInsetLeftRight,
bottom: bottom,
right: contentInsetLeftRight)

self.collectionView.contentOffset.x = -contentInsetLeftRight
}

private typealias HiddenScrollViewData = (contentSize: CGSize, pageSize: CGSize,
contentInsetLeftRight: CGFloat)

private func hiddenScrollViewData(from layout: UICollectionViewFlowLayout,
using collectionView: UICollectionView) -> HiddenScrollViewData {
let itemSize = layout.itemSize
let lineSpacing = layout.minimumLineSpacing
let totalItemWidth = itemSize.width + lineSpacing

let pageWidth = totalItemWidth
let pageHeight = itemSize.height
let pageSize = CGSize(width: pageWidth, height: pageHeight)

let contentSize = CGSize(width: collectionView.contentSize.width + lineSpacing,
height: collectionView.contentSize.height)

let contentInsetLeftRight = (collectionView.frame.width - itemSize.width) / 2

return (contentSize, pageSize, contentInsetLeftRight)
}

private func calculateItemSize(from layout: UICollectionViewFlowLayout,
using collectionView: UICollectionView) -> CGSize {
let cardWidth = Layout.Card.width

let sectionInsets = layout.sectionInset
var adjustedContentInset = UIEdgeInsets.zero

if #available(iOS 11.0, *) {
adjustedContentInset = collectionView.adjustedContentInset
}

let topBottomSectionInsets = sectionInsets.top + sectionInsets.bottom
let topBottomContentInsets = adjustedContentInset.top + adjustedContentInset.bottom
let leftRightInsets = sectionInsets.left + sectionInsets.right

let itemHeight = collectionView.frame.height - topBottomSectionInsets - topBottomContentInsets
let itemWidth = cardWidth - leftRightInsets

return CGSize(width: itemWidth, height: itemHeight)
}

// MARK: - Public Functions
Expand All @@ -115,13 +201,39 @@ final class RewardsCollectionViewController: UICollectionViewController {
}
}

// MARK: - Styles
// MARK: - UIScrollViewDelegate

extension RewardsCollectionViewController {
override func scrollViewDidScroll(_ scrollView: UIScrollView) {
guard scrollView == self.hiddenPagingScrollView else { return }

let leftInset = self.collectionView.contentInset.left

self.collectionView.contentOffset.x = scrollView.contentOffset.x - leftInset
}
}

// MARK: - UICollectionViewDelegateFlowLayout

extension RewardsCollectionViewController: UICollectionViewDelegateFlowLayout {
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout,
sizeForItemAt indexPath: IndexPath) -> CGSize {
guard let layout = collectionViewLayout as? UICollectionViewFlowLayout else {
return .zero
}

// Cache the itemSize so we can recalculate hidden scroll view data efficiently
layout.itemSize = self.calculateItemSize(from: layout, using: collectionView)

return layout.itemSize
}
}

// MARK: Styles

private var collectionViewStyle: CollectionViewStyle = { collectionView -> UICollectionView in
collectionView
|> \.alwaysBounceHorizontal .~ true
|> \.backgroundColor .~ .ksr_grey_200
|> \.isPagingEnabled .~ true
|> \.clipsToBounds .~ false
|> \.allowsSelection .~ true
}
4 changes: 4 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
778215EB20F79FA300F3D09F /* HelpDataSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 778215EA20F79FA300F3D09F /* HelpDataSourceTests.swift */; };
778215EE20F7AB8300F3D09F /* HelpViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 778215EC20F7AA5F00F3D09F /* HelpViewControllerTests.swift */; };
77891BDE20CEB6DB00B46D5D /* ThanksProjectsDataSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77891BA720CEB6DB00B46D5D /* ThanksProjectsDataSourceTests.swift */; };
778CCCDA2285BF8900FB8D35 /* UIView+AutolayoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 778CCCD92285BF8900FB8D35 /* UIView+AutolayoutTests.swift */; };
7790DF502200D395005DBB11 /* SettingsFormFieldView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7790DF4F2200D395005DBB11 /* SettingsFormFieldView.swift */; };
7790DF892200D3BD005DBB11 /* SettingsFormFieldView.xib in Resources */ = {isa = PBXBuildFile; fileRef = 7790DF882200D3BD005DBB11 /* SettingsFormFieldView.xib */; };
7793B16D21077AEB007857C0 /* SettingsHeaderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7793B16C21077AEB007857C0 /* SettingsHeaderView.swift */; };
Expand Down Expand Up @@ -2017,6 +2018,7 @@
778215EA20F79FA300F3D09F /* HelpDataSourceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelpDataSourceTests.swift; sourceTree = "<group>"; };
778215EC20F7AA5F00F3D09F /* HelpViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HelpViewControllerTests.swift; sourceTree = "<group>"; };
77891BA720CEB6DB00B46D5D /* ThanksProjectsDataSourceTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThanksProjectsDataSourceTests.swift; sourceTree = "<group>"; };
778CCCD92285BF8900FB8D35 /* UIView+AutolayoutTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIView+AutolayoutTests.swift"; sourceTree = "<group>"; };
7790DF4F2200D395005DBB11 /* SettingsFormFieldView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsFormFieldView.swift; sourceTree = "<group>"; };
7790DF882200D3BD005DBB11 /* SettingsFormFieldView.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = SettingsFormFieldView.xib; sourceTree = "<group>"; };
7793B16C21077AEB007857C0 /* SettingsHeaderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsHeaderView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3899,6 +3901,7 @@
37F39BB4221D05FA00E0FA65 /* UITraitCollection.swift */,
37F39BED221D062900E0FA65 /* UITraitCollectionTests.swift */,
37BB45C82243031100E7392A /* UIView+AutoLayout.swift */,
778CCCD92285BF8900FB8D35 /* UIView+AutolayoutTests.swift */,
A733795F1D0EDFEE00C91445 /* UIViewController-Preparation.swift */,
370BE71522541C8100B44DB2 /* UIViewController+URL.swift */,
370BE74E22541C8F00B44DB2 /* UIViewController+URLTests.swift */,
Expand Down Expand Up @@ -6183,6 +6186,7 @@
A7ED1FAF1E831C5C00BFFA01 /* LiveStreamDiscoveryViewModelTests.swift in Sources */,
A7ED20011E831C5C00BFFA01 /* SignupViewModelTests.swift in Sources */,
A7ED1FF71E831C5C00BFFA01 /* ResetPasswordViewModelTests.swift in Sources */,
778CCCDA2285BF8900FB8D35 /* UIView+AutolayoutTests.swift in Sources */,
373AB25F222A0DAC00769FC2 /* PasswordValidationTests.swift in Sources */,
D6C9A20E1F755FE200981E64 /* GraphSchemaTests.swift in Sources */,
A7ED1FF51E831C5C00BFFA01 /* ProjectNavBarViewModelTests.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Library/UIView+AutoLayout.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ public func ksr_addSubviewToParent() -> ((UIView, UIView) -> (UIView, UIView)) {
}
}

public func ksr_insertSubviewInParent(at index: Int) -> ((UIView, UIView) -> (UIView, UIView)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized we don't have tests for these? Shouldn't be so hard to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually don't have tests for any of these functions, but sure I can add one.

return { (subview, parent) in
parent.insertSubview(subview, at: index)
return (subview, parent)
}
}

public func ksr_constrainViewToEdgesInParent(priority: UILayoutPriority = .required)
-> ((UIView, UIView) -> (UIView, UIView)) {
return { (subview, parent) in
Expand Down
16 changes: 16 additions & 0 deletions Library/UIView+AutolayoutTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import XCTest
@testable import Library

final class UIViewAutoLayoutExtensionTests: TestCase {
func test_insertSubviewInParentAtIndex_oneSubview() {
let view1 = UIView(frame: .zero)
let view2 = UIView(frame: .zero)

_ = (view2, view1)

_ = ksr_insertSubviewInParent(at: 0)(view2, view1)

XCTAssertEqual(view1.subviews.count, 1)
XCTAssertEqual(view1.subviews[0], view2)
}
}