fix: code review notes

2022-10-03 12:01:32 +03:00 · 2022-10-03 12:01:32 +03:00 · 070d7c199a
parent a8785b35e8
commit 070d7c199a
6 changed files with 103 additions and 88 deletions
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Models/Formatter/BaseRangeValuesFormatter.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Models/Formatter/BaseRangeValuesFormatter.swift
@ -26,9 +26,9 @@ open class BaseRangeValuesFormatter: RangeValuesFormatterProtocol {

    public let formatter: NumberFormatter

-    open var floatValueDelimiter: String {
-        "."
-    }
+    public var floatValueDelimiter = "."
+
+    public var formattingFailureValue = 0.0

    public init() {
        formatter = NumberFormatter()
@ -49,10 +49,10 @@ open class BaseRangeValuesFormatter: RangeValuesFormatterProtocol {
    open func string(fromDouble value: Double) -> String {
        let nsNumber = NSNumber(floatLiteral: value)

-        return formatter.string(from: nsNumber) ?? ""
+        return formatter.string(from: nsNumber) ?? "\(formattingFailureValue)"
    }

    open func double(fromString value: String) -> Double {
-        formatter.number(from: value)?.doubleValue ?? 0
+        formatter.number(from: value)?.doubleValue ?? formattingFailureValue
    }
 }
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/BaseRangeFilterViewModel.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/BaseRangeFilterViewModel.swift
@ -24,16 +24,16 @@ import CoreGraphics
 import Foundation
 import TISwiftUtils

-public typealias FilterRangeValue = (fromValue: Double, toValue: Double)
+public typealias FilterRangeValue = (fromValue: CGFloat, toValue: CGFloat)

 open class BaseRangeFilterViewModel: RangeFilterViewModelProtocol {

-    public let fromValue: Double
-    public let toValue: Double
-    public let stepValues: [Double]
+    public let fromValue: CGFloat
+    public let toValue: CGFloat
+    public let stepValues: [CGFloat]

-    public var initialFromValue: Double?
-    public var initialToValue: Double?
+    public var initialFromValue: CGFloat?
+    public var initialToValue: CGFloat?

    public weak var filterRangeView: FilterRangeViewRepresenter?
    public weak var pickerDelegate: RangeFiltersPickerDelegate?
@ -46,11 +46,11 @@ open class BaseRangeFilterViewModel: RangeFilterViewModelProtocol {
        initialFromValue != fromValue || initialToValue != toValue
    }

-    public init(fromValue: Double,
-                toValue: Double,
-                stepValues: [Double],
-                initialFromValue: Double? = nil,
-                initialToValue: Double? = nil) {
+    public init(fromValue: CGFloat,
+                toValue: CGFloat,
+                stepValues: [CGFloat],
+                initialFromValue: CGFloat? = nil,
+                initialToValue: CGFloat? = nil) {

        self.fromValue = fromValue
        self.toValue = toValue
@ -69,23 +69,27 @@ open class BaseRangeFilterViewModel: RangeFilterViewModelProtocol {
        pickerDelegate?.valueDidEndChanging(values)
    }

-    open func fromValueIsChanging(_ values: FilterRangeValue) {
-        filterRangeView?.configureRangeView(with: values)
-        pickerDelegate?.valueDidEndChanging(values)
+    open func intervalInputValueIsChanging(_ values: FilterRangeValue, side: RangeBoundSide) {
+        switch side {
+        case lower:
+            filterRangeView?.configureRangeView(with: values)
+            pickerDelegate?.valueDidEndChanging(values)
+
+        case upper:
+            filterRangeView?.configureRangeView(with: values)
+            pickerDelegate?.valueDidEndChanging(values)
+        }
    }

-    open func toValueIsChanging(_ values: FilterRangeValue) {
-        filterRangeView?.configureRangeView(with: values)
-        pickerDelegate?.valueDidEndChanging(values)
-    }
+    open func intervalInputValueDidEndChanging(_ values: FilterRangeValue, side: RangeBoundSide) {
+        switch side {
+        case lower:
+            filterRangeView?.configureRangeView(with: values)
+            pickerDelegate?.valueDidEndChanging(values)

-    open func fromValueDidEndChanging(_ values: FilterRangeValue) {
-        filterRangeView?.configureRangeView(with: values)
-        pickerDelegate?.valueDidEndChanging(values)
-    }
-
-    open func toValueDidEndChanging(_ values: FilterRangeValue) {
-        filterRangeView?.configureRangeView(with: values)
-        pickerDelegate?.valueDidEndChanging(values)
+        case upper:
+            filterRangeView?.configureRangeView(with: values)
+            pickerDelegate?.valueDidEndChanging(values)
+        }
    }
 }
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/RangeFilterViewModelProtocol.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/RangeFilterViewModelProtocol.swift
@ -31,9 +31,6 @@ public protocol RangeFilterViewModelProtocol {
    func rangeSliderValueIsChanging(_ values: FilterRangeValue)
    func rangeSliderValueDidEndChanging(_ values: FilterRangeValue)

-    func toValueIsChanging(_ values: FilterRangeValue)
-    func toValueDidEndChanging(_ values: FilterRangeValue)
-
-    func fromValueDidEndChanging(_ values: FilterRangeValue)
-    func fromValueIsChanging(_ values: FilterRangeValue)
+    func intervalInputValueIsChanging(_ values: FilterRangeValue, side: RangeBoundSide)
+    func intervalInputValueDidEndChanging(_ values: FilterRangeValue, side: RangeBoundSide)
 }
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/FilterRangeView/BaseFilterRangeView.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/FilterRangeView/BaseFilterRangeView.swift
@ -41,20 +41,22 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
    public let rangeSlider: BaseFilterRangeSlider

    public let textFieldsContainer = UIView()
-    public let fromValueView = BaseIntervalInputView(state: .fromValue)
-    public let toValueView = BaseIntervalInputView(state: .toValue)
+    public let fromValueView = BaseIntervalInputView(state: .lower)
+    public let toValueView = BaseIntervalInputView(state: .upper)

    public var viewModel: ViewModel?

    // MARK: - Open properties

    open var rangeSliderValue: FilterRangeValue {
-        (rangeSlider.leftValue, rangeSlider.rightValue)
+        (fromValue: rangeSlider.leftValue, toValue: rangeSlider.rightValue)
    }

    open var textFieldsValues: FilterRangeValue {
-        (min(fromValueView.currentValue, viewModel?.toValue ?? .zero),
-         max(toValueView.currentValue, viewModel?.fromValue ?? .zero))
+        (
+            fromValue: min(fromValueView.currentValue, viewModel?.toValue ?? .zero),
+            toValue: max(toValueView.currentValue, viewModel?.fromValue ?? .zero)
+         )
    }
    // MARK: - Text Fields Setup

@ -183,26 +185,36 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
        let toTextFieldWidthConstraint = toValueView.widthAnchor.constraint(greaterThanOrEqualToConstant: textFieldsMinWidth)
        let fromTextFieldWidthConstraint = fromValueView.widthAnchor.constraint(greaterThanOrEqualToConstant: textFieldsMinWidth)

+        let fromTextFieldConstraints = [
+            fromTextFieldWidthConstraint,
+            fromValueView.leadingAnchor.constraint(equalTo: textFieldsContainer.leadingAnchor),
+            fromValueView.topAnchor.constraint(equalTo: textFieldsContainer.topAnchor),
+            fromValueView.bottomAnchor.constraint(equalTo: textFieldsContainer.bottomAnchor),
+        ]
+
+        let toTextFieldConstraints = [
+            toTextFieldWidthConstraint,
+            toValueView.trailingAnchor.constraint(equalTo: textFieldsContainer.trailingAnchor),
+            toValueView.topAnchor.constraint(equalTo: textFieldsContainer.topAnchor),
+            toValueView.bottomAnchor.constraint(equalTo: textFieldsContainer.bottomAnchor),
+        ]
+
+        let sliderConstraints = [
+            rangeSlider.leadingAnchor.constraint(equalTo: textFieldsContainer.leadingAnchor),
+            rangeSlider.trailingAnchor.constraint(equalTo: textFieldsContainer.trailingAnchor),
+            layout == .textFieldsOnTop
+                ? rangeSlider.bottomAnchor.constraint(equalTo: bottomAnchor)
+                : rangeSlider.topAnchor.constraint(equalTo: topAnchor)
+        ]
+
        NSLayoutConstraint.activate(
            contentEdgesConstraints.allConstraints
            +
-            [
-                toTextFieldWidthConstraint,
-                fromTextFieldWidthConstraint,
-
-                toValueView.trailingAnchor.constraint(equalTo: textFieldsContainer.trailingAnchor),
-                toValueView.topAnchor.constraint(equalTo: textFieldsContainer.topAnchor),
-                toValueView.bottomAnchor.constraint(equalTo: textFieldsContainer.bottomAnchor),
-                fromValueView.leadingAnchor.constraint(equalTo: textFieldsContainer.leadingAnchor),
-                fromValueView.topAnchor.constraint(equalTo: textFieldsContainer.topAnchor),
-                fromValueView.bottomAnchor.constraint(equalTo: textFieldsContainer.bottomAnchor),
-
-                rangeSlider.leadingAnchor.constraint(equalTo: textFieldsContainer.leadingAnchor),
-                rangeSlider.trailingAnchor.constraint(equalTo: textFieldsContainer.trailingAnchor),
-                layout == .textFieldsOnTop
-                    ? rangeSlider.bottomAnchor.constraint(equalTo: bottomAnchor)
-                    : rangeSlider.topAnchor.constraint(equalTo: topAnchor)
-            ]
+            fromTextFieldConstraints
+            +
+            toTextFieldConstraints
+            +
+            sliderConstraints
        )

        self.toTextFieldWidthConstraint = toTextFieldWidthConstraint
@ -212,6 +224,8 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
    open override func bindViews() {
        super.bindViews()

+        defaultConfigure()
+
        fromValueView.configure(with: formatter)
        toValueView.configure(with: formatter)
        rangeSlider.configure(with: formatter)
@ -234,7 +248,6 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
    open override func configureAppearance() {
        super.configureAppearance()

-        defaultConfigure()
        updateAppearance()

        layoutSubviews()
@ -250,13 +263,13 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
        fromValueView.configure(with: viewModel.initialFromValue ?? viewModel.fromValue)
        toValueView.configure(with: viewModel.initialToValue ?? viewModel.toValue)

-        rangeSlider.minimumValue = CGFloat(viewModel.fromValue)
-        rangeSlider.maximumValue = CGFloat(viewModel.toValue)
+        rangeSlider.minimumValue = viewModel.fromValue
+        rangeSlider.maximumValue = viewModel.toValue

-        rangeSlider.leftValue = CGFloat(viewModel.initialFromValue ?? viewModel.fromValue)
-        rangeSlider.rightValue = CGFloat(viewModel.initialToValue ?? viewModel.toValue)
+        rangeSlider.leftValue = viewModel.initialFromValue ?? viewModel.fromValue
+        rangeSlider.rightValue = viewModel.initialToValue ?? viewModel.toValue

-        rangeSlider.addStep(stepValues: viewModel.stepValues.map { CGFloat($0) })
+        rangeSlider.addStep(stepValues: viewModel.stepValues)
    }

    // MARK: - FilterRangeViewRepresenter
@ -318,18 +331,18 @@ open class BaseFilterRangeView<ViewModel: RangeFilterViewModelProtocol>: BaseIni
    }

    @objc private func fromValueIsChanging() {
-        viewModel?.fromValueIsChanging(textFieldsValues)
+        viewModel?.intervalInputValueIsChanging(textFieldsValues, side: .lower)
    }

    @objc private func toValueIsChanging() {
-        viewModel?.toValueIsChanging(textFieldsValues)
+        viewModel?.intervalInputValueIsChanging(textFieldsValues, side: .upper)
    }

    @objc private func fromValueDidEndChanging() {
-        viewModel?.fromValueDidEndChanging(textFieldsValues)
+        viewModel?.intervalInputValueDidEndChanging(textFieldsValues, side: .lower)
    }

    @objc private func toValueDidEndChanging() {
-        viewModel?.toValueDidEndChanging(textFieldsValues)
+        viewModel?.intervalInputValueDidEndChanging(textFieldsValues, side: .upper)
    }
 }
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/IntervalInputView/BaseIntervalInputView.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/IntervalInputView/BaseIntervalInputView.swift
@ -24,17 +24,17 @@ import TIUIElements
 import TIUIKitCore
 import UIKit

-open class BaseIntervalInputView: BaseInitializableView, UITextFieldDelegate {
+public enum RangeBoundSide {
+    case lower
+    case upper
+}

-    public enum TextFieldState {
-        case fromValue
-        case toValue
-    }
+open class BaseIntervalInputView: BaseInitializableView, UITextFieldDelegate {

    private var contentEdgesConstraints: EdgeConstraints?
    private var labelsSpacingConstraint: NSLayoutConstraint?

-    public let state: TextFieldState
+    public let state: RangeBoundSide

    public let intervalLabel = UILabel()
    public let inputTextField = UITextField()
@ -45,11 +45,11 @@ open class BaseIntervalInputView: BaseInitializableView, UITextFieldDelegate {
        }
    }

-    open var validCharacterSet: CharacterSet {
+    open lazy var validCharacterSet: CharacterSet = {
        CharacterSet
            .decimalDigits
            .union(CharacterSet(charactersIn: formatter?.floatValueDelimiter ?? "."))
-    }
+    }()

    open var fontColor: UIColor = .black {
        didSet {
@ -65,7 +65,12 @@ open class BaseIntervalInputView: BaseInitializableView, UITextFieldDelegate {
    }

    open var currentValue: Double {
-        formatter?.double(fromString: inputTextField.text ?? "") ?? 0
+        guard let formatter = formatter, 
+              let inputText = inputTextField.text else {
+            return 0 
+        }
+
+        return formatter.double(fromString: inputTextField.text)
    }

    open override var intrinsicContentSize: CGSize {
@ -78,7 +83,7 @@ open class BaseIntervalInputView: BaseInitializableView, UITextFieldDelegate {

    // MARK: - Init

-    public init(state: TextFieldState) {
+    public init(state: RangeBoundSide) {
        self.state = state

        super.init(frame: .zero)
--- a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/SliderView/BaseFilterRangeSlider.swift
+++ b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/SliderView/BaseFilterRangeSlider.swift
@ -184,14 +184,16 @@ open class BaseFilterRangeSlider: StepRangeSlider {
    }

    private func createStepLabels() {
+        guard let formatter = formatter else { return }
+
        stepLabels.forEach {
            $0.removeFromSuperview()
        }

        stepValues.forEach {
            let label = UILabel()
-            let formattedText = formatter?.string(fromDouble: $0)
-            label.text = formattedText ?? ""
+            let formattedText = formatter.string(fromDouble: $0)
+            label.text = formattedText

            stepLabels.append(label)
        }
@ -207,8 +209,8 @@ open class BaseFilterRangeSlider: StepRangeSlider {

            let circleView = stepCircleViews[index]
            let yPosition = layout == .textFieldsOnTop
-                ? circleView.frame.maxY + .stepLabelsTopInset
-                : circleView.frame.minY - label.bounds.height - .stepLabelsTopInset
+                ? circleView.frame.maxY
+                : circleView.frame.minY - label.bounds.height

            label.center.x = circleView.center.x
            label.sizeToFit()
@ -224,9 +226,3 @@ open class BaseFilterRangeSlider: StepRangeSlider {
        return (value - minimumValue) * unit
    }
 }
-
-// MARK: - Constant
-
-private extension CGFloat {
-    static let stepLabelsTopInset: CGFloat = 11
-}