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

Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fix autolayout warnings on device rotation
  • Loading branch information
@ifbarrera
ifbarrera committed May 6, 2019
commit d60f4aabac401ef86a5bd0815dd0035b60fe8d1a
43 changes: 35 additions & 8 deletions Kickstarter-iOS/Views/Controllers/RewardsCollectionViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class RewardsCollectionViewController: UICollectionViewController {
}()

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

static func instantiate(with project: Project, refTag: RefTag?) -> RewardsCollectionViewController {
Expand Down Expand Up @@ -65,7 +65,7 @@ final class RewardsCollectionViewController: UICollectionViewController {
super.viewDidLoad()

_ = self.collectionView
|> \.dataSource .~ dataSource
|> \.dataSource .~ self.dataSource

self.collectionView.register(RewardCell.self)

Expand All @@ -79,11 +79,15 @@ 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 👍


layout.itemSize = self.calculateItemSize(from: layout, using: self.collectionView)

self.updateHiddenScrollViewBoundsIfNeeded(for: layout)
}

override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) {
super.viewWillTransition(to: size, with: coordinator)

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() {
super.bindStyles()

Expand Down Expand Up @@ -170,21 +174,29 @@ final class RewardsCollectionViewController: UICollectionViewController {

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

let sectionInsets = layout.sectionInset
let topBottomInsets = sectionInsets.top + sectionInsets.bottom
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.contentSize.height - topBottomInsets
let itemHeight = collectionView.frame.height - topBottomSectionInsets - topBottomContentInsets
let itemWidth = cardWidth - leftRightInsets

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

// MARK: - Public Functions

@objc func closeButtonTapped() {
self.navigationController?.dismiss(animated: true, completion: nil)
self.navigationController?.dismiss(animated: true)
}
}

Expand All @@ -198,7 +210,22 @@ extension RewardsCollectionViewController {
}
}

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 = { collectionView -> UICollectionView in
collectionView
|> \.backgroundColor .~ .ksr_grey_200
Expand Down