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

[fix] 1차 QA 반영 #364

Merged
merged 19 commits into from
Sep 4, 2024
Merged

[fix] 1차 QA 반영 #364

merged 19 commits into from
Sep 4, 2024

Conversation

youz2me
Copy link
Member

@youz2me youz2me commented Sep 4, 2024

🔗 연결된 이슈

📄 작업 내용

  • 1차 QA 반영했습니다.

👀 기타 더 이야기해볼 점

  • 현재 네비게이션 바 구분선 삭제 문제와 우리들의 준비 현황 CollectionView 업데이트 문제 제외하고는 전부 해결된 상태입니다.
  • 우리들의 준비 현황 CollectionView 업데이트 문제는 로직 자체를 변경해야 할 것 같아서 ... 일단 TL님과 이야기해본 결과 릴리즈 이후로 넘길까 해요. 의견 있으시면 말씀해주세요!
  • 네비게이션 바 구분선 삭제 문제는 지금 Lookin이 지속해서 돌아가지 않는 문제가 있기도 하고 ... 제 뷰에서 강제로 구분선을 건드리면 약속 상세 화면이 아니라 모임 상세 화면의 구분선이 삭제되더라구요. 이유를 몰라서 일단 PR 먼저 올립니다!

@youz2me youz2me added 🛠️ fix 버그나 오류 해결시 사용 💚 YouJin 저댄고?저댄고?저댄고?저댄고?저댄고? labels Sep 4, 2024
@youz2me youz2me self-assigned this Sep 4, 2024
@youz2me youz2me linked an issue Sep 4, 2024 that may be closed by this pull request
4 tasks
Copy link
Member

@mmaybei mmaybei left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Copy link
Contributor

@JinUng41 JinUng41 left a comment

Choose a reason for hiding this comment

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

고생하셨어요~

Comment on lines +119 to +148
if let moveTime = formatter.date(from: moveStartTime.value) {
let calendar = Calendar.current

let moveTimeComponents = calendar.dateComponents([.hour, .minute], from: moveTime)
let currentTimeComponents = calendar.dateComponents([.hour, .minute], from: Date())

if let moveHour = moveTimeComponents.hour, let moveMinute = moveTimeComponents.minute,
let currentHour = currentTimeComponents.hour, let currentMinute = currentTimeComponents.minute {
if currentHour > moveHour || (currentHour == moveHour && currentMinute > moveMinute) {
self.isLate.value = true
} else {
self.isLate.value = false
}
}
}
case .ready:
if let readyTime = formatter.date(from: readyStartTime.value) {
let calendar = Calendar.current

let readyTimeComponents = calendar.dateComponents([.hour, .minute], from: readyTime)
let currentTimeComponents = calendar.dateComponents([.hour, .minute], from: Date())

if let readyHour = readyTimeComponents.hour, let readyMinute = readyTimeComponents.minute,
let currentHour = currentTimeComponents.hour, let currentMinute = currentTimeComponents.minute {
if currentHour > readyHour || (currentHour == readyHour && currentMinute > readyMinute) {
self.isLate.value = true
} else {
self.isLate.value = false
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

공통되는 부분은 메서드로 분리할 수 있을 것 같아요.
'DRY'라고 'Don't Repeat Yourself'인데, 개발 시 동일한 코드를 반복하지 말라라는 뜻입니다.

Comment on lines +99 to +100
viewModel.promiseInfo.bindOnMain(with: self) { owner, model in
owner.rootView.myReadyStatusProgressView.isUserInteractionEnabled = (model?.isParticipant ?? false)
Copy link
Contributor

Choose a reason for hiding this comment

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

괄호는 삭제 해도 괜찮지 않을까요?

// MARK: - UICollectionViewDelegateFlowLayout

extension PromiseInfoViewController: UICollectionViewDelegateFlowLayout {
func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 메서드가 너무 길기 때문에 개행해도 좋을 것 같네요.

@youz2me youz2me merged commit 2a5777d into suyeon Sep 4, 2024
@JinUng41 JinUng41 deleted the fix/#357-qa branch September 13, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ fix 버그나 오류 해결시 사용 💚 YouJin 저댄고?저댄고?저댄고?저댄고?저댄고?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fix] 1차 QA 반영
3 participants