From 070d7c199ae6bbfe35dc0d69cf67e79e7d0495a6 Mon Sep 17 00:00:00 2001 From: Nikita Semenov Date: Mon, 3 Oct 2022 12:01:32 +0300 Subject: [PATCH] fix: code review notes --- .../Formatter/BaseRangeValuesFormatter.swift | 10 +-- .../ViewModels/BaseRangeFilterViewModel.swift | 56 +++++++------- .../RangeFilterViewModelProtocol.swift | 7 +- .../FilterRangeView/BaseFilterRangeView.swift | 77 +++++++++++-------- .../BaseIntervalInputView.swift | 25 +++--- .../SliderView/BaseFilterRangeSlider.swift | 16 ++-- 6 files changed, 103 insertions(+), 88 deletions(-) diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Models/Formatter/BaseRangeValuesFormatter.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Models/Formatter/BaseRangeValuesFormatter.swift index 121293ad..001a05d5 100644 --- 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 } } diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/BaseRangeFilterViewModel.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/BaseRangeFilterViewModel.swift index deaf5d82..ed3c90a4 100644 --- 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) + } } } diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/RangeFilterViewModelProtocol.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/ViewModels/RangeFilterViewModelProtocol.swift index d9965c90..81c925a4 100644 --- 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) } diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/FilterRangeView/BaseFilterRangeView.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/FilterRangeView/BaseFilterRangeView.swift index 9da1217d..0bf85b40 100644 --- 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: 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: 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: 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: BaseIni open override func configureAppearance() { super.configureAppearance() - defaultConfigure() updateAppearance() layoutSubviews() @@ -250,13 +263,13 @@ open class BaseFilterRangeView: 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: 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) } } diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/IntervalInputView/BaseIntervalInputView.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/IntervalInputView/BaseIntervalInputView.swift index 3dff48a2..2859b633 100644 --- 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) diff --git a/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/SliderView/BaseFilterRangeSlider.swift b/TIEcommerce/Sources/Filters/FiltersViews/RangeFilters/Views/SliderView/BaseFilterRangeSlider.swift index 8bb7e87e..c3d2b27a 100644 --- 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 -}