-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[NT-394] - Loading Button #892
Changes from 16 commits
4794e14
eac31d3
fe1be2e
b68e868
4b8c628
1326e7b
db3938b
aa6ea49
e214021
d57058a
271ff74
864522b
bfdb793
7598cce
073d592
9e48387
d6ecc39
f7a1724
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
import Library | ||
import Prelude | ||
import UIKit | ||
|
||
final class LoadingButton: UIButton { | ||
// MARK: - Properties | ||
|
||
private let viewModel: LoadingButtonViewModelType = LoadingButtonViewModel() | ||
|
||
private lazy var activityIndicator: UIActivityIndicatorView = { | ||
UIActivityIndicatorView(style: .white) | ||
|> \.hidesWhenStopped .~ true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the magic |
||
|> \.translatesAutoresizingMaskIntoConstraints .~ false | ||
}() | ||
|
||
public var isLoading: Bool = false { | ||
didSet { | ||
self.viewModel.inputs.isLoading(self.isLoading) | ||
} | ||
} | ||
|
||
private var originalTitles: [UInt: String] = [:] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason you chose to do this without a view model? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 👍 |
||
|
||
// MARK: - Lifecycle | ||
|
||
override init(frame: CGRect) { | ||
super.init(frame: frame) | ||
|
||
self.configureViews() | ||
self.bindViewModel() | ||
} | ||
|
||
required init?(coder: NSCoder) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Init with coder so that this supports XIBs |
||
super.init(coder: coder) | ||
|
||
self.configureViews() | ||
self.bindViewModel() | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
super.setTitle(title, for: state) | ||
} | ||
|
||
// MARK: - Configuration | ||
|
||
private func configureViews() { | ||
self.addSubview(self.activityIndicator) | ||
self.activityIndicator.centerXAnchor.constraint(equalTo: self.centerXAnchor).isActive = true | ||
self.activityIndicator.centerYAnchor.constraint(equalTo: self.centerYAnchor).isActive = true | ||
} | ||
|
||
// MARK: - View model | ||
|
||
override func bindViewModel() { | ||
super.bindViewModel() | ||
|
||
self.viewModel.outputs.isUserInteractionEnabled | ||
.observeForUI() | ||
.observeValues { [weak self] isUserInteractionEnabled in | ||
_ = self | ||
?|> \.isUserInteractionEnabled .~ isUserInteractionEnabled | ||
} | ||
|
||
self.viewModel.outputs.startLoading | ||
.observeForUI() | ||
.observeValues { [weak self] in | ||
self?.startLoading() | ||
} | ||
|
||
self.viewModel.outputs.stopLoading | ||
.observeForUI() | ||
.observeValues { [weak self] in | ||
self?.stopLoading() | ||
} | ||
} | ||
|
||
// MARK: - Loading | ||
|
||
private func startLoading() { | ||
self.removeTitle() | ||
|
||
self.activityIndicator.startAnimating() | ||
} | ||
|
||
private func stopLoading() { | ||
self.activityIndicator.stopAnimating() | ||
|
||
self.restoreTitle() | ||
} | ||
|
||
// MARK: - Titles | ||
|
||
private func removeTitle() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
} |
||
let disabledState = UIControl.State.disabled | ||
let highlightedState = UIControl.State.highlighted | ||
let normalState = UIControl.State.normal | ||
let selectedState = UIControl.State.selected | ||
|
||
if let disabledTitle = self.title(for: disabledState) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
self.originalTitles[disabledState.rawValue] = disabledTitle | ||
self.setTitle(nil, for: disabledState) | ||
} | ||
|
||
if let highlightedTitle = self.title(for: highlightedState) { | ||
self.originalTitles[highlightedState.rawValue] = highlightedTitle | ||
self.setTitle(nil, for: highlightedState) | ||
} | ||
|
||
if let normalTitle = self.title(for: normalState) { | ||
self.originalTitles[normalState.rawValue] = normalTitle | ||
self.setTitle(nil, for: normalState) | ||
} | ||
|
||
if let selectedTitle = self.title(for: selectedState) { | ||
self.originalTitles[selectedState.rawValue] = selectedTitle | ||
self.setTitle(nil, for: selectedState) | ||
} | ||
|
||
_ = self | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sure that during the loading phase it's still accessible.. |
||
|> \.accessibilityLabel %~ { _ in Strings.Loading() } | ||
|
||
UIAccessibility.post(notification: .layoutChanged, argument: self) | ||
} | ||
|
||
private func restoreTitle() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably do this here too. |
||
let disabledState = UIControl.State.disabled | ||
let highlightedState = UIControl.State.highlighted | ||
let normalState = UIControl.State.normal | ||
let selectedState = UIControl.State.selected | ||
|
||
if let disabledTitle = self.originalTitles[disabledState.rawValue] { | ||
self.setTitle(disabledTitle, for: disabledState) | ||
self.originalTitles[disabledState.rawValue] = nil | ||
} | ||
|
||
if let highlightedTitle = self.originalTitles[highlightedState.rawValue] { | ||
self.setTitle(highlightedTitle, for: highlightedState) | ||
self.originalTitles[highlightedState.rawValue] = nil | ||
} | ||
|
||
if let normalTitle = self.originalTitles[normalState.rawValue] { | ||
self.setTitle(normalTitle, for: normalState) | ||
self.originalTitles[normalState.rawValue] = nil | ||
} | ||
|
||
if let selectedTitle = self.originalTitles[selectedState.rawValue] { | ||
self.setTitle(selectedTitle, for: selectedState) | ||
self.originalTitles[selectedState.rawValue] = nil | ||
} | ||
|
||
_ = self | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this ensures that after loading is done it will again use title for the accessibility label |
||
|> \.accessibilityLabel .~ nil | ||
|
||
UIAccessibility.post(notification: .layoutChanged, argument: self) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import Prelude | ||
import ReactiveSwift | ||
|
||
public protocol LoadingButtonViewModelInputs { | ||
func isLoading(_ isLoading: Bool) | ||
} | ||
|
||
public protocol LoadingButtonViewModelOutputs { | ||
var isUserInteractionEnabled: Signal<Bool, Never> { get } | ||
var startLoading: Signal<Void, Never> { get } | ||
var stopLoading: Signal<Void, Never> { get } | ||
} | ||
|
||
public protocol LoadingButtonViewModelType { | ||
var inputs: LoadingButtonViewModelInputs { get } | ||
var outputs: LoadingButtonViewModelOutputs { get } | ||
} | ||
|
||
public final class LoadingButtonViewModel: | ||
LoadingButtonViewModelType, | ||
LoadingButtonViewModelInputs, | ||
LoadingButtonViewModelOutputs { | ||
public init() { | ||
let isLoading = self.isLoadingProperty.signal | ||
.skipNil() | ||
.skipRepeats() | ||
|
||
self.isUserInteractionEnabled = isLoading | ||
.negate() | ||
|
||
self.startLoading = isLoading | ||
.filter(isTrue) | ||
.ignoreValues() | ||
|
||
self.stopLoading = isLoading | ||
.filter(isFalse) | ||
.ignoreValues() | ||
} | ||
|
||
private let isLoadingProperty = MutableProperty<Bool?>(nil) | ||
public func isLoading(_ isLoading: Bool) { | ||
self.isLoadingProperty.value = isLoading | ||
} | ||
|
||
public let isUserInteractionEnabled: Signal<Bool, Never> | ||
public let startLoading: Signal<Void, Never> | ||
public let stopLoading: Signal<Void, Never> | ||
|
||
public var inputs: LoadingButtonViewModelInputs { return self } | ||
public var outputs: LoadingButtonViewModelOutputs { return self } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
@testable import Library | ||
import ReactiveExtensions | ||
import ReactiveExtensions_TestHelpers | ||
import ReactiveSwift | ||
import XCTest | ||
|
||
final class LoadingButtonViewModelTests: TestCase { | ||
private let vm: LoadingButtonViewModelType = LoadingButtonViewModel() | ||
|
||
private let isUserInteractionEnabled = TestObserver<Bool, Never>() | ||
private let startLoading = TestObserver<Void, Never>() | ||
private let stopLoading = TestObserver<Void, Never>() | ||
|
||
override func setUp() { | ||
super.setUp() | ||
|
||
self.vm.outputs.isUserInteractionEnabled.observe(self.isUserInteractionEnabled.observer) | ||
self.vm.outputs.startLoading.observe(self.startLoading.observer) | ||
self.vm.outputs.stopLoading.observe(self.stopLoading.observer) | ||
} | ||
|
||
func testIsUserInteractionEnabled() { | ||
self.vm.inputs.isLoading(false) | ||
self.isUserInteractionEnabled.assertValues([true]) | ||
|
||
self.vm.inputs.isLoading(false) | ||
self.isUserInteractionEnabled.assertValues([true]) | ||
|
||
self.vm.inputs.isLoading(true) | ||
self.isUserInteractionEnabled.assertValues([true, false]) | ||
|
||
self.vm.inputs.isLoading(true) | ||
self.isUserInteractionEnabled.assertValues([true, false]) | ||
} | ||
|
||
func testStartLoading() { | ||
self.vm.inputs.isLoading(true) | ||
self.startLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(true) | ||
self.startLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(false) | ||
self.startLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(true) | ||
self.startLoading.assertValueCount(2) | ||
} | ||
|
||
func testStopLoading() { | ||
self.vm.inputs.isLoading(false) | ||
self.stopLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(false) | ||
self.stopLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(true) | ||
self.stopLoading.assertValueCount(1) | ||
|
||
self.vm.inputs.isLoading(false) | ||
self.stopLoading.assertValueCount(2) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style is something that could potentially be injected in the initializer in the future but for now I think this style will suffice