Skip to content

Conversation

@soopeach
Copy link
Member

@soopeach soopeach commented Dec 5, 2024

😎 작업 내용

  • 챌린지 상세 페이지에서 도전 내용 인디케이터 변경 사항 반영

🧐 변경된 내용

  • 챌린지 관련 모델 일부 수정

🥳 동작 화면

유형 1 유형 2 유형 3 유형 4
Screenshot 2024-12-05 at 3 47 49 PM Screenshot 2024-12-05 at 3 47 55 PM Screenshot 2024-12-05 at 3 48 03 PM Screenshot 2024-12-05 at 3 49 10 PM

🤯 이슈 번호

  • 이슈 없음

🥲 비고

  • 비고 없음

@soopeach soopeach self-assigned this Dec 5, 2024
@soopeach soopeach force-pushed the design/challenge_goal_indicator branch from 166a6ef to 7b99c6a Compare December 5, 2024 07:16
Copy link
Member

@peter-j0y peter-j0y left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 👍

val progressBarColor = challengeColor.progressBarColor

val progress =
if (challenge.progress == null) 0f else requireNotNull(challenge.progress) / 100f
Copy link
Member

Choose a reason for hiding this comment

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

requireNotNull을 쓰면 challenge.progress가 null이면 exception 나지 않나요? 아마 challenge.progress는 nullable한 상황이라 사용하셨을 것 같은데 다음과 같은 방식은 어떨까요?

Suggested change
if (challenge.progress == null) 0f else requireNotNull(challenge.progress) / 100f
val challengeProgress = challenge.progress
if (challengProgress == null) 0f else challenge.progress / 100f

goal = "목표",
limit = when (challenge.challengeType) {
ChallengeType.LIFE -> {
"연속 " + challenge.period.toString() + "일"
Copy link
Member

Choose a reason for hiding this comment

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

nit) "연속 ${challenge.period}일"로 하면 더 간결하게 할 수 있을 것 같아요!

)

Text(
text = "${deadLineInfo[0]}일 ${deadLineInfo[1]}시간 ${deadLineInfo[2]}분" + "남음",
Copy link
Member

Choose a reason for hiding this comment

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

index exception이 날 수도 있을 것 같아서 default 값이 있으면 좋을 것 같긴하네요!

@yonghanJu yonghanJu merged commit 062bc5c into compose/develop Dec 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants