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

Jump to bottom

[1/2] Shipping rules - Selection UI #823

Merged
merged 11 commits into from
Sep 4, 2019
Merged

[1/2] Shipping rules - Selection UI #823

merged 11 commits into from
Sep 4, 2019

Conversation

dusi
Copy link
Contributor

@dusi dusi commented Sep 1, 2019

📲 What

Improves the UI so that it provides UX for selecting shipping rule from a list

🤔 Why

First of two PRs allowing users changing shipping rule.

👀 See

shipping rule selection

✅ Acceptance criteria

  • Tapping on shipping rule that has not been previously selected deselects previous selection and selects the new shipping rule
  • Scrolling newly selected shipping rule off screen and then scrolling it back on shows the proper rule selected (this proves that data source has been updated properly)
  • Tapping on shipping rule that has been previously selected doesn't do anything
  • The modal opens with selected shipping rule scrolled to top
  • The modal opens and flashes scroll indicators

⏰ TODO

  • Dismiss shipping rule modal on selection (will be done in a separate PR)

@ksr-ci-bot
Copy link

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@dusi dusi changed the title [1/2] Adds the ability to select a shipping rule [1/2] Shipping rules - Selection UI Sep 1, 2019
@dusi dusi assigned ifbarrera and unassigned Scollaco Sep 1, 2019
@dusi dusi removed the request for review from ifbarrera September 1, 2019 01:59
ifbarrera
ifbarrera reviewed Sep 3, 2019
View reviewed changes
Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

Cool this works well! Couple comments.

Kickstarter-iOS/Views/Controllers/ShippingRulesTableViewController.swift Show resolved Hide resolved
Library/ViewModels/ShippingRuleCellViewModel.swift Outdated Show resolved Hide resolved
Library/ViewModels/ShippingRulesViewModelTests.swift Outdated Show resolved Hide resolved
Library/ViewModels/ShippingRulesViewModelTests.swift Show resolved Hide resolved
Library/ViewModels/ShippingRulesViewModelTests.swift Show resolved Hide resolved
@dusi dusi requested a review from ifbarrera September 3, 2019 20:57
ifbarrera
ifbarrera approved these changes Sep 4, 2019
View reviewed changes
Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

Seems chill

@dusi dusi merged commit 4391c1e into master Sep 4, 2019
@dusi dusi deleted the shipping-rules-selection branch October 4, 2019 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ifbarrera ifbarrera ifbarrera approved these changes

@cdolm92 cdolm92 Awaiting requested review from cdolm92

Assignees

@ifbarrera ifbarrera

Labels
needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dusi @ksr-ci-bot @ifbarrera @Scollaco