Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

[feature/mission-detail] 미션 상세 화면 제작 #25

Merged
merged 17 commits into from
Dec 14, 2022

Conversation

l2hyunwoo
Copy link
Member

Reference

  • 미션 상세 화면을 만들어봅시다

@l2hyunwoo l2hyunwoo self-assigned this Dec 9, 2022
@l2hyunwoo l2hyunwoo requested a review from a team as a code owner December 9, 2022 10:03
@l2hyunwoo l2hyunwoo force-pushed the feature/mission-detail branch from 681683b to 0d5210c Compare December 12, 2022 02:48
Copy link
Contributor

@jinsu4755 jinsu4755 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~ 크게 보이는 부분 이외는 시간상 넘어갈게요~

@DrawableRes icon: Int,
maxStars: Int = DEFAULT_MAX,
stars: Int,
gapSize: Int = 10,
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 부분에서 결국 Dp 단위의 사이즈를 받아야 하는 것이라면, 받을 수 있는 인자를 Dp 타입으로 고정하는 것은 어떨까요?
Component 로 사용된다면, 대부분의 경우 dp 를 넣기 때문에 추후 인수인계를 위해서도 여기에서만 int 로 들어간다 라기보다 dp 를 받을 수 있도록 통일하면 좋을 것 같다는 생각이 들었습니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok! 수정하겠습니다.

Comment on lines 31 to 32
fun getIdFrom(type: ToolbarIconType) = values().find { it.name == type.name }?.resId
?: throw IllegalArgumentException("There's no icon in this class. Icon: ${type.name}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Enum 타입을 받아서 Enum 타입 들 중에서 해당하는 Enum의 id 를 뽑아오는 모습이 개인적으로 부자연스럽다는 생각이 듭니다.
ToolbarIconType.WRITE.resId 와 차이점이 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 그렇군요! 이 부분은 제가 따로 로직 만들어서 수정하도록 하겠습니다.

@l2hyunwoo l2hyunwoo merged commit 58874c1 into develop Dec 14, 2022
@delete-merged-branch delete-merged-branch bot deleted the feature/mission-detail branch December 14, 2022 02:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants