[NT-262] Remember this card UI #848
Conversation
There was a problem hiding this comment.
Looks good...
Had couple comments and noticed that Dynamic Type isn't supported ... I remember us chatting about it offline. One suggestion that maybe we should consider ... I've been working on ToggleViewController that would be easily reused here + it supports Dynamic Type.
| @@ -22,6 +22,10 @@ internal final class AddNewCardViewController: UIViewController, | |||
| @IBOutlet private var creditCardTextField: STPPaymentCardTextField! | |||
| @IBOutlet private var creditCardValidationErrorLabel: UILabel! | |||
| @IBOutlet private var creditCardValidationErrorContainer: UIView! | |||
| @IBOutlet private var reusableCardStackViewContainer: UIView! | |||
There was a problem hiding this comment.
Ooooh, good old storyboard thingys 🍿
| |> \.alignment .~ .center | ||
|
|
||
| _ = self.reusableCardSwitch | ||
| |> baseSwitchControlStyle |
There was a problem hiding this comment.
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.
| |> baseSwitchControlStyle | |
| |> baseSwitchControlStyle | |
| |> \.accessibilityLabel %~ { _ in Strings.Remember_this_card() } |
(not sure if this will pass SwiftFormat)
There was a problem hiding this comment.
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.
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 |
There was a problem hiding this comment.
Should we order this alphabetically (so before the saveButtonIsEnabled signal)?
Library/AddNewCardIntent.swift
Outdated
| import Foundation | ||
|
|
||
| public enum AddNewCardIntent: Equatable { | ||
| case pledgeView |
There was a problem hiding this comment.
Do we even need the View suffix? Simply just pledge 🤔
| @@ -36,6 +37,7 @@ public protocol AddNewCardViewModelOutputs { | |||
| var dismissKeyboard: Signal<Void, Never> { get } | |||
| var paymentDetails: Signal<PaymentDetails, Never> { get } | |||
| var paymentDetailsBecomeFirstResponder: Signal<Void, Never> { get } | |||
| var reusableCardSwitchIsHidden: Signal<Bool, Never> { get } | |||
There was a problem hiding this comment.
Does this name not match reusableCardStackViewContainer intentionally for it being too long?
| @@ -22,6 +22,10 @@ internal final class AddNewCardViewController: UIViewController, | |||
| @IBOutlet private var creditCardTextField: STPPaymentCardTextField! | |||
| @IBOutlet private var creditCardValidationErrorLabel: UILabel! | |||
| @IBOutlet private var creditCardValidationErrorContainer: UIView! | |||
| @@ -23,7 +23,7 @@ internal final class PaymentMethodsViewModelTests: TestCase { | |||
|
|
|||
| self.vm.outputs.editButtonIsEnabled.observe(self.editButtonIsEnabled.observer) | |||
| self.vm.outputs.editButtonTitle.observe(self.editButtonTitle.observer) | |||
| self.vm.outputs.goToAddCardScreen.observe(self.goToAddCardScreen.observer) | |||
| self.vm.outputs.goToAddCardScreenWithIntent.observe(self.goToAddCardScreenWithIntent.observer) | |||
📲 What
Adds a Remember this card switch to
AddNewCardViewControllerwhen being presented fromPledgeViewController.Note: VM binding and GraphQL requests will be added in upcoming PR.
🤔 Why
This allows the user to indicate that a card used during checkout should be stored for future backings.
🛠 How
AddNewCardIntent.👀 See
♿️ Accessibility
✅ Acceptance criteria
AddNewCardViewControlleris presented fromPledgeViewController.AddNewCardViewControlleris presented from Settings.⏰ TODO