[AN/USER] 홈 화면 회전시 이전에 선택한 화면을 그대로 보여준다. (#534) #544
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
re4rk
requested changes
Oct 15, 2023
Comment on lines
+58
to
+61
| when (event) { | ||
| is SignInEvent.SignInSuccess -> handleSuccessEvent() | ||
| is SignInEvent.SignInFailure -> handleFailureEvent() | ||
| } |
There was a problem hiding this comment.
저는 지금도 충분히 깔끔하다고 생각합니다ㅎㅎ uistate, event 둘다 있다면 모를까 지금은 분리하면 오히려 가독성이 떨어질 것 같아요!
| vm.event.collect { | ||
| handleEvent(it) | ||
| private fun initBackPressed() { | ||
| val callback = object : OnBackPressedCallback(true) { |
| } | ||
| } | ||
|
|
||
| private fun selectItemWhenSignIn(homeItemType: HomeItemType) { |
There was a problem hiding this comment.
함수명이 조금 어색하게 느껴진다고 생각하는데 베르의 의견은 어떤가요??
selectItemOrSignIn와 같이 다른 함수들과 비슷하게 진행하는건 어떤가요??
Comment on lines
35
to
38
| if (!authRepository.isSigned) { | ||
| _event.emit(HomeEvent.ShowSignIn) | ||
| } else { | ||
| _selectedItem.emit(homeItemType) |
There was a problem hiding this comment.
Suggested change
| if (!authRepository.isSigned) { | |
| _event.emit(HomeEvent.ShowSignIn) | |
| } else { | |
| _selectedItem.emit(homeItemType) | |
| if (authRepository.isSigned) { | |
| _selectedItem.emit(homeItemType) | |
| } else { | |
| _event.emit(HomeEvent.ShowSignIn) |
흐름상 !을 빼도 괜찮을 것같은데 어떻게 생각하시나요??
EmilyCh0
approved these changes
Oct 16, 2023
Comment on lines
+58
to
+61
| when (event) { | ||
| is SignInEvent.SignInSuccess -> handleSuccessEvent() | ||
| is SignInEvent.SignInFailure -> handleFailureEvent() | ||
| } |
There was a problem hiding this comment.
저는 지금도 충분히 깔끔하다고 생각합니다ㅎㅎ uistate, event 둘다 있다면 모를까 지금은 분리하면 오히려 가독성이 떨어질 것 같아요!
| @@ -38,85 +38,104 @@ class HomeViewModelTest { | |||
| } | |||
|
|
|||
| @Test | |||
| fun `축제 목록을 요청했을 때 토큰이 있으면 축제 목록이 보인다`() = runTest { | |||
| fun `축제 목록을 요청했을 때 토큰이 있으면 축제 목록이 보이고 이벤트가 발생하지 않는다`() = runTest { | |||
There was a problem hiding this comment.
축제 목록 요청시에는 사용자 인증 유무 확인이 필요 없는 거 아닌가요?_?
There was a problem hiding this comment.
필요없다는 것도 테스트로 확인하고 싶었어요!
| assertThat(vm.selectedItem.value).isEqualTo(HomeItemType.FESTIVAL_LIST) | ||
|
|
||
| // and | ||
| expectNoEvents() |
| } | ||
| } | ||
|
|
||
| @Test | ||
| fun `마이페이지를 요청했을 때 토큰이 있으면 마이페이지 보기 이벤트가 발생한다`() = runTest { | ||
| fun `마이페이지를 요청했을 때 토큰이 있으면 마이페이지가 보이고 이벤트가 발생하지 않는다`() = runTest { |
There was a problem hiding this comment.
테스트 이름과 내용이 일치하지 않아요..! 마이페이지 테스트인데 TICKET_LIST를 테스트하고 있습니다ㅋㅋ
BGuga
pushed a commit
that referenced
this pull request
Oct 17, 2023
* refactor: show 이벤트 제거 * refactor: 홈 화면 회전시 축제리스트로 되돌아가는 문제 해결 * refactor: 필요없는 이벤트 제거 * feat: 로그인하지 않고 돌아가면 축제리스트 화면으로 돌아간다 * refactor: 홈화면 로그인화면 테스트 리팩터링 * refactor: 필요 없는 코드 제거 * refactor: 테스트 함수명 의미에 맞게 변경 * refactor: selectItemOrSignIn 함수명 변경 * refactor: 함수 순서 변경 * fix: 잘못된 테스트 코드 내용 수정 * refactor: laitInit 를 by lazy 로 변경
BGuga
pushed a commit
that referenced
this pull request
Oct 17, 2023
* refactor: show 이벤트 제거 * refactor: 홈 화면 회전시 축제리스트로 되돌아가는 문제 해결 * refactor: 필요없는 이벤트 제거 * feat: 로그인하지 않고 돌아가면 축제리스트 화면으로 돌아간다 * refactor: 홈화면 로그인화면 테스트 리팩터링 * refactor: 필요 없는 코드 제거 * refactor: 테스트 함수명 의미에 맞게 변경 * refactor: selectItemOrSignIn 함수명 변경 * refactor: 함수 순서 변경 * fix: 잘못된 테스트 코드 내용 수정 * refactor: laitInit 를 by lazy 로 변경
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 세부 내용
홈화면에서 회전에도 이전에 선택한 화면을 기억
티켓 리스트, 마이페이지 로그인하지 않았을 경우 이벤트 발생
마이페이지 ScrollView 적용 필요