[NT-394] - Loading Button #892
Conversation
| @@ -25,7 +25,7 @@ final class PledgePaymentMethodsViewController: UIViewController { | |||
| private lazy var cardsStackView: UIStackView = { UIStackView(frame: .zero) }() | |||
| internal weak var delegate: PledgePaymentMethodsViewControllerDelegate? | |||
| internal weak var messageDisplayingDelegate: PledgeViewControllerMessageDisplaying? | |||
| private lazy var pledgeButton: UIButton = { UIButton.init(type: .custom) }() | |||
| private(set) lazy var pledgeButton: LoadingButton = { LoadingButton(frame: .zero) }() | |||
There was a problem hiding this comment.
I can expose this another way but thought this was simplest for what we need to do (RAC binding)
| @@ -11,7 +11,7 @@ protocol PledgeViewControllerDelegate: AnyObject { | |||
| final class PledgeViewController: UIViewController, MessageBannerViewControllerPresenting { | |||
| // MARK: - Properties | |||
|
|
|||
| private lazy var confirmButton: UIButton = { UIButton(type: .custom) }() | |||
| private lazy var confirmButton: LoadingButton = { LoadingButton(frame: .zero) }() | |||
There was a problem hiding this comment.
Changing to LoadingButton as well as using designated initializer
| // MARK: - Properties | ||
|
|
||
| private lazy var activityIndicator: UIActivityIndicatorView = { | ||
| UIActivityIndicatorView(style: .white) |
There was a problem hiding this comment.
Style is something that could potentially be injected in the initializer in the future but for now I think this style will suffice
|
|
||
| private lazy var activityIndicator: UIActivityIndicatorView = { | ||
| UIActivityIndicatorView(style: .white) | ||
| |> \.hidesWhenStopped .~ true |
There was a problem hiding this comment.
This is the magic prop that does not require us to add/remove it from the view hierarchy...it will automatically show/hide the component depending on the isAnimating property.
| self.activityIndicator.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true | ||
| } | ||
|
|
||
| required init?(coder: NSCoder) { |
There was a problem hiding this comment.
Init with coder so that this supports XIBs
|
|
||
| override func setTitle(_ title: String?, for state: UIControl.State) { | ||
| // Do not allow changing the title while the activity indicator is animating | ||
| guard !self.activityIndicator.isAnimating else { return } |
There was a problem hiding this comment.
Setting a title while the activity indicator is loading would display both the indicator and the title so I think this is the right approach here..we can potentially add logic to stop animating if we're setting title but I think due to our RAC bindings it could cause some flickering
| self.setTitle(nil, for: normalState) | ||
| self.setTitle(nil, for: selectedState) | ||
|
|
||
| _ = self |
There was a problem hiding this comment.
This makes sure that during the loading phase it's still accessible..
| self.originalTitles[normalState.rawValue] = nil | ||
| self.originalTitles[selectedState.rawValue] = nil | ||
|
|
||
| _ = self |
There was a problem hiding this comment.
And this ensures that after loading is done it will again use title for the accessibility label
| } | ||
| } | ||
|
|
||
| private var originalTitles: [UInt: String] = [:] |
There was a problem hiding this comment.
Is there a reason you chose to do this without a view model?
There was a problem hiding this comment.
I simply tried to go with the simplest approach but can back this by a VM to be more consistent with the rest of the codebase 👍
| let normalState = UIControl.State.normal | ||
| let selectedState = UIControl.State.selected | ||
|
|
||
| if let disabledTitle = self.title(for: disabledState) { |
There was a problem hiding this comment.
Extracting the title first will make sure that if for some reason we want to start the label with a loading state, we don't just clean the titles, but rather only cache previous titles if there were any.
|
|
||
| // MARK: - Titles | ||
|
|
||
| private func removeTitle() { |
There was a problem hiding this comment.
What do you think about something like this for DRYness:
private func removeTitle() {
let states: [UIControl.State] = [.disabled, .highlighted, .normal, .selected]
states.compactMap { state -> (String, UIControl.State)? in
guard let title = self.title(for: state) else { return nil }
return (title, state)
}
.forEach { title, state in
self.originalTitles[state.rawValue] = title
self.setTitle(nil, for: state)
}
_ = self
|> \.accessibilityLabel %~ { _ in Strings.Loading() }
UIAccessibility.post(notification: .layoutChanged, argument: self)
}| UIAccessibility.post(notification: .layoutChanged, argument: self) | ||
| } | ||
|
|
||
| private func restoreTitle() { |
There was a problem hiding this comment.
You could probably do this here too.
|
PS. Thanks for everything! 🎉 |
📲 What
Adds a
LoadingButtonclass that could be used to display a button that has a loading state.🤔 Why
Our CTA buttons will be performing network requests so this came up as a design tweak to our checkout workflow.
🛠 How
The button has a
BoolpropertyisLoadingthat could be toggled ON/OFF in order to show the activity indicator or hide it. This in addition requires storing titles for couple states into a temporary dictionary so that these could be restored later when the loading is stopped.👀 See
Pledge
Update pledge
♿️ Accessibility
Loadingand it's localized (this could be verified by slowing down your connection using Network Link Conditioner and using VoiceOver to tap on the button✅ Acceptance criteria
Pledgebutton shows a spinner when tapped and during the network call on the pledge screenConfirmbutton shows a spinner when tapped and during the network call on the update pledge screen