fix: code review notes
This commit is contained in:
Nikita Semenov
parent
0abda665bf
commit
9eaf42aa4e
|
|
@ -22,42 +22,27 @@
|
|||
|
||||
import UIKit
|
||||
|
||||
open class BaseFilterCellAppearance {
|
||||
public struct FilterCellStateAppearance {
|
||||
|
||||
public var selectedBorderColor: UIColor
|
||||
public var normalBorderColor: UIColor
|
||||
public let borderColor: UIColor
|
||||
public let backgroundColor: UIColor
|
||||
public let fontColor: UIColor
|
||||
|
||||
public var selectedBgColor: UIColor
|
||||
public var normalBgColor: UIColor
|
||||
public let borderWidth: CGFloat
|
||||
public let contentInsets: UIEdgeInsets
|
||||
public let cornerRadius: CGFloat
|
||||
|
||||
public var selectedFontColor: UIColor
|
||||
public var normalFontColor: UIColor
|
||||
|
||||
public var selectedBorderWidth: CGFloat
|
||||
public var normalBorderWidth: CGFloat
|
||||
|
||||
public var contentInsets: UIEdgeInsets
|
||||
public var cornerRadius: CGFloat
|
||||
|
||||
public init(selectedBorderColor: UIColor = .systemGreen,
|
||||
normalBorderColor: UIColor = .lightGray,
|
||||
selectedBgColor: UIColor = .white,
|
||||
normalBgColor: UIColor = .lightGray,
|
||||
selectedFontColor: UIColor = .systemGreen,
|
||||
normalFontColor: UIColor = .black,
|
||||
selectedBorderWidth: CGFloat = 1,
|
||||
normalBorderWidth: CGFloat = 0,
|
||||
public init(borderColor: UIColor,
|
||||
backgroundColor: UIColor,
|
||||
fontColor: UIColor,
|
||||
borderWidth: CGFloat,
|
||||
contentInsets: UIEdgeInsets = .init(top: 4, left: 8, bottom: 4, right: 8),
|
||||
cornerRadius: CGFloat = 6) {
|
||||
|
||||
self.selectedBorderColor = selectedBorderColor
|
||||
self.normalBorderColor = normalBorderColor
|
||||
self.selectedBgColor = selectedBgColor
|
||||
self.normalBgColor = normalBgColor
|
||||
self.selectedFontColor = selectedFontColor
|
||||
self.normalFontColor = normalFontColor
|
||||
self.selectedBorderWidth = selectedBorderWidth
|
||||
self.normalBorderWidth = normalBorderWidth
|
||||
self.borderColor = borderColor
|
||||
self.backgroundColor = backgroundColor
|
||||
self.fontColor = fontColor
|
||||
self.borderWidth = borderWidth
|
||||
self.contentInsets = contentInsets
|
||||
self.cornerRadius = cornerRadius
|
||||
}
|
||||
|
|
@ -21,7 +21,7 @@
|
|||
//
|
||||
|
||||
public protocol FilterCellViewModelProtocol {
|
||||
var id: String { get set }
|
||||
var title: String { get set }
|
||||
var id: String { get }
|
||||
var title: String { get }
|
||||
var isSelected: Bool { get set }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,8 +22,8 @@
|
|||
|
||||
public struct DefaultFilterCellViewModel: FilterCellViewModelProtocol, Hashable {
|
||||
|
||||
public var id: String
|
||||
public var title: String
|
||||
public let id: String
|
||||
public let title: String
|
||||
public var isSelected: Bool
|
||||
|
||||
public init(id: String,
|
||||
|
|
|
|||
|
|
@ -24,35 +24,41 @@ import TIUIKitCore
|
|||
import TIUIElements
|
||||
import UIKit
|
||||
|
||||
open class DefaultFilterCollectionCell: ContainerCollectionViewCell<UILabel>,
|
||||
ConfigurableView {
|
||||
/*
|
||||
|
||||
public enum SelectionState {
|
||||
case normal
|
||||
case selected
|
||||
У тебя в кейсах по сути одно и то-же написано. Я таки рекомендую у cellAppearance хранить 2 экземпляра структуры с этими свойствами в normalAppearance и в selectedAppearance, а здесь сделать один метод в который ты будешь эту структуру передавать и применять оттуда цвета и ширину линии.
|
||||
|
||||
|
||||
|
||||
|
||||
Предлагаю визуально разделить переменные, либо завести структурку FilterCellState и дальше либо Appearance или Configuration или какой-нибудь микс из них в названии. Держать там 3 свойства color, backgroundColor и fontColor. А здесь держать 2 экземпляра этой струкруты для normal и selected состояний. Ну и кажется что нет необходимости делать BaseFilterCellAppearance классом, вместо стукруры
|
||||
|
||||
У тебя в инициализаторе 10 аргументов, а могло быть 4. + это упроситло бы применение апиранса. Ты просто пилишь один метод применения, и туда передаешь либо свойство normalAppearance, либо selectedAppearance и все. А внутри у тебя одинаковая реализация. + можно было бы определить набор стандартных апирансов для разных стейтов и при инициализации использовать их через .defaultSelectedAppearance, .defaultNormalAppearance и комбинировать их в нужном порядке. А сейчас нужно будет заполнять 10 аргументов инициализатора.
|
||||
|
||||
Ну и остался открытым вопрос, а почему собственно BaseFilterCellAppearance класс, а не структура? И есть ли смысл делать свойства переменными, а не константами?
|
||||
*/
|
||||
|
||||
open class DefaultFilterCollectionCell: ContainerCollectionViewCell<UILabel>, ConfigurableView {
|
||||
|
||||
open var selectedStateAppearance: FilterCellStateAppearance {
|
||||
.defaultSelectedAppearance
|
||||
}
|
||||
|
||||
public var currentSelectionState: SelectionState {
|
||||
isSelected ? .selected : .normal
|
||||
}
|
||||
|
||||
open var cellAppearance: BaseFilterCellAppearance {
|
||||
.init()
|
||||
open var normalStateAppearance: FilterCellStateAppearance {
|
||||
.defaultNormalAppearance
|
||||
}
|
||||
|
||||
open override var isSelected: Bool {
|
||||
didSet {
|
||||
setAppearance()
|
||||
let appearance = isSelected ? selectedStateAppearance : normalStateAppearance
|
||||
updateAppearance(with: appearance)
|
||||
}
|
||||
}
|
||||
|
||||
open override func configureAppearance() {
|
||||
super.configureAppearance()
|
||||
|
||||
layer.round(corners: .allCorners, radius: cellAppearance.cornerRadius)
|
||||
contentInsets = cellAppearance.contentInsets
|
||||
|
||||
setAppearance()
|
||||
updateAppearance(with: normalStateAppearance)
|
||||
}
|
||||
|
||||
// MARK: - ConfigurableView
|
||||
|
|
@ -63,19 +69,24 @@ open class DefaultFilterCollectionCell: ContainerCollectionViewCell<UILabel>,
|
|||
|
||||
// MARK: - Open methdos
|
||||
|
||||
open func setAppearance() {
|
||||
switch currentSelectionState {
|
||||
case .normal:
|
||||
wrappedView.textColor = cellAppearance.normalFontColor
|
||||
backgroundColor = cellAppearance.normalBgColor
|
||||
layer.borderColor = cellAppearance.normalBorderColor.cgColor
|
||||
layer.borderWidth = cellAppearance.normalBorderWidth
|
||||
open func updateAppearance(with appearance: FilterCellStateAppearance) {
|
||||
contentInsets = appearance.contentInsets
|
||||
wrappedView.textColor = appearance.fontColor
|
||||
|
||||
case .selected:
|
||||
wrappedView.textColor = cellAppearance.selectedFontColor
|
||||
backgroundColor = cellAppearance.selectedBgColor
|
||||
layer.borderColor = cellAppearance.selectedBorderColor.cgColor
|
||||
layer.borderWidth = cellAppearance.selectedBorderWidth
|
||||
}
|
||||
backgroundColor = appearance.backgroundColor
|
||||
layer.borderColor = appearance.borderColor.cgColor
|
||||
layer.borderWidth = appearance.borderWidth
|
||||
layer.round(corners: .allCorners, radius: appearance.cornerRadius)
|
||||
}
|
||||
}
|
||||
|
||||
extension FilterCellStateAppearance {
|
||||
static var defaultSelectedAppearance: FilterCellStateAppearance {
|
||||
.init(borderColor: .systemGreen, backgroundColor: .white, fontColor: .systemGreen, borderWidth: 1)
|
||||
}
|
||||
|
||||
static var defaultNormalAppearance: FilterCellStateAppearance {
|
||||
|
||||
.init(borderColor: .lightGray, backgroundColor: .lightGray, fontColor: .black, borderWidth: 0)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,82 +21,73 @@
|
|||
//
|
||||
|
||||
import Foundation
|
||||
import TISwiftUtils
|
||||
|
||||
public protocol FilterViewModelProtocol: AnyObject {
|
||||
|
||||
associatedtype Property: FilterPropertyValueRepresenter & Hashable
|
||||
associatedtype PropertyValue: FilterPropertyValueRepresenter & Hashable
|
||||
associatedtype CellViewModelType: FilterCellViewModelProtocol & Hashable
|
||||
|
||||
typealias Change = (indexPath: IndexPath, viewModel: CellViewModelType)
|
||||
var values: [PropertyValue] { get set }
|
||||
var selectedValues: [PropertyValue] { get set }
|
||||
|
||||
var properties: [Property] { get set }
|
||||
var selectedProperties: [Property] { get set }
|
||||
|
||||
func filterDidSelected(atIndexPath indexPath: IndexPath) -> [Change]
|
||||
func toggleProperty(atIndexPath indexPath: IndexPath) -> (selected: [Property], deselected: [Property])
|
||||
func filterDidSelected(atIndexPath indexPath: IndexPath) -> [(indexPath: IndexPath, viewModel: CellViewModelType)]
|
||||
func toggleProperty(atIndexPath indexPath: IndexPath) -> (selected: [PropertyValue], deselected: [PropertyValue])
|
||||
}
|
||||
|
||||
public extension FilterViewModelProtocol {
|
||||
|
||||
func toggleProperty(atIndexPath indexPath: IndexPath) -> (selected: [Property], deselected: [Property]) {
|
||||
guard let item = getPropertySafely(indexPath.item) else { return ([], []) }
|
||||
func toggleProperty(atIndexPath indexPath: IndexPath) -> (selected: [PropertyValue], deselected: [PropertyValue]) {
|
||||
guard let item = values[safe: indexPath.item] else { return ([], []) }
|
||||
|
||||
return toggleProperty(item)
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
private func toggleProperty(_ property: Property) -> (selected: [Property], deselected: [Property]) {
|
||||
var propertiesToDeselect = [Property]()
|
||||
var propertiesToSelect = [Property]()
|
||||
let selectedPropertyId = selectedProperties.firstIndex { selectedProperty in
|
||||
selectedProperty.id == property.id
|
||||
private func toggleProperty(_ value: PropertyValue) -> (selected: [PropertyValue], deselected: [PropertyValue]) {
|
||||
var valuesToDeselect = [PropertyValue]()
|
||||
var valuesToSelect = [PropertyValue]()
|
||||
let selectedValueId = selectedValues.firstIndex { selectedValue in
|
||||
selectedValue.id == value.id
|
||||
}
|
||||
|
||||
if let selectedPropertyId = selectedPropertyId {
|
||||
if let selectedValueId = selectedValueId {
|
||||
// Removes previously selected filter
|
||||
selectedProperties.remove(at: selectedPropertyId)
|
||||
propertiesToDeselect.append(property)
|
||||
selectedValues.remove(at: selectedValueId)
|
||||
valuesToDeselect.append(value)
|
||||
} else {
|
||||
// Selectes unselected filter
|
||||
selectedProperties.append(property)
|
||||
propertiesToSelect.append(property)
|
||||
selectedValues.append(value)
|
||||
valuesToSelect.append(value)
|
||||
|
||||
// If the filter has filters to exclude, these filters marks as deselected
|
||||
let excludedProperties = excludeProperties(property)
|
||||
propertiesToDeselect.append(contentsOf: excludedProperties)
|
||||
let excludedValues = excludeValues(value)
|
||||
valuesToDeselect.append(contentsOf: excludedValues)
|
||||
}
|
||||
|
||||
return (propertiesToSelect, propertiesToDeselect)
|
||||
return (valuesToSelect, valuesToDeselect)
|
||||
}
|
||||
|
||||
private func excludeProperties(_ filter: Property) -> [Property] {
|
||||
let propertiesIdsToExclude = filter.excludingPropertiesIds
|
||||
private func excludeValues(_ values: PropertyValue) -> [PropertyValue] {
|
||||
let valuesIdsToExclude = values.excludingPropertiesIds
|
||||
|
||||
guard !propertiesIdsToExclude.isEmpty else {
|
||||
guard !valuesIdsToExclude.isEmpty else {
|
||||
return []
|
||||
}
|
||||
|
||||
var excludedProperties = [Property]()
|
||||
var excludedValues = [PropertyValue]()
|
||||
|
||||
for propertiesIdToExclude in propertiesIdsToExclude {
|
||||
let propertyToExclude = selectedProperties.first { property in
|
||||
property.id == propertiesIdToExclude
|
||||
for valuesIdToExclude in valuesIdsToExclude {
|
||||
let propertyToExclude = selectedValues.first { property in
|
||||
property.id == valuesIdToExclude
|
||||
}
|
||||
|
||||
if let propertyToExclude = propertyToExclude {
|
||||
let (_, deselected) = toggleProperty(propertyToExclude)
|
||||
excludedProperties.append(contentsOf: deselected)
|
||||
excludedValues.append(contentsOf: deselected)
|
||||
}
|
||||
}
|
||||
|
||||
return excludedProperties
|
||||
}
|
||||
|
||||
private func getPropertySafely(_ index: Int) -> Property? {
|
||||
guard (0..<properties.count).contains(index) else {
|
||||
return nil
|
||||
}
|
||||
|
||||
return properties[index]
|
||||
return excludedValues
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@
|
|||
// THE SOFTWARE.
|
||||
//
|
||||
|
||||
import TIUIKitCore
|
||||
import TISwiftUtils
|
||||
import UIKit
|
||||
|
||||
open class BaseFilterViewModel<CellViewModelType: FilterCellViewModelProtocol & Hashable,
|
||||
|
|
@ -28,44 +28,43 @@ open class BaseFilterViewModel<CellViewModelType: FilterCellViewModelProtocol &
|
|||
|
||||
// MARK: - FilterViewModelProtocol
|
||||
|
||||
public typealias Property = PropertyValue
|
||||
public typealias CellViewModelType = CellViewModelType
|
||||
public typealias Change = (indexPath: IndexPath, viewModel: CellViewModelType)
|
||||
|
||||
public var properties: [PropertyValue] = [] {
|
||||
public var values: [PropertyValue] = [] {
|
||||
didSet {
|
||||
filtersCollection?.update()
|
||||
}
|
||||
}
|
||||
public var selectedProperties: [PropertyValue] = [] {
|
||||
public var selectedValues: [PropertyValue] = [] {
|
||||
didSet {
|
||||
filtersCollection?.update()
|
||||
}
|
||||
}
|
||||
|
||||
public weak var filtersCollection: UpdatableView?
|
||||
public weak var filtersCollection: Updatable?
|
||||
|
||||
public private(set) var cellsViewModels: [CellViewModelType]
|
||||
|
||||
public init(filters: [PropertyValue], cellsViewModels: [CellViewModelType] = []) {
|
||||
self.properties = filters
|
||||
public init(filterPropertyValues: [PropertyValue], cellsViewModels: [CellViewModelType] = []) {
|
||||
self.values = filterPropertyValues
|
||||
self.cellsViewModels = cellsViewModels
|
||||
}
|
||||
|
||||
open func filterDidSelected(atIndexPath indexPath: IndexPath) -> [Change] {
|
||||
let (selected, deselected) = toggleProperty(atIndexPath: indexPath)
|
||||
|
||||
let changedProperties = properties
|
||||
let changedValues = values
|
||||
.enumerated()
|
||||
.filter { selected.contains($0.element) || deselected.contains($0.element) }
|
||||
|
||||
changedProperties.forEach { index, element in
|
||||
let isSelected = selectedProperties.contains(element)
|
||||
changedValues.forEach { index, element in
|
||||
let isSelected = selectedValues.contains(element)
|
||||
|
||||
setSelectedCell(atIndex: index, isSelected: isSelected)
|
||||
setSelectedProperty(atIndex: index, isSelected: isSelected)
|
||||
}
|
||||
|
||||
let changedItems = changedProperties
|
||||
let changedItems = changedValues
|
||||
.map {
|
||||
Change(indexPath: IndexPath(item: $0.offset, section: .zero),
|
||||
viewModel: cellsViewModels[$0.offset])
|
||||
|
|
@ -79,10 +78,6 @@ open class BaseFilterViewModel<CellViewModelType: FilterCellViewModelProtocol &
|
|||
}
|
||||
|
||||
open func setSelectedProperty(atIndex index: Int, isSelected: Bool) {
|
||||
properties[index].isSelected = isSelected
|
||||
}
|
||||
|
||||
open func isPropertyInArray(_ property: PropertyValue, properties: [PropertyValue]) -> Bool {
|
||||
properties.contains(where: { $0.id == property.id })
|
||||
values[index].isSelected = isSelected
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,13 +22,13 @@
|
|||
|
||||
open class DefaultFilterViewModel: BaseFilterViewModel<DefaultFilterCellViewModel, DefaultFilterPropertyValue> {
|
||||
|
||||
public init(filters: [DefaultFilterPropertyValue]) {
|
||||
let cellsViewModel = filters.compactMap {
|
||||
public init(filterPropertyValues: [DefaultFilterPropertyValue]) {
|
||||
let cellsViewModel = filterPropertyValues.compactMap {
|
||||
DefaultFilterCellViewModel(id: $0.id,
|
||||
title: $0.title,
|
||||
isSelected: $0.isSelected)
|
||||
}
|
||||
|
||||
super.init(filters: filters, cellsViewModels: cellsViewModel)
|
||||
super.init(filterPropertyValues: filterPropertyValues, cellsViewModels: cellsViewModel)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,15 +20,18 @@
|
|||
// THE SOFTWARE.
|
||||
//
|
||||
|
||||
import TISwiftUtils
|
||||
import TIUIKitCore
|
||||
import UIKit
|
||||
|
||||
@available(iOS 13.0, *)
|
||||
open class BaseFiltersCollectionView<CellType: UICollectionViewCell & ConfigurableView,
|
||||
PropertyValue: FilterPropertyValueRepresenter & Hashable>: UICollectionView,
|
||||
InitializableViewProtocol,
|
||||
UpdatableView,
|
||||
UICollectionViewDelegate where CellType.ViewModelType: FilterCellViewModelProtocol & Hashable {
|
||||
PropertyValue: FilterPropertyValueRepresenter & Hashable>:
|
||||
UICollectionView,
|
||||
InitializableViewProtocol,
|
||||
Updatable,
|
||||
UICollectionViewDelegate where CellType.ViewModelType: FilterCellViewModelProtocol & Hashable {
|
||||
|
||||
public enum DefaultSection: String {
|
||||
case main
|
||||
}
|
||||
|
|
@ -148,9 +151,9 @@ open class BaseFiltersCollectionView<CellType: UICollectionViewCell & Configurab
|
|||
}
|
||||
|
||||
open func applyChange(_ changes: [BaseFilterViewModel<CellType.ViewModelType, PropertyValue>.Change]) {
|
||||
for change in changes {
|
||||
changes.forEach { change in
|
||||
guard let cell = cellForItem(at: change.indexPath) as? CellType else {
|
||||
continue
|
||||
return
|
||||
}
|
||||
|
||||
cell.configure(with: change.viewModel)
|
||||
|
|
|
|||
|
|
@ -16,4 +16,5 @@ Pod::Spec.new do |s|
|
|||
s.dependency 'TINetworking', s.version.to_s
|
||||
s.dependency 'TIUIKitCore', s.version.to_s
|
||||
s.dependency 'TIUIElements', version.to_s
|
||||
s.dependency 'TISwiftUtils', version.to_s
|
||||
end
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
//
|
||||
// Copyright (c) 2022 Touch Instinct
|
||||
//
|
||||
// Permission is hereby granted, free of charge, to any person obtaining a copy
|
||||
// of this software and associated documentation files (the Software), to deal
|
||||
// in the Software without restriction, including without limitation the rights
|
||||
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
// copies of the Software, and to permit persons to whom the Software is
|
||||
// furnished to do so, subject to the following conditions:
|
||||
//
|
||||
// The above copyright notice and this permission notice shall be included in
|
||||
// all copies or substantial portions of the Software.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
|
||||
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
|
||||
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
|
||||
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
|
||||
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
|
||||
// THE SOFTWARE.
|
||||
//
|
||||
|
||||
public extension Array {
|
||||
subscript(safe index: Index) -> Element? {
|
||||
self.enumerated().first(where: { $0.offset == index })?.element
|
||||
}
|
||||
}
|
||||
|
|
@ -20,6 +20,6 @@
|
|||
// THE SOFTWARE.
|
||||
//
|
||||
|
||||
public protocol UpdatableView: AnyObject {
|
||||
public protocol Updatable: AnyObject {
|
||||
func update()
|
||||
}
|
||||
Loading…
Reference in New Issue