Babacros todayextension #17
Conversation
# Conflicts: # ISICbalance/ViewControllers/BalanceViewController.swift
| view.addSubview(balanceStack) | ||
|
|
||
| let balanceTitle = UILabel() | ||
| // we are not able to use swiftgen, because of retain cycle |
There was a problem hiding this comment.
❔ Jaký retain cycle máš na mysli?
There was a problem hiding this comment.
Kompilátor hlásí rectain cycle, pokud přidám file s vygenerovanými enumy k targetu. Doplním do komentáře.
There was a problem hiding this comment.
Ja spis neverim, ze se ti tohle deje 😊
| import Result | ||
|
|
||
| class TodayViewController: UIViewController, NCWidgetProviding { | ||
| private weak var isicImageView: UIButton? |
There was a problem hiding this comment.
❔ má nějaký důvod proč použít UIButton? místo UIButton!?
|
|
||
| // MARK: - Bindings | ||
|
|
||
| func setupBindings() { |
There was a problem hiding this comment.
❕ co může být private nechť je private
| } | ||
|
|
||
| let isicImage = UIImage(named: "todayExtension") | ||
| let isicImageView = UIButton(type: .custom) |
There was a problem hiding this comment.
❗️ pojmenovat něco isicImageView, když je to instance UIButton, je velice špatné
There was a problem hiding this comment.
Jo on to byl první imageView, ale předělal jsem na button a nechal původní jméno.
|
|
||
| // MARK: - Actions | ||
|
|
||
| @objc func isicImageTapped() { |
There was a problem hiding this comment.
konvence vypadá takto:
@objc
private func isicImageTapped(sender: UIButton) {
// Implementation
}
| } | ||
|
|
||
| protocol TodayViewModelingActions { | ||
| var getBalance: Action<(),Balance,RequestError> { get } |
There was a problem hiding this comment.
konvence: Action<Void, Balance, RequestError> { get }
|
|
||
| let balance: Property<Int> | ||
| let localeBalance: Property<String> | ||
| let getBalance: Action<(),Balance,RequestError> |
| final class TodayViewModel: BaseViewModel, TodayViewModeling, TodayViewModelingActions { | ||
| typealias Dependencies = HasRequestManager | ||
|
|
||
| let balance: Property<Int> |
There was a problem hiding this comment.
❔ používáš někde balance jako Int?
There was a problem hiding this comment.
Používám. Cpu ho numberFormatteru
There was a problem hiding this comment.
Zvenku ho cpes do numberFormatteru nebo jenom interne ve viewModelu?
Podfile
Outdated
|
|
||
| target 'ISICbalanceExtension' do | ||
| # UI | ||
| pod 'SnapKit', '~> 4.0' |
There was a problem hiding this comment.
vyupdatuj závislosti, máš tu spoustu zastaralých verzí
Podfile
Outdated
| pod 'ACKategories', '~> 6.0' | ||
| pod 'Alamofire', '~> 4.0' | ||
| pod 'SwiftSoup', '~> 1.7' | ||
| pod 'SwiftKeychainWrapper', '~> 3.2' |
There was a problem hiding this comment.
pokud máš více targetů, které sdílejí stejné závislosti, můžeš si v Podfilu vytvořit globální funkci a v ní jednotlivé závislosti vypsat
def shared_pods
pod 'SnapKit', '~> 5.0'
pod 'ReactiveSwift', '~> 5.0'
end
potom pro každý target můžeš zavolat
target 'ISICbalanceExtension' do
shared_pods
end
Podfile.lock
Outdated
| SwiftSoup: 92f5b348c131aed8af04f4326db1579ab19fa08c | ||
|
|
||
| PODFILE CHECKSUM: cab1eaf6010bbd2bcccbce071e41555f9c529f8c | ||
| PODFILE CHECKSUM: 5aeffec0e081013dde4c9372a1aea3b6a96466f8 | ||
|
|
||
| COCOAPODS: 1.7.1 |
There was a problem hiding this comment.
A pouzivas starou verzi Cocoapods
| @@ -19,7 +18,7 @@ protocol AccountViewModeling { | |||
| } | |||
|
|
|||
| protocol AccountViewModelingActions { | |||
| var login: Action<(),(),LoginError> { get } | |||
| var login: Action<Void,Void,LoginError> { get } | |||
There was a problem hiding this comment.
ještě mezery a bude to 🔝 nejlepší by bylo, kdybys nasadil Swiftlint
| import ReactiveSwift | ||
| import ACKReactiveExtensions | ||
|
|
||
| class TodayViewController: UIViewController, NCWidgetProviding { |
There was a problem hiding this comment.
💡 co může být final nechť je final
| self.balanceLabel = balanceLabel | ||
| balanceStack.addArrangedSubview(balanceLabel) | ||
|
|
||
| balanceStack.snp.makeConstraints { make in |
There was a problem hiding this comment.
přesuň layoutování tam, kde vytváříš balanceStack, takhle to není moc přehledné
| // MARK: - Bindings | ||
|
|
||
| private func setupBindings() { | ||
| balanceLabel?.reactive.text <~ viewModel.localeBalance |
There was a problem hiding this comment.
❔ proč tady může být otazník, když je to nahoře zadefinováno jako UILabel!? A jaký to má rozdíl proti tomu, když tam ten otazník nebude?
| } | ||
| } | ||
|
|
||
| extension Int { |
There was a problem hiding this comment.
❕ přesunul bych extensionu do vlastního filu
|
|
||
| COCOAPODS: 1.7.1 | ||
| COCOAPODS: 1.7.2 |
There was a problem hiding this comment.
zajímavý je, že je venku už 1.7.3 😄
No description provided.