-
Notifications
You must be signed in to change notification settings - Fork 15
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] #459 - 일정 상세뷰 UI 구현 #463
base: develop
Are you sure you want to change the base?
Conversation
- cell 클릭 시 indexPath 전달하여 section과 item 값에 따라 viewModel에서 처리
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 많으셨습니다 ㅎ.ㅎ 🫶🏻🫶🏻
PR에 남겨주신 부분들에 대해서 아래에 코멘트 달아두었습니다.
Scene내부에는 VC, VM, Views, Cell, CollectionView 등만 남겨두고 모두 Source의 하위로 옮기는 건 어떠신가요?
➡️ 네 동의합니다!
HomeCategoryTagView와 HomeSquareTagView 컴포넌트 명이 헷갈리다고 느꼈는데, HomeSquareTagView를 더 직관적인 이름으로 바꾸는건 어떨까요 ? DashBoard와 CalendarView 외에서 사용되지 않는다면 DashBoardCalendarCategoryType과 맞춰가도 좋을 듯 합니당
➡️ HomeSquareTagView
가 이후에 다른 곳에서도 사용될 수 있을 것 같아서 .. 최대한 컴포넌트의 생김새로 명명하려고 했었는데요. 컴포넌트의 쓰임에 따라서 명명하면, 이후에 다른 기능에서 활용해야 할 경우 확장성이 부족할 것이라고 생각했습니다. ..! 혹시 HomeCategoryTagView
의 명칭을 HomeLabelTagView
이런 식으로 변경하면.. 어떠실까요? 컴포넌트 명명의 근거는 아래와 같이 생각했습니다.
.changeCornerRadius(radius: 12) | ||
.setConfigForState(enabledFont: DSKitFontFamily.Suit.semiBold.font(size: 18)) | ||
|
||
private let gradationView = UIView().then { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dim도 꼼꼼하게 챙겨주셨네요 👀
|
||
if let gradientLayer = gradationView.layer.sublayers?.first as? CAGradientLayer { | ||
gradientLayer.frame = gradationView.bounds | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gradationView의 크기가 확정된 이후에, 그 크기만큼 gradientLayer의 채우기 위함인가요?
|
||
naviBar.snp.makeConstraints { make in | ||
make.top.leading.trailing.equalTo(view.safeAreaLayoutGuide) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공백 제거해주세욯
|
||
gradationView.snp.makeConstraints { make in | ||
make.leading.trailing.bottom.equalToSuperview() | ||
make.height.equalTo(209.adjustedH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
height에 기기 대응해두신 이유가 있나용?.?
collectionView.register(HomeCalendarDetailCVC.self, forCellWithReuseIdentifier: HomeCalendarDetailCVC.className) | ||
} | ||
|
||
@MainActor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MainActor
를 사용하신 이유가 궁금합니다.!
if let index = self.viewModel.calendarDetailList.firstIndex(where: {$0.isRecentSchedule}) { | ||
self.collectionView.scrollToItem(at: IndexPath(item: index, section: 0), | ||
at: .top, | ||
animated: true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize { | ||
let length = self.collectionView.frame.size.width | ||
return CGSize(width: length, height: 96.adjustedH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 기기 대응 height에만 넣으신 이유가 있나요?
@@ -29,7 +29,7 @@ enum DashBoardCalenderCategoryTagType { | |||
case .event: | |||
return DSKitAsset.Colors.success.color | |||
case .seminar: | |||
return DSKitAsset.Colors.success.color | |||
return DSKitAsset.Colors.secondary.color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
같은 색상에 대해서 secondary
를 success
로 쓰고 있었..네용 .. 다른 곳에서 발견되면 변경해두겠습니다.!
return HomeForMemberSectionLayoutKind.allCases.count | ||
} | ||
|
||
public func collectionView(_ collectionView: UICollectionView, viewForSupplementaryElementOfKind kind: String, at indexPath: IndexPath) -> UICollectionReusableView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public
삭제 감사합니닷 🙇🏼
} | ||
addDependency(coordinator) | ||
coordinator.start() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
1. 폴더링 수정 제안
Components>TagType>DashBoardCalenderCategoryTagType
,Components>HomeSquareTagView
,Coordinator
를 HomeScene과 CalendarScene이 공유하는 상태가 되었어요 !2. 컴포넌트명 변경
HomeCategoryTagView
와HomeSquareTagView
컴포넌트 명이 헷갈리다고 느꼈는데,HomeSquareTagView
를 더 직관적인 이름으로 바꾸는건 어떨까요 ? DashBoard와 CalendarView 외에서 사용되지 않는다면DashBoardCalendarCategoryType
과 맞춰가도 좋을 듯 합니당📌 참고 사항
navigationBar
를 추가했어요!현재 피그마 뷰에서는 home화면에 cell을 클릭하거나 tabBarItem을 클릭하는 경우 솝트로그로 이동할 수 있는데, cell을 클릭했을 때 이동하는 방식이 불필요하다고 생각되어 내일 회의 후 다시 반영하겠습니다.
퍼블리셔로 전달하는 값에
HomeForMemberSectionLayoutKind
를 넘겨주어 뷰모델에서 직관적으로 section을 구분하려고 했는데, dashBoard 외에서는IndexPath.row
도 필요할 것 같아서IndexPath
전체를 넘겨주었습니다.📸 스크린샷
📮 관련 이슈