Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GWL-16] 운동 동료 선택 화면 UI 구현 #69

Merged
merged 13 commits into from
Nov 20, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import DesignSystem
import OSLog
import UIKit

// MARK: - WorkoutEnvironmentSetupViewController
Expand All @@ -15,21 +16,17 @@ public final class WorkoutEnvironmentSetupViewController: UIViewController {
override public func viewDidLoad() {
super.viewDidLoad()
setup()
}

override public func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)

insertTempSource()
}

lazy var contentNAV: UINavigationController = {
let nav = UINavigationController(rootViewController: workoutSelectView)
lazy var contentNavigationController: UINavigationController = {
let nav = UINavigationController(rootViewController: workoutSelectViewController)

return nav
}()

private let workoutSelectView = WorkoutSelectViewController()
private let workoutSelectViewController = WorkoutSelectViewController()
private let workoutPeerSelectViewController = WorkoutPeerSelectViewController()

private let pageControl: GWPageControl = {
let pageControl = GWPageControl(count: Constant.countOfPage)
Expand All @@ -39,21 +36,29 @@ public final class WorkoutEnvironmentSetupViewController: UIViewController {
}()

var dataSource: UICollectionViewDiffableDataSource<Int, UUID>!
var workoutCardCollectionView: UICollectionView!
var workoutTypesCollectionView: UICollectionView!
var workoutPaerTypesCollectionView: UICollectionView!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2

사용되지 않는 프로퍼티 같습니다~

Suggested change
var workoutPaerTypesCollectionView: UICollectionView!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 이 부분은, dataSource를 WorkoutEnvironmentSetupViewController에서 다루고 싶었습니다. 현재는 종속적인 VC 내부에서 dataSource를 init하지만, 차후 리팩토링을 위해 남겨두었습니다. 이부분을 주석을 작성하지 않아 혼란을 야기한 것 같습니다...!

}

private extension WorkoutEnvironmentSetupViewController {
func bind() {
workoutTypesCollectionView = workoutSelectViewController.workoutTypesCollectionView

workoutSelectViewController.delegate = self
}

func setup() {
view.backgroundColor = .systemBackground
view.backgroundColor = DesignSystemColor.primaryBackground
setupViewHierarchyAndConstraints()
workoutCardCollectionView = workoutSelectView.workoutCardCollectionView
addNavigationGesture()
bind()

configureDataSource()
}

func configureDataSource() {
dataSource = .init(collectionView: workoutCardCollectionView, cellProvider: { collectionView, indexPath, _ in
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: WorkoutCardCell.identifier, for: indexPath)
dataSource = .init(collectionView: workoutTypesCollectionView, cellProvider: { collectionView, indexPath, _ in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2

위에 workoutTypesCollectionView를 전역으로 강제언래핑 하셨는데요!
여기서만 사용되는거같아서 bind()랑 전역을 지우고 이 함수 안에서만 선언하고 사용하시는건 어떠신가요~?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다!

let cell = collectionView.dequeueReusableCell(withReuseIdentifier: WorkoutSelectTypeCell.identifier, for: indexPath)
return cell
})
}
Expand All @@ -76,15 +81,49 @@ private extension WorkoutEnvironmentSetupViewController {
pageControl.trailingAnchor.constraint(equalTo: safeArea.trailingAnchor, constant: -23).isActive = true
pageControl.heightAnchor.constraint(equalToConstant: 30).isActive = true

view.addSubview(contentNAV.view)
contentNAV.view.translatesAutoresizingMaskIntoConstraints = false
contentNAV.view.leadingAnchor.constraint(equalTo: safeArea.leadingAnchor).isActive = true
contentNAV.view.trailingAnchor.constraint(equalTo: safeArea.trailingAnchor).isActive = true
contentNAV.view.topAnchor.constraint(equalTo: pageControl.bottomAnchor).isActive = true
contentNAV.view.bottomAnchor.constraint(equalTo: safeArea.bottomAnchor).isActive = true
view.addSubview(contentNavigationController.view)
contentNavigationController.view.translatesAutoresizingMaskIntoConstraints = false
contentNavigationController.view.leadingAnchor.constraint(equalTo: safeArea.leadingAnchor).isActive = true
Comment on lines +85 to +86
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3

NSLaoutConstraint를 사용하면 .isActive를 사용하지 않아도되서 편리하더라구요!
추천드립니다~

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 사실 NSLayoutConstraint를 좋아하지 않는데요, 이유는 가끔가다 Constraint를 저장해야할 때가 있기 때문입니다. 그 때는 그 프로퍼티만 따로 뺴서 지장하는 방식이 예쁘지 않더군요... 그래도 이번 기회에 고민해보겠습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이유가 있으셨군요..!
저는 그런 경험을 못해봐서 이렇게 말씀드린거 같아요 ㅠㅠ
다함님 편하실대로 하시면 될거같습니다~!

contentNavigationController.view.trailingAnchor.constraint(equalTo: safeArea.trailingAnchor).isActive = true
contentNavigationController.view.topAnchor.constraint(equalTo: pageControl.bottomAnchor).isActive = true
contentNavigationController.view.bottomAnchor.constraint(equalTo: safeArea.bottomAnchor).isActive = true
}

func addNavigationGesture() {
guard let recognizer = contentNavigationController.interactivePopGestureRecognizer else { return }
recognizer.delegate = self
recognizer.isEnabled = true
contentNavigationController.delegate = self
}

enum Constant {
static let countOfPage = 2
}
}

// MARK: UIGestureRecognizerDelegate

extension WorkoutEnvironmentSetupViewController: UIGestureRecognizerDelegate {
public func gestureRecognizerShouldBegin(_: UIGestureRecognizer) -> Bool {
return contentNavigationController.viewControllers.count > 1
}
}

// MARK: UINavigationControllerDelegate

extension WorkoutEnvironmentSetupViewController: UINavigationControllerDelegate {
public func navigationController(_: UINavigationController, didShow viewController: UIViewController, animated _: Bool) {
if viewController == workoutSelectViewController {
pageControl.makePage(index: 0)
}
}
}

// MARK: WorkoutSelectViewDelegate

extension WorkoutEnvironmentSetupViewController: WorkoutSelectViewDelegate {
func nextButtonDidTap() {
pageControl.makeNextPage()
contentNavigationController.pushViewController(workoutPeerSelectViewController, animated: true)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
//
// WorkoutPeerSelectViewController.swift
// RecordFeature
//
// Created by MaraMincho on 11/19/23.
// Copyright © 2023 kr.codesquad.boostcamp8. All rights reserved.
//

import DesignSystem
import UIKit

// MARK: - WorkoutPeerSelectViewController

final class WorkoutPeerSelectViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()
view.backgroundColor = DesignSystemColor.primaryBackground

setup()
}

var dataSource: UICollectionViewDiffableDataSource<Int, UUID>!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2

여기도 강제언래핑으로 보여지는데요!
사용하신걸 확인해보니 아래처럼 선언해도 괜찮을것 같아요!

Suggested change
var dataSource: UICollectionViewDiffableDataSource<Int, UUID>!
var dataSource: UICollectionViewDiffableDataSource<Int, UUID>?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3

또한 셀이 2개밖에 없어서 DiffableDatasource를 사용하지 않아도 될것 같다는 의견입니다!

스프린트할 때, 리뷰어님들께서 재사용되는 셀이 없을때는 CollectionView나 TableView를 사용하지 않는게 좋을것 같다고 하셨던게 기억나네요.
오버엔지니어링처럼 보여서 그러신걸까요 ㅎㅎ.. 이유는 찾지 못한것같습니다

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 CollectionView로 구현한 이유는, 다양한 환경을 고려해서 였습니다. 현재는 랜덤 매칭과 혼자 달리기 뿐이지만, 다양한 조건을 추가할 수 있다고 생각했습니다. 가령 클럽 멤버와 달리기나 친구와 달리기 그리고 동시에 시간에 맞춰 달리기 등 다양한 메뉴가 추가될 수 있다고 생각했습니다. 유연한 구조를 생각하다 보니, 재사용 관점에서의 diffable datasource는 그닥 구미가 당기지 않는 선택지 같네요...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아닙니다!
재사용관점에서 Diffable Datasource는 제 생각에 가장 좋은 선택지 같아요!
제가 말씀드린건 그저 셀이 2개인데 CollectionView를 사용하는게 오버엔지니어링이 될 수 있겠다고 말씀드린건데요!
다함님께서 납득할만한 이유를 설명해주셔서 변경안하셔도 될것같아요 🤗


private let workoutSelectDescriptionLabel: UILabel = {
let label = UILabel()
label.font = .preferredFont(forTextStyle: .title1, with: .traitBold)
label.textAlignment = .left

label.text = "2. 누구랑 할까요?"

label.translatesAutoresizingMaskIntoConstraints = false
return label
}()

private let startButton: UIButton = {
let button = UIButton()
button.configurationUpdateHandler = UIButton.Configuration.mainCircular(label: "출발")

button.translatesAutoresizingMaskIntoConstraints = false
return button
}()

lazy var pearTypeSelectCollectionView: UICollectionView = {
let collectionView = UICollectionView(frame: .zero, collectionViewLayout: makeCollectionViewLayout())
collectionView.register(WorkoutPeerTypeSelectCell.self, forCellWithReuseIdentifier: WorkoutPeerTypeSelectCell.identifier)
collectionView.backgroundColor = .clear
collectionView.isScrollEnabled = false

collectionView.translatesAutoresizingMaskIntoConstraints = false
return collectionView
}()
}

private extension WorkoutPeerSelectViewController {
func makeCollectionViewLayout() -> UICollectionViewCompositionalLayout {
let item = NSCollectionLayoutItem(layoutSize: .init(widthDimension: .fractionalWidth(1), heightDimension: .fractionalHeight(1)))
item.contentInsets = .init(top: Metrics.itemInsets, leading: Metrics.itemInsets, bottom: Metrics.itemInsets, trailing: Metrics.itemInsets)

let groupSize = NSCollectionLayoutSize(widthDimension: .fractionalWidth(1), heightDimension: .fractionalHeight(0.15))
let group = NSCollectionLayoutGroup.vertical(layoutSize: groupSize, subitems: [item])

let section = NSCollectionLayoutSection(group: group)

return UICollectionViewCompositionalLayout(section: section)
}

func setup() {
setHierarchyAndConstraints()

dataSource = .init(collectionView: pearTypeSelectCollectionView, cellProvider: { collectionView, indexPath, _ in
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: WorkoutPeerTypeSelectCell.identifier, for: indexPath)

return cell
})

tempInitDataSource()
}

func setHierarchyAndConstraints() {
let safeArea = view.safeAreaLayoutGuide

view.addSubview(workoutSelectDescriptionLabel)
workoutSelectDescriptionLabel.topAnchor.constraint(equalTo: safeArea.topAnchor).isActive = true
workoutSelectDescriptionLabel.leadingAnchor
.constraint(equalTo: safeArea.leadingAnchor, constant: ConstraintsGuideLine.value).isActive = true
workoutSelectDescriptionLabel.trailingAnchor
.constraint(equalTo: safeArea.trailingAnchor, constant: -ConstraintsGuideLine.value).isActive = true

view.addSubview(startButton)
startButton.bottomAnchor.constraint(equalTo: safeArea.bottomAnchor, constant: -50).isActive = true
startButton.centerXAnchor.constraint(equalTo: safeArea.centerXAnchor).isActive = true
startButton.widthAnchor.constraint(equalToConstant: Metrics.buttonHeight).isActive = true
startButton.heightAnchor.constraint(equalToConstant: Metrics.buttonHeight).isActive = true

view.addSubview(pearTypeSelectCollectionView)
pearTypeSelectCollectionView.topAnchor
.constraint(equalTo: workoutSelectDescriptionLabel.bottomAnchor, constant: 15).isActive = true
pearTypeSelectCollectionView.leadingAnchor
.constraint(equalTo: safeArea.leadingAnchor, constant: ConstraintsGuideLine.secondaryValue).isActive = true
pearTypeSelectCollectionView.trailingAnchor
.constraint(equalTo: safeArea.trailingAnchor, constant: -ConstraintsGuideLine.secondaryValue).isActive = true
pearTypeSelectCollectionView.bottomAnchor.constraint(equalTo: view.bottomAnchor).isActive = true
}

func tempInitDataSource() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3

함수이름이 명확했으면 좋겠습니다..!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하겠습니다!

var snapshot = dataSource.snapshot()
snapshot.appendSections([0])
snapshot.appendItems([.init(), .init(), .init()])
dataSource.apply(snapshot)
}

enum Metrics {
static let buttonHeight: CGFloat = 150
static let buttonWidth: CGFloat = 150

static let itemInsets: CGFloat = 9
}
}
Loading
Loading