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

[chore] 자체 QA - 코스 상세(Course Detail) 뷰 #207

Merged
merged 4 commits into from
Aug 28, 2024

Conversation

t1nm1ksun
Copy link
Contributor

@t1nm1ksun t1nm1ksun commented Aug 28, 2024

Related issue 🛠

Work Description ✏️

  • 금액 태그 String Mapper 적용
  • total cost가 0원일 때 무지출로 출력
  • 미개봉 코스 아이콘 png로 수정
  • 코스 상세 태그들 영역 비율 설정

Screenshot 📸

금액 태그 적용

image

무지출 출력

image

미개봉 코스 아이콘 변경

image

태그들 영역 비율 설정

image

Uncompleted Tasks 😅

  • N/A

To Reviewers 📢

@t1nm1ksun t1nm1ksun self-assigned this Aug 28, 2024
@t1nm1ksun t1nm1ksun requested a review from a team as a code owner August 28, 2024 07:05
@t1nm1ksun t1nm1ksun changed the title [refactor] 자체 QA - 코스 상세(Course Detail) 뷰 [chore] 자체 QA - 코스 상세(Course Detail) 뷰 Aug 28, 2024
Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

왜 벌써 4시 반이에요?? ㅠㅠ

@t1nm1ksun t1nm1ksun merged commit b06b09a into develop Aug 28, 2024
1 check passed
@t1nm1ksun t1nm1ksun deleted the refactor-course-detail branch August 28, 2024 07:25
Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

밀린 코드 리뷰 다는 중 ㅋㅋ,,

@@ -236,6 +236,7 @@
<string name="course_detail_bottom_sheet_confirm">글 삭제</string>
<string name="course_detail_bottom_sheet_delete">닫기</string>
<string name="course_detail_bottom_sheet_report">신고하기</string>
<string name="course_detail_total_cost_zero">무지출</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

course_detail_total_cost_zero _cost

네이밍 통일시켜줍시다

Comment on lines +21 to +24
object TotalCostZero {
const val ZEROCOST = "무지출"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

data Layer는 presentation Layer에 의존성을 가지면 안 됩니다 (이러면 클린 아키택처 위배)
요거는 Mapper에서만 쓰이는 부분이니 domain Layer로 이동 시켜주는 게 좋을 것 같아요

@@ -18,6 +18,10 @@ object TimePicker {
const val PM = "오후"
}

object TotalCostZero {
const val ZEROCOST = "무지출"
Copy link
Contributor

Choose a reason for hiding this comment

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

ZERO_COST

Comment on lines +8 to +12
fun Int.toCost(): String = if (this == 0) {
ZEROCOST
} else {
"${NumberFormat.getNumberInstance(Locale.KOREA).format(this)}${Cost.COST}"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

개인적으로 확장성을 고려하여 when문을 사용하는 걸 선호하는 편이지만 갠취니까 안 따라도 되긴 합니당
그리고 0도 상수화 해주는 게 좋아보여요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[chore] 자체 QA - 코스 상세(Course Detail) 뷰
2 participants