-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR/#402] 유빈 뷰 / 리팩토링 #405
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
Conversation
boiledeggg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 좋습니다! 따따따봉👍
오랜만에 터닝에 리뷰다니까 좋네ㅋㅎ
| @Composable | ||
| fun ProfileWithPlusButton( | ||
| profileImage: String, | ||
| onClick: (Boolean) -> Unit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 컴포넌트를 사용하는 곳들에서 람다 파라미터로 주어지는 Boolean 값을 사용하지 않는 것 같던데 확인 한번 부탁드려요~
사용되지 않는 파라미터는 없애는게 좋을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예리하시군요!! 저도 코드를 안 본지 오래돼서... ㅎㅎ 불린 값을 사용 안 하고 있을 거라 생각하고 찾아봤는데, 여기서 true로 넘겨준 값을 각 스크린의 뷰모델로 넘겨서 바텀시트의 가시 여부를 결정하고 있더라구요! 그래서 이대로 둬도 괜찮을 것 같아요:)
확인 감사합니당👍
| Image( | ||
| painter = painterResource(R.drawable.ic_my_page_go_detail), | ||
| contentDescription = "go detail" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 painter를 인자로 받는 Image를 사용하신 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imageVector를 사용하는 방법도 있다는 말씀이시죠?!
사실.. 이전 코드들이 painter를 사용하고 있어서 그대로 사용했습니다 ..ㅎㅎ 수정해놓을게요!!
⛳️ Work Description
📸 Screenshot
📢 To Reviewers