[Feature/#873] 오늘의 솝마디 콕찌르기 추천 기능 구현 #881
Conversation
- di 모듈 추가 - network api 구현
- mapper 구현 - 레포지토리 인터페이스 생성 및 적용 - 힐트모듈 수정
…into feature/869
|
s9hn
changed the title
| @@ -60,7 +60,7 @@ internal fun TodayFortuneDashboard( | |||
| ) { | |||
| Spacer(modifier = Modifier.height(height = 32.dp)) | |||
| Image( | |||
| painter = painterResource(R.drawable.img_fortune_title), | |||
| painter = painterResource(img_fortune_title), | |||
There was a problem hiding this comment.
이렇게도 할 수 있는건 처음 알았네요 ㄷㄷ
There was a problem hiding this comment.
그냥 문득 든 생각인데, PokeRepository 함수 수 왤케 많냐...이거 완전 갓 인터페이스 느낌인데ㅋㅋㅋㅋㅋㅋㅋ
| onProfileClick = { userId -> | ||
| context.startActivity( | ||
| Intent(Intent.ACTION_VIEW, Uri.parse("https://playground.sopt.org/members/${userId}")) | ||
| ) | ||
| }, |
There was a problem hiding this comment.
외부 뷰로 빠지는건가? 우리 WebViewActivity 활용하는 스펙은 아닌거?
There was a problem hiding this comment.
기존에 콕찌르기에서 저렇게 되어있어서 세훈햄한테 외부 뷰로 빠진다고 이야기했었습니다!
근데 음... 협의만 된다면 WebVIewActivity 쓰는것도 좋을 것 같네용
| isEmptyProfile = false, | ||
| onPokeClick = { }, | ||
| onProfileClick = { }, | ||
| userDescription = "1100기 iOS", |
| when (isEmptyProfile) { | ||
| true -> { | ||
| Image( | ||
| painter = painterResource(ic_empty_profile), | ||
| contentDescription = "profileImageEmpty", | ||
| modifier = modifier, | ||
| ) | ||
| } | ||
|
|
||
| false -> { | ||
| UrlImage( | ||
| url = profile, | ||
| contentDescription = "profileImage", | ||
| modifier = modifier, | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
음? 이거 이렇게 구현한 이유가 있을까?
차라리 profile: String = ""
| when (isEmptyProfile) { | |
| true -> { | |
| Image( | |
| painter = painterResource(ic_empty_profile), | |
| contentDescription = "profileImageEmpty", | |
| modifier = modifier, | |
| ) | |
| } | |
| false -> { | |
| UrlImage( | |
| url = profile, | |
| contentDescription = "profileImage", | |
| modifier = modifier, | |
| ) | |
| } | |
| } | |
| } | |
| when (profile.isBlankOrEmpty()) { | |
| true -> { | |
| Image( | |
| painter = painterResource(ic_empty_profile), | |
| contentDescription = "profileImageEmpty", | |
| modifier = modifier, | |
| ) | |
| } | |
| false -> { | |
| UrlImage( | |
| url = profile, | |
| contentDescription = "profileImage", | |
| modifier = modifier, | |
| ) | |
| } | |
| } | |
| } |
정도로 줄이고 isEmptyProfile은 빼도 되지 않을까? 상태가 이원화되어서 관리가 복잡한것 같아.
There was a problem hiding this comment.
처음에 아래처럼 했었는데, 모델에서 로직 적용 후 결과값을 넘겨주는게 맞나..해서 위처럼 구현했습니다.
저도 확신은 없었어서 누누씨 의견대로 가겠읍니다
| data class Success( | ||
| val todaySentence: TodaySentence, | ||
| val userInfo: UserInfo, |
There was a problem hiding this comment.
?? 저는 원래 인정파엿는데요 ㅋㅋ 뷰싸개는 도메인도 필요업다..
아무리 생각해도 ui의 state 키는 서버에서 내려주는 data이고 success를 대체할만한 워딩이 떠오르지않음 .. :(
| onProfileClick = { userId -> | ||
| context.startActivity( | ||
| Intent(Intent.ACTION_VIEW, Uri.parse("https://playground.sopt.org/members/${userId}")) | ||
| ) | ||
| }, |
There was a problem hiding this comment.
기존에 콕찌르기에서 저렇게 되어있어서 세훈햄한테 외부 뷰로 빠진다고 이야기했었습니다!
근데 음... 협의만 된다면 WebVIewActivity 쓰는것도 좋을 것 같네용
| onPokeClick = { onPokeClick(uiState.userInfo.userId) }, | ||
| onProfileClick = { onProfileClick(uiState.userInfo.userId) } |
There was a problem hiding this comment.
onPokeClick 함수에서 userId 값을 넘겨준다면 아래처럼 간단하게 쓸 수 있어요.
아래처럼 작성한다면 코드가 간단해지고, 캡쳐되기 때문에 약간의 성능 향상이 있지만, id라는 값을 넘겨주는 것 때문에 너무 강한 결합이 생기지 않을까?? 라는 고민을 요즘 하고있습니다. 혹시 어떻게 생각하시나용???
| onPokeClick = { onPokeClick(uiState.userInfo.userId) }, | |
| onProfileClick = { onProfileClick(uiState.userInfo.userId) } | |
| onPokeClick = onPokeClick, | |
| onProfileClick = onProfileClick |
There was a problem hiding this comment.
오호~ 킹치만 호이스팅된 onXXXClick 함수들은 인자로 아무것도 넘겨주지 않아서 아래코드처럼 쓸 수 없습니당!
본 함수 람다에서 무엇을 넘겨줘야할지 구현해줘야해서 위처럼 작성했습니다
실제 onPokeClick, onProfileClick 등을 생성자로 받고있는 하위 컴포넌트들은 클릭 시 id를 넘겨줘야함!과 같은 구체적인 구현사항에 의존하지 않고, 호이스팅 된 상위 컴포넌트에서 위처럼 람다를 구현해서 상위로 전달해줍니다.
람다와 id가 강한결합.. 사실 그렇게 와닿지 않아서 잘 이해가 가진 않아요! 결국 필연적인 로직이라 생각하거든요 :)
There was a problem hiding this comment.
아 코드를 제가 잘못 이해했었네요.
PokeRecommendationDashboard에 userInfo의 id도 같이 넘겨주고 있는 것으로 착각했습니다!
| } | ||
| IconButton( | ||
| onClick = onPokeClick, | ||
| modifier = Modifier.size(size = 44.dp).background( |
There was a problem hiding this comment.
여기서 size를 명시적으로 작성해주신 이유가 있을까요??
iconButton의 디폴트가 44라서 해당 컴포넌트를 사용했다는 것 자체로 사이즈가 44라는 것을 나타내고 있다고 생각해요
There was a problem hiding this comment.
오.. 휴먼 컴포넌트의 defaultSize까지 외우고 계십니까..? ㄷㄷ
처음안 사실이라 내부를 봤는데 44는 아니고 40인 것 같습니다!
| Icon( | ||
| painter = painterResource(ic_poke), | ||
| contentDescription = "콕 찌르기", | ||
| ) |
There was a problem hiding this comment.
혹시 ic_poke가 svg로 설정되어있다면 ImageVector로 바꾸는건 어떨까요??
painter가 Stable하지 않아 리컴포지션의 대상이 된다고 하더라구요
There was a problem hiding this comment.
오 새로 알게된 사실이네요! svg 사용중인 부분 모두 벡터리소스로 바꿨습니다!
What is this issue?
To Reviewer