-
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] #473 - 신규 홈뷰 - 홍보 섹션 API 연동 #480
base: develop
Are you sure you want to change the base?
[Feat] #473 - 신규 홈뷰 - 홍보 섹션 API 연동 #480
Conversation
profileImage, name, images
|
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 func getAnnouncementPosts() { | ||
repository.getAnnouncementPosts() | ||
.withUnretained(self) | ||
.sink { event in | ||
print("GetAnnouncementPosts State: \(event)") | ||
} receiveValue: { owner, posts in | ||
owner.announcementPosts.send(posts) | ||
} | ||
.store(in: cancelBag) | ||
} |
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.
이 부분 어제 논의한대로 Subject를 반환하는 방식으로 변경하는건 어떨까요?
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.
넹 변경하는 건 동의하는데, 한꺼번에 기존 코드들과 같이 리팩하는 방향으로 가는 건 어떨까요?
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.
좋아요!
import Domain | ||
import Networks | ||
|
||
extension HomeEmploymentEntity { |
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.
ResponseEntity와 RequestEntity 구분해주세요!
제가 재현님 다음에 올린 PR에 ResponseEntity 폴더를 만들어놓았는데, #481 머지 후 수정해도 좋을 것 같네용
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.
옙옙! 머지 해주시면 수정하겠습니닷
@@ -305,10 +293,11 @@ extension HomeForMemberVC: UICollectionViewDataSource { | |||
case .announcement: | |||
/// 홍보 카드 셀 | |||
let announcementIndex = indexPath.item | |||
guard let announcement = viewModel.announcementPosts?[safe: announcementIndex] else { return UICollectionViewCell() } |
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.
컬렉션뷰 UI 업데이트할 때 문득 ViewModel의 변수를 참조한다는 것이 어색하게 느껴져서 고민한 부분이 있는데 재현님의 생각도 궁금합니다 !! #481 에 생각 적어두었는데 의견 주세요 ! !
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.
확인했어요! 홈뷰가 많이 복잡한 편이라서 PresentationModel 만들어서 분리해주는 거 좋은 방법인 듯 해요 🙇🏼 요거 이후에 이슈 파서 한 번에 처리하겠습니다!
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
신규 홈뷰의 홍보 섹션 API를 연동했습니다.
📌 참고 사항
📸 스크린샷
📮 관련 이슈