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

[Feat] OverView, PlaceDetail뷰 구현 #164

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

Conversation

Chandrarla
Copy link
Collaborator

🔍 PR Content

홈 화면에 해당하는 OverView, PlaceDetail의 UI와 관련 로직을 구현했습니다.

📸 Screenshot

Simulator.Screen.Recording.-.iPhone.13.pro.-.2024-10-30.at.17.33.19.mp4

📍 PR Point

  • MockData를 생성했습니다.
  • OverView에 있는 장소 개수만큼 버튼, 컬렉션뷰를 동적 생성합니다.
  • PlaceDetailButton을 누르면 장소에 대한 데이터를 가지고 PlaceDetailVC로 이어집니다.
  • PlaceDetailVC에서 전시 리스트, 후기 영상을 보여주는 SegmentedControl을 구현했습니다
  • 전시 리스트에서 Place-PlaceInfo의 개수만큼 해당 장소의 전시에 대한 Cell이 생성됩니다.

resolve #154

@Chandrarla Chandrarla added 🎥 Feature UI & 기능 개발 형근 labels Oct 30, 2024
@Chandrarla Chandrarla self-assigned this Oct 30, 2024
Copy link
Collaborator

@sozohoy sozohoy left a comment

Choose a reason for hiding this comment

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

고생하셨습닌다!
작업량이 상당하시네요.. 한가지 당부 드리자면
이슈 단위가 현재 너무 큽니다. 크면 클 수록 코드 리뷰를 하기 어려워지는 부분이 있어 조금만 나눠보아도 괜찮을 것 같습니다~


public class PlaceDetailButton: UIButton {

public var place: Place?
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1
데이터를 가지고 있을 필요 없어보입니다

import Core

final class ExhibitionListView: UIView {
private var exhibitionData: Place?
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2
데이터를 들고 있는 이유가 무엇인가요?
만약 들고 있어야 한다면, place로 네이밍을 수정하는 것이 좋아보여요

Comment on lines +143 to +165
private func createCollectionView() -> UICollectionView {
let layout = UICollectionViewFlowLayout()
layout.scrollDirection = .horizontal
layout.minimumLineSpacing = 12
layout.itemSize = CGSize(width: 135, height: 240)
layout.sectionInset = UIEdgeInsets(
top: 0,
left: 0,
bottom: 0,
right: 0
)

let collectionView = UICollectionView(
frame: .zero,
collectionViewLayout: layout
)
collectionView.showsHorizontalScrollIndicator = false
collectionView.register(
UICollectionViewCell.self,
forCellWithReuseIdentifier: "DefaultCell"
)
collectionView.delegate = self
collectionView.dataSource = self
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3
컬뷰를 구현하도록 리턴한 이유가 무엇인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

configureStackView()에 작성하려고 컬뷰를 리턴했어요
OverView 화면에서 전시 공간 개수만큼 (버튼, 컬뷰) 세트를 만들어야 하는걸로 알고 있어서
for문 돌면서 (버튼, 컬뷰)를 동적 생성해줬고, 컬뷰를 생성하는 함수는 별도로 작성한것입니당

Comment on lines +171 to +177
if sender == exhibitionListView.allFilterButton {
viewModel.updateFilterState(selected: .all)
} else if sender == exhibitionListView.freeFilterButton {
viewModel.updateFilterState(selected: .free)
} else if sender == exhibitionListView.endSoonFilterButton {
viewModel.updateFilterState(selected: .endSoon)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2
이거 개선할 방법이 없을까여??

Comment on lines +197 to +201
viewModel.onFilterChanged?(
viewModel.allFilterState,
viewModel.freeFilterState,
viewModel.endSoonFilterState
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

p2
뷰모델 내부에서 내부의 상태를 변경하는게 더 적합해 보입니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
형근 🎥 Feature UI & 기능 개발
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[Feat] Overview, PlaceDetail뷰 구현
2 participants