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

[Feature/mission component] 미션 컴포넌트 제작 #27

Merged
merged 17 commits into from
Dec 12, 2022

Conversation

jinsu4755
Copy link
Contributor

What is this issue?

공통으로 사용될 미션 컴포넌트 제작입니다.

현우형이 도메인 로직이 얽혀있다는 점을 이야기 해주었는데

초기 설계 의도는 앱 자체 미션 자체가 1~3단계로 구성되어있기 때문에 해당 1,2,3 이란 Int 값은 특별한 의미를 가지는 숫자라고 생각하였습니다.
따라서 미션의 레벨이라는 의미를 가지도록 각 숫자를 래핑 하였으며 1,2,3 이 아닌 숫자가 뷰에 들어간다면 요구 사항에 충족하지 못하는 CASE 라고 생각하여 이를 방지하고자 UI가 외부 파라미터로 도메인 로직을 주입 받게 됩니다.

피드백은 환영입니다

Reference

  • [x]

feature/mission-component
미션 레벨의 경우 기획에서 의도한 경우가 아닌 경우를 막기 위해 의미를 가지는
미션의 레벨이라는 객체로 래핑하였습니다.

feature/mission-component
feature/mission-component
feature/mission-component
@jinsu4755 jinsu4755 requested a review from a team as a code owner December 9, 2022 11:49
Copy link
Member

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

PR 내에 공통 컴포넌트 만드는 것과 미션 컴포넌트 만든 로직이 겹쳐있어서 살짝 양이 많은데 이제부터 PR 진짜 잘 쪼개야할듯....
우선 내가 이해가 안 가는 로직이 몇 부분 있어서 RC 누를게 답변 달아줘

Comment on lines +12 to +35
@Composable
fun LevelOfMission(stamp: Stamp, spaceSize: Dp) {
Row(
horizontalArrangement = Arrangement.spacedBy(spaceSize)
) {
MissionLevelOfStar(stamp = stamp)
}
}

@Composable
private fun MissionLevelOfStar(stamp: Stamp) {
repeat(MissionLevel.MAXIMUM_LEVEL) {
val starColor = if (it <= stamp.missionLevel.value) {
stamp.starColor
} else {
Stamp.defaultStarColor
}
Icon(
painter = painterResource(id = R.drawable.level_star),
contentDescription = "Star Of Mission Level",
tint = starColor
)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

이게 Ui 패키지 내에서 이런 컴포넌트를 제작하는 것에는 동의를 하지만, designsystem 패키지에 있다는 것은 전역에서 사용할 수 있는 컴포넌트를 넣어야 한다고 생각해. 디자인시스템이란 것 자체가 사용자에게 보여지는 뷰에 연관된 것만 정의를 해야지 이걸 도메인과 연관시켜서 구현하는 것은 살짝 오바인 것 같아.

Comment on lines 28 to 30
) {
val shape = MissionShape.DefaultWave
val stamp = Stamp.findStampByLevel(mission.level)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
) {
val shape = MissionShape.DefaultWave
val stamp = Stamp.findStampByLevel(mission.level)
) {
val (id, title, level, isCompleted, display) = mission
val shape = MissionShape.DefaultWave
val stamp = Stamp.findStampByLevel(level)

구조분해할당으로 사용하는건 어떰? 어차피 컴포넌트 내에서 사용할 uiState니까 굳이 멤버변수 참조하지말고 분해할당해서 사용하는게 더 편할 것 같음

Comment on lines +12 to +18
operator fun invoke(length: Float): MissionPattern {
return MissionPattern(
length = length,
diameter = length * DIAMETER_RATIO,
gap = length * GAP_RATIO
)
}
Copy link
Member

Choose a reason for hiding this comment

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

팩토리를 이렇게도 구현할 수 있다니... 👍🏻 또 배워갑니다.

feature/mission-component
feature/mission-component
Shape 의 Count는 프로퍼티로 들어가있으므로 함수에서 전달할 인자가 아니기 때문에 이를 수정하였음.

feature/mission-component
feature/mission-component
Copy link
Member

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

머지합시다

@jinsu4755 jinsu4755 merged commit fa163fe into develop Dec 12, 2022
@delete-merged-branch delete-merged-branch bot deleted the feature/mission-component branch December 12, 2022 23:03
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