[AN] 마이페이지 수정 및 보기 기능 개선 #602
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
no1msh
approved these changes
Nov 3, 2023
Comment on lines
40
to
43
| class MyPageFragment : Fragment() { | ||
| private val myPageViewModel: MyPageViewModel by viewModels() | ||
| private var _binding: FragmentMyPageBinding? = null | ||
| private val binding: FragmentMyPageBinding get() = _binding!! |
There was a problem hiding this comment.
오래동안 안써서 까먹을 법도 한데 세심하게 잘 처리해주셨네요!
Comment on lines
183
to
191
| private fun setEditTextChangeListener() { | ||
| binding.etMyPageProfileNickname.filters = myPageViewModel.getInputFilter() | ||
| binding.etMyPageProfileNickname.doOnTextChanged { text, start, before, count -> | ||
| myPageViewModel.setNickname(text.toString()) | ||
| } | ||
| binding.etMyPageProfileIntroduction.doOnTextChanged { text, _, _, _ -> | ||
| myPageViewModel.setIntroduction(text.toString()) | ||
| } | ||
| } |
There was a problem hiding this comment.
함수 파라미터가 없기 때문에 prefix를 set 대신 setup으로 변경하시는게 좋을 것 같네요
Comment on lines
239
to
243
| sealed interface State { | ||
| object SUCCESS : State | ||
| object FAIL : State | ||
| object IDLE : State | ||
| } | ||
|
|
||
| private fun Profile.toUiModel(): ProfileUiModel = ProfileUiModel( | ||
| githubId = githubId, | ||
| id = id, | ||
| profileInformation = profileInformation.toUiModel(), | ||
| profileImageUrl = profileImageUrl, | ||
| successRate = successRate, | ||
| successfulRoundCount = successfulRoundCount, | ||
| tier = tier, | ||
| tierProgress = tierProgress, | ||
| ) | ||
|
|
||
| private fun ProfileInformation.toUiModel(): ProfileInformationUiModel = | ||
| ProfileInformationUiModel( | ||
| nickname = nickname.toUiModel(), | ||
| introduction = introduction, | ||
| ) | ||
|
|
||
| private fun Nickname.toUiModel(): NicknameUiModel = NicknameUiModel( | ||
| nickname = nickname, | ||
| ) | ||
|
|
||
| private fun ProfileUiModel.toDomain(): Profile = Profile( | ||
| githubId = githubId, | ||
| id = id, | ||
| profileInformation = profileInformation.toDomain(), | ||
| profileImageUrl = profileImageUrl, | ||
| successRate = successRate, | ||
| successfulRoundCount = successfulRoundCount, | ||
| tier = tier, | ||
| tierProgress = tierProgress, | ||
| ) | ||
|
|
||
| private fun ProfileInformationUiModel.toDomain(): ProfileInformation = | ||
| ProfileInformation( | ||
| nickname = nickname.toDomain(), | ||
| introduction = introduction, | ||
| ) | ||
| object Loading : State | ||
| object Success : State | ||
| object Fail : State | ||
| } |
There was a problem hiding this comment.
현재 상태는 Enum Class로 완벽하게 대체할 수 있을 것 같습니다.
조금 더 해당 부분을 고도화하고 싶다면,
혹시 이 포스트의 2번째 방법처럼 리팩터링 해보는 거 어떨까요?
써니의 의견이 궁금합니다. 제가 놓치는 점이 있다면 언제든 말씀주세요!
| @@ -51,7 +51,7 @@ | |||
| <string name="hom_study_list_must_do_default_message">Must Do를 아직 등록하지 않았어요!</string> | |||
| <string name="home_study_list_guide">Must Do를 인증하면 잔디씨앗을 드려요! \n스터디를 완주하면 잔디를 심어드려요!</string> | |||
| <string name="home_no_study">참여중인 스터디가 없어요!\n스터디를 개설/참여 해볼까요?</string> | |||
| <string name="home_study_list_date">D - %d</string> | |||
| <string name="home_study_list_date">D - %d</string>수 | |||
There was a problem hiding this comment.
Suggested change
| <string name="home_study_list_date">D - %d</string>수 | |
| <string name="home_study_list_date">D - %d</string> |
아마 IDE의 한글관련 버그 때문에 들어간거 같네요! 그럴 수 있습니다 😆
s9hn
approved these changes
Nov 3, 2023
There was a problem hiding this comment.
많은 collect와 HotFlow들로 인해 두 클래스의 무게가 상당하다고 생각합니다.
하나의 state로 통합하여 관리해보는건 어떤가요?
제 생각엔 useCase를 통한 비즈니스로직 분리와 하나의 uiState로 통합하여 관리하면 많은 보일러플레이트 코드가 개선될 것이라고 생각합니다.
당장 급한 피알이 아니라면, 리팩터링 후 머지하는것도 좋아보입니다.
적용에 어려움이 있다면 언제든지 연락주세요 페어도 환영입니다!
뷰바인딩 마이그레이션을 위해 baseClass를 만들어두었습니다.
dataBinding을 쓰게되면 결국 뷰의 관심사는 또 흩어지게 됩니다. 어떤경우에 데바가 필연적인가요?
| import dagger.hilt.android.AndroidEntryPoint | ||
|
|
||
| @AndroidEntryPoint | ||
| class MyPageFragment : BindingFragment<FragmentMyPageBinding>(R.layout.fragment_my_page) { | ||
| class MyPageFragment : Fragment() { |
There was a problem hiding this comment.
새로운 ViewBinding BaseClass 만들어두었습니다! HomeFragement참고
|
변경점
|
2023-11-09.3.13.09.mov |
s9hn
approved these changes
Nov 9, 2023
| with(binding.tvMyPageNicknameValidateIntroduction) { | ||
| text = getString(state.introduction) | ||
| setTextColor(requireContext().getColor(state.color)) | ||
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
😋 작업한 내용
🙏 PR Point
👍 관련 이슈