code review notes

This commit is contained in:
Ivan Smolin 2018-03-22 14:55:43 +03:00
parent 7c012db927
commit b0cee2dee6
6 changed files with 49 additions and 51 deletions

View File

@ -398,9 +398,9 @@
67EB7FF32061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */; };
67EB7FF42061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */; };
67EB7FF8206175F700BDD9FB /* PaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FF7206175F700BDD9FB /* PaginationWrappable.swift */; };
67EB7FFD206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; };
67EB7FFE206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; };
67EB7FFF206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */; };
67EB7FFD206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; };
67EB7FFE206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; };
67EB7FFF206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */; };
67EB8001206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; };
67EB8002206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; };
67EB8003206177D600BDD9FB /* PaginationWrapperDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */; };
@ -623,7 +623,7 @@
67EB7FEA2061667900BDD9FB /* DefaultTotalCountCursorListingResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultTotalCountCursorListingResult.swift; sourceTree = "<group>"; };
67EB7FF02061682F00BDD9FB /* TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TotalCountCursorListingResult+DefaultTotalCountCursorListingResult.swift"; sourceTree = "<group>"; };
67EB7FF7206175F700BDD9FB /* PaginationWrappable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginationWrappable.swift; sourceTree = "<group>"; };
67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnyPaginationWrappableView.swift; sourceTree = "<group>"; };
67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnyPaginationWrappable.swift; sourceTree = "<group>"; };
67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginationWrapperDelegate.swift; sourceTree = "<group>"; };
67FDC25E1FA310EA00C76A77 /* RequestError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RequestError.swift; sourceTree = "<group>"; };
78405D3B3D3C3E17456877FF /* Pods_LeadKit_iOSTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_LeadKit_iOSTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
@ -1254,7 +1254,7 @@
677452BA2062812C0024EEEF /* PaginationDataLoading */ = {
isa = PBXGroup;
children = (
67EB7FFC206176C900BDD9FB /* AnyPaginationWrappableView.swift */,
67EB7FFC206176C900BDD9FB /* AnyPaginationWrappable.swift */,
67EB8000206177D600BDD9FB /* PaginationWrapperDelegate.swift */,
);
path = PaginationDataLoading;
@ -2289,7 +2289,7 @@
A676AE551F98112E001F9214 /* ObservableMappable.swift in Sources */,
A6E0DDE11F8A696F002CA74E /* SeparatorRowBox.swift in Sources */,
A6E0DDDE1F8A696F002CA74E /* EmptyCellRow.swift in Sources */,
67EB7FFD206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */,
67EB7FFD206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */,
671463041EB3396E00EAB194 /* UIView+LoadingIndicator.swift in Sources */,
6774527420624E820024EEEF /* DataLoadingModel.swift in Sources */,
40F118491F8FF223004AADAF /* TableRow+AppearanceExtension.swift in Sources */,
@ -2492,7 +2492,7 @@
67EB7FD220615B8900BDD9FB /* TotalCountCursorConfiguration.swift in Sources */,
EFBE57D31EC35EF20040E00A /* Array+Extensions.swift in Sources */,
6714638B1EB3396E00EAB194 /* RoundDrawingOperation.swift in Sources */,
67EB7FFF206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */,
67EB7FFF206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */,
A676AE4A1F97D28A001F9214 /* String+Extensions.swift in Sources */,
671463831EB3396E00EAB194 /* PaddingDrawingOperation.swift in Sources */,
6774527C206252020024EEEF /* DataLoadingState.swift in Sources */,
@ -2589,7 +2589,7 @@
671463491EB3396E00EAB194 /* ResettableType.swift in Sources */,
A676AE491F97D28A001F9214 /* String+Extensions.swift in Sources */,
671462E51EB3396E00EAB194 /* UIColor+Hex.swift in Sources */,
67EB7FFE206176C900BDD9FB /* AnyPaginationWrappableView.swift in Sources */,
67EB7FFE206176C900BDD9FB /* AnyPaginationWrappable.swift in Sources */,
671462CD1EB3396E00EAB194 /* String+SizeCalculation.swift in Sources */,
677452B720627FE00024EEEF /* PaginationWrappable.swift in Sources */,
67EB7FC1206140E600BDD9FB /* TotalCountCursor.swift in Sources */,

View File

@ -29,11 +29,11 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
// "Segmentation fault: 11" in Xcode 9.2 without redundant same-type constraint :(
where Cursor == Delegate.DataSourceType, Cursor.ResultType == [Cursor.Element] {
fileprivate typealias DataLoadingModel = PaginationDataLoadingModel<Cursor>
private typealias DataLoadingModel = PaginationDataLoadingModel<Cursor>
fileprivate typealias LoadingState = DataLoadingModel.LoadingStateType
private typealias LoadingState = DataLoadingModel.LoadingStateType
private var wrappedView: AnyPaginationWrappableView
private var wrappedView: AnyPaginationWrappable
private let paginationViewModel: DataLoadingModel
private weak var delegate: Delegate?
@ -71,7 +71,7 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
/// - wrappedView: UIScrollView instance to work with.
/// - cursor: Cursor object that acts as data source.
/// - delegate: Delegate object for data loading events handling and UI customization.
public init(wrappedView: AnyPaginationWrappableView, cursor: Cursor, delegate: Delegate) {
public init(wrappedView: AnyPaginationWrappable, cursor: Cursor, delegate: Delegate) {
self.wrappedView = wrappedView
self.delegate = delegate
@ -137,7 +137,7 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
private func onLoadingMoreState(afterState: LoadingState) {
if case .error = afterState { // user tap retry button in table footer
delegate?.retryLoadMoreButtonWillBeHidden()
delegate?.footerRetryButtonWillDisappear()
wrappedView.footerView = nil
addInfiniteScroll(withHandler: false)
wrappedView.scrollView.beginInfiniteScroll(true)
@ -183,8 +183,8 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
} else if case .loadingMore = afterState {
removeInfiniteScroll()
guard let retryButton = delegate?.retryLoadMoreButton(),
let retryButtonHeigth = delegate?.retryLoadMoreButtonHeight() else {
guard let retryButton = delegate?.footerRetryButton(),
let retryButtonHeigth = delegate?.footerRetryButtonHeight() else {
return
}
@ -196,7 +196,7 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
.drive(reloadEvent)
.disposed(by: disposeBag)
delegate?.retryLoadMoreButtonWillBeShown()
delegate?.footerRetryButtonWillAppear()
wrappedView.footerView = retryButton
}
@ -224,13 +224,13 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
placeholderView.isHidden = false
// I was unable to add pull-to-refresh placeholder scroll behaviour without this trick
let wrapperView = UIView()
wrapperView.addSubview(placeholderView)
let placeholderWrapperView = UIView()
placeholderWrapperView.addSubview(placeholderView)
let leadingConstraint = placeholderView.leadingAnchor.constraint(equalTo: wrapperView.leadingAnchor)
let trailingConstraint = placeholderView.trailingAnchor.constraint(equalTo: wrapperView.trailingAnchor)
let topConstraint = placeholderView.topAnchor.constraint(equalTo: wrapperView.topAnchor)
let bottomConstraint = placeholderView.bottomAnchor.constraint(equalTo: wrapperView.bottomAnchor)
let leadingConstraint = placeholderView.leadingAnchor.constraint(equalTo: placeholderWrapperView.leadingAnchor)
let trailingConstraint = placeholderView.trailingAnchor.constraint(equalTo: placeholderWrapperView.trailingAnchor)
let topConstraint = placeholderView.topAnchor.constraint(equalTo: placeholderWrapperView.topAnchor)
let bottomConstraint = placeholderView.bottomAnchor.constraint(equalTo: placeholderWrapperView.bottomAnchor)
NSLayoutConstraint.activate([leadingConstraint,
trailingConstraint,
@ -239,7 +239,7 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
currentPlaceholderViewTopConstraint = topConstraint
wrappedView.backgroundView = wrapperView
wrappedView.backgroundView = placeholderWrapperView
currentPlaceholderView = placeholderView
}
@ -329,7 +329,7 @@ final public class PaginationWrapper<Cursor: ResettableRxDataSourceCursor, Deleg
private extension PaginationWrapper {
var stateChanged: Binder<LoadingState> {
private var stateChanged: Binder<LoadingState> {
return Binder(self) { base, value in
switch value {
case .initial:

View File

@ -23,12 +23,10 @@
import RxSwift
import RxCocoa
open class RxDataLoadingModel<DLS: DataLoadingState>: DataLoadingModel
where DLS.DataSourceType: RxDataSource {
open class RxDataLoadingModel<LoadingStateType: DataLoadingState>: DataLoadingModel
where LoadingStateType.DataSourceType: RxDataSource {
public typealias LoadingStateType = DLS
public typealias DataSourceType = DLS.DataSourceType
public typealias DataSourceType = LoadingStateType.DataSourceType
public typealias ResultType = DataSourceType.ResultType
public typealias EmptyResultChecker = (ResultType) -> Bool
@ -69,8 +67,7 @@ open class RxDataLoadingModel<DLS: DataLoadingState>: DataLoadingModel
}
func onGot(error: Error) {
state = .errorState(error: error,
after: state)
state = .errorState(error: error, after: state)
}
private func onGot(result: ResultType, from dataSource: DataSourceType) {

View File

@ -75,7 +75,7 @@ public extension PaginationWrapperDelegate {
return AnyLoadingIndicator(indicator)
}
func retryLoadMoreButton() -> UIButton {
func footerRetryButton() -> UIButton {
let retryButton = UIButton(type: .custom)
retryButton.backgroundColor = .lightGray
retryButton.setTitle("Retry load more", for: .normal)
@ -83,15 +83,15 @@ public extension PaginationWrapperDelegate {
return retryButton
}
func retryLoadMoreButtonHeight() -> CGFloat {
func footerRetryButtonHeight() -> CGFloat {
return 44
}
func retryLoadMoreButtonWillBeShown() {
func footerRetryButtonWillAppear() {
// by default - nothing will happen
}
func retryLoadMoreButtonWillBeHidden() {
func footerRetryButtonWillDisappear() {
// by default - nothing will happen
}

View File

@ -23,7 +23,7 @@
import UIKit
/// Type that performs some kind of type erasure for PaginationWrappable.
public struct AnyPaginationWrappableView: PaginationWrappable {
public final class AnyPaginationWrappable: PaginationWrappable {
private typealias ViewSetter = (UIView?) -> Void

View File

@ -26,7 +26,7 @@ public protocol PaginationWrapperDelegate: class {
associatedtype DataSourceType: DataSource
/// Delegate method that handles loading new chunk of data.
/// Handles loading new chunk of data.
///
/// - Parameters:
/// - newItems: New items.
@ -34,7 +34,7 @@ public protocol PaginationWrapperDelegate: class {
func paginationWrapper(didLoad newItems: DataSourceType.ResultType,
using dataSource: DataSourceType)
/// Delegate method that handles reloading or initial loading of data.
/// Handles reloading or initial loading of data.
///
/// - Parameters:
/// - allItems: New items.
@ -42,12 +42,12 @@ public protocol PaginationWrapperDelegate: class {
func paginationWrapper(didReload allItems: DataSourceType.ResultType,
using dataSource: DataSourceType)
/// Delegate method that returns placeholder view for empty state.
/// Returns placeholder view for empty state.
///
/// - Returns: Configured instace of UIView.
func emptyPlaceholder() -> UIView
/// Delegate method that is called when initial loading error is occured.
/// Called when initial loading error is occured.
/// It should return true if error is handled and false if it doesn't.
///
/// - Parameters:
@ -55,42 +55,43 @@ public protocol PaginationWrapperDelegate: class {
/// - Returns: Bool value. If true - then error placeholder wouldn't be shown.
func customInitialLoadingErrorHandling(for error: Error) -> Bool
/// Delegate method that returns placeholder view for error state.
/// Returns placeholder view for error state.
///
/// - Parameters:
/// - error: Error that occured due data loading.
/// - Returns: Configured instace of UIView.
func errorPlaceholder(for error: Error) -> UIView
/// Delegate method that returns loading idicator for initial loading state.
/// Returns loading idicator for initial loading state.
/// This indicator will appear at center of the placeholders container.
///
/// - Returns: Configured instace of AnyLoadingIndicator.
func initialLoadingIndicator() -> AnyLoadingIndicator
/// Delegate method that returns loading idicator for initial loading state.
/// Returns loading idicator for initial loading state.
///
/// - Returns: Configured instace of AnyLoadingIndicator.
func loadingMoreIndicator() -> AnyLoadingIndicator
/// Delegate method that returns instance of UIButton for "retry load more" action.
/// Returns instance of UIButton for "retry load more" action.
///
/// - Returns: Configured instace of AnyLoadingIndicator.
func retryLoadMoreButton() -> UIButton
func footerRetryButton() -> UIButton
/// Delegate method that returns preferred height for "retry load more" button.
/// Returns height for "retry load more" button.
///
/// - Returns: Preferred height of "retry load more" button.
func retryLoadMoreButtonHeight() -> CGFloat
/// - Returns: Height of "retry load more" button.
func footerRetryButtonHeight() -> CGFloat
/// Method is called before "retry load more" will be shown.
/// Typically, it's used when you need to show custom footer view.
func retryLoadMoreButtonWillBeShown()
func footerRetryButtonWillAppear()
/// Method is called before "retry load more" will be hidden.
/// Typically, it's used when you need to hide custom footer view.
func retryLoadMoreButtonWillBeHidden()
func footerRetryButtonWillDisappear()
/// Delegate method, used to clear view if placeholder is shown.
/// Clears view when placeholder is shown.
func clearView()
}