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

[NT-262] Remember this card UI #848

Merged
merged 8 commits into from
Sep 24, 2019
Merged

[NT-262] Remember this card UI #848

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ee766a
Add remember this card switch to storyboard
justinswart Sep 20, 2019
ac82294
Add AddNewCardIntent, only show switch from pledge view
justinswart Sep 20, 2019
8779b12
Fix snapshot tests
justinswart Sep 20, 2019
1fe85fb
Formatting
justinswart Sep 20, 2019
fa3a55d
Remove incorrect intent in test
justinswart Sep 23, 2019
3fd0ed0
Merge branch 'master' into remember_this_card_ui
justinswart Sep 23, 2019
ff214a1
Use adaptable stack view
justinswart Sep 23, 2019
16fcd81
Use ToggleViewController, alphabetize
justinswart Sep 24, 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
25 changes: 22 additions & 3 deletions Kickstarter-iOS/Views/Controllers/AddNewCardViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ internal final class AddNewCardViewController: UIViewController,
@IBOutlet private var creditCardTextField: STPPaymentCardTextField!
@IBOutlet private var creditCardValidationErrorLabel: UILabel!
@IBOutlet private var creditCardValidationErrorContainer: UIView!
Copy link
Contributor

Choose a reason for hiding this comment

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

alphabetize

@IBOutlet private var reusableCardStackViewContainer: UIView!
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooooh, good old storyboard thingys 🍿

@IBOutlet private var reusableCardStackView: UIStackView!
@IBOutlet private var reusableCardLabel: UILabel!
@IBOutlet private var reusableCardSwitch: UISwitch!
@IBOutlet private var scrollView: UIScrollView!
@IBOutlet private var stackView: UIStackView!
@IBOutlet private var zipcodeView: SettingsFormFieldView!
Expand All @@ -46,6 +50,10 @@ internal final class AddNewCardViewController: UIViewController,
return Storyboard.Settings.instantiate(AddNewCardViewController.self)
}

func configure(with intent: AddNewCardIntent) {
self.viewModel.inputs.configure(with: intent)
}

override func viewDidLoad() {
super.viewDidLoad()

Expand Down Expand Up @@ -125,9 +133,6 @@ internal final class AddNewCardViewController: UIViewController,
_ = self.scrollView
|> \.alwaysBounceVertical .~ true

_ = self.stackView
|> \.layoutMargins .~ .init(leftRight: Styles.grid(2))

_ = self.zipcodeView.titleLabel
|> \.text %~ { _ in
localizedPostalCode()
Expand All @@ -136,11 +141,25 @@ internal final class AddNewCardViewController: UIViewController,
_ = self.zipcodeView
|> \.autocapitalizationType .~ .allCharacters
|> \.returnKeyType .~ .done

_ = self.reusableCardLabel
|> \.font .~ .ksr_body()
|> \.text %~ { _ in Strings.Remember_this_card() }

_ = self.reusableCardStackView
|> checkoutAdaptableStackViewStyle(
self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory
)

_ = self.reusableCardSwitch
|> baseSwitchControlStyle
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we would only have 1 accessible element (the toggle - whose focus area would be the whole cell) but because we're faking a table view cell here we don't get that functionality...so because of that I think we should also add VoiceOver label on the switch that will simply duplicate the label but since it's a UISwitch it will also use double tap to toggle hint by default.

Suggested change
|> baseSwitchControlStyle
|> baseSwitchControlStyle
|> \.accessibilityLabel %~ { _ in Strings.Remember_this_card() }

(not sure if this will pass SwiftFormat)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also note that there seems to be an issue with UISwitch and right alignment..I've ran into it in my PR - #850

Basically even if you align right edges the switch still overlaps a little. Checked this out in view inspector.

Screen Shot 2019-09-23 at 11 09 36 AM

Basically what helped was to align left edge of the switch to the right edge of superview minus the switch width.

}

override func bindViewModel() {
super.bindViewModel()

self.reusableCardStackViewContainer.rac.hidden = self.viewModel.outputs.reusableCardSwitchIsHidden
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we order this alphabetically (so before the saveButtonIsEnabled signal)?


self.creditCardValidationErrorContainer.rac.hidden =
self.viewModel.outputs.creditCardValidationErrorContainerHidden
self.cardholderNameTextField.rac.becomeFirstResponder =
Expand Down
14 changes: 14 additions & 0 deletions Kickstarter-iOS/Views/Controllers/AddNewCardViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ final class AddNewCardViewControllerTests: TestCase {
combos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(language: language) {
let controller = AddNewCardViewController.instantiate()
controller.configure(with: .settings)
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller)

FBSnapshotVerifyView(parent.view, identifier: "lang_\(language)_device_\(device)")
}
}
}

func testAddNewCard_PledgeViewIntent() {
combos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(language: language) {
let controller = AddNewCardViewController.instantiate()
controller.configure(with: .pledgeView)
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller)

FBSnapshotVerifyView(parent.view, identifier: "lang_\(language)_device_\(device)")
Expand All @@ -31,6 +44,7 @@ final class AddNewCardViewControllerTests: TestCase {
combos(locales, Device.allCases).forEach { locale, device in
withEnvironment(locale: locale) {
let controller = AddNewCardViewController.instantiate()
controller.configure(with: .settings)
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller)

FBSnapshotVerifyView(parent.view, identifier: "locale_\(locale)_device_\(device)")
Expand Down
9 changes: 5 additions & 4 deletions Kickstarter-iOS/Views/Controllers/PaymentMethodsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ internal final class PaymentMethodsViewController: UIViewController, MessageBann
self?.tableView.reloadData()
}

self.viewModel.outputs.goToAddCardScreen
self.viewModel.outputs.goToAddCardScreenWithIntent
.observeForUI()
.observeValues { [weak self] in
self?.goToAddCardScreen()
.observeValues { [weak self] intent in
self?.goToAddCardScreen(with: intent)
}

self.viewModel.outputs.errorLoadingPaymentMethods
Expand Down Expand Up @@ -137,8 +137,9 @@ internal final class PaymentMethodsViewController: UIViewController, MessageBann
self.viewModel.inputs.editButtonTapped()
}

private func goToAddCardScreen() {
private func goToAddCardScreen(with intent: AddNewCardIntent) {
let vc = AddNewCardViewController.instantiate()
vc.configure(with: intent)
vc.delegate = self
let nav = UINavigationController(rootViewController: vc)
nav.modalPresentationStyle = .formSheet
Expand Down
3 changes: 2 additions & 1 deletion Kickstarter-iOS/Views/Controllers/PledgePaymentMethodsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,10 @@ final class PledgePaymentMethodsViewController: UIViewController {
}

extension PledgePaymentMethodsViewController: PledgeAddNewCardViewDelegate {
func pledgeAddNewCardViewDidTapAddNewCard(_: PledgeAddNewCardView) {
func pledgeAddNewCardView(_: PledgeAddNewCardView, didTapAddNewCardWith intent: AddNewCardIntent) {
let addNewCardViewController = AddNewCardViewController.instantiate()
|> \.delegate .~ self
addNewCardViewController.configure(with: intent)
let navigationController = UINavigationController.init(rootViewController: addNewCardViewController)
let offset = navigationController.navigationBar.bounds.height

Expand Down
8 changes: 4 additions & 4 deletions Kickstarter-iOS/Views/PledgeAddNewCardView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Prelude
import UIKit

protocol PledgeAddNewCardViewDelegate: AnyObject {
func pledgeAddNewCardViewDidTapAddNewCard(_ view: PledgeAddNewCardView)
func pledgeAddNewCardView(_ view: PledgeAddNewCardView, didTapAddNewCardWith intent: AddNewCardIntent)
}

final class PledgeAddNewCardView: UIView {
Expand Down Expand Up @@ -60,12 +60,12 @@ final class PledgeAddNewCardView: UIView {
override func bindViewModel() {
super.bindViewModel()

self.viewModel.outputs.notifyDelegateAddNewCardTapped
self.viewModel.outputs.notifyDelegateAddNewCardTappedWithIntent
.observeForControllerAction()
.observeValues { [weak self] in
.observeValues { [weak self] intent in
guard let self = self else { return }

self.delegate?.pledgeAddNewCardViewDidTapAddNewCard(self)
self.delegate?.pledgeAddNewCardView(self, didTapAddNewCardWith: intent)
}
}

Expand Down
Loading