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.

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

Rewards Collection View Footer #786

Merged
merged 12 commits into from
Aug 8, 2019
Merged

Rewards Collection View Footer #786

merged 12 commits into from
Aug 8, 2019

Conversation

ifbarrera
Copy link
Contributor

@ifbarrera ifbarrera commented Aug 7, 2019

πŸ“² What

Adds a footer below the rewards collection view that shows the total number of rewards.

πŸ€” Why

To help users orient themselves within the rewards browsing experience.

πŸ›  How

My first instinct was to try to use supplementaryViews which come for free with UICollectionView. Unfortunately though I quickly realized that this wouldn't work because UICollectionView only supports section footer/header "supplementary" views - because we have a horizontally scrolling collection view, it was then showing the footer at the "end" of the section of rewards, which is basically on the right side of the last reward. This wasn't want we wanted.

The current approach is to add a footer view as a subview of the RewardCollectionViewController's view. The footer view is configured with the number of rewards to display, and is shown/hidden depending on whether the current traitCollection.verticalSizeClass is .regular or .compact.

πŸ‘€ See

Trello, screenshots, external resources?

iPhone X iPhone 5S iPad
Simulator Screen Shot - iPhone X - 2019-08-07 at 16 06 43 Simulator Screen Shot - iPhone 5s - 2019-08-07 at 16 40 17 Simulator Screen Shot - iPad Pro (12 9-inch) - 2019-08-07 at 16 08 50

iPhone Landscape:

Simulator Screen Shot - iPhone X - 2019-08-07 at 16 06 52

efOUgww2vr

♿️ Accessibility

  • Tap targets use minimum of 44x44 pts dimensions
  • Works with VoiceOver
  • Supports Dynamic Type

🏎 Performance

  • Optimized Blended Layers (screenshots)

βœ… Acceptance criteria

On iPhone 5, 6, 7, 8, X

  • navigate to any project, tap "Back this project", the rewards carousel should show the footer in portrait mode, but not show it in landscape mode

On iPhone XS max, iPad

  • navigate to any project, tap "Back this project", the rewards carousel should show the footer in portrait mode, and also show it in landscape mode

⏰ TODO

  • move reward count configuration to view model output
  • add accessibility label to the count label

@ifbarrera ifbarrera marked this pull request as ready for review August 8, 2019 15:33
Copy link
Contributor

@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

Looking pretty legit! Did an first pass on the code and had some minor comments.

Unfortunately this does also make the transition look a little weird for long rewards, but we can fix that separately, it's also looking a little strange since the gradient was added πŸ€¦β€β™‚ All those things need to be accounted for during the animation.

self.viewModel.outputs.configureRewardsCollectionViewFooterWithCount
.observeForUI()
.observeValues { [weak self] count in
self?.updateRewardsCollectionFooterView(with: count)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we match this function name to the output name? Just for consistency and easier to find things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor

@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

Nice!

@ifbarrera ifbarrera merged commit 60ecd8c into master Aug 8, 2019
@ifbarrera ifbarrera deleted the rewards-carousel-footer branch August 8, 2019 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants