-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: 약속 리스트 화면 리팩터링 #973
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
약속 상세 화면 도메인명 DetailMeeting으로 변경하는거 좋네요! |
@haeum808 Meeting -> DetailMeeting / MeetingCatalog -> Meeting으로 변경했습니다! |
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.
고생하셨습니다! 코드가 훨씬 보기 좋아졌네요!
override fun onClickSetting() { | ||
viewModelScope.launch { | ||
_navigateAction.emit(MeetingsNavigateAction.NavigateToSetting) | ||
} |
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.
설정하면으로 갈 때도 _isSelectedFloatingNavigator.emit(false) 해주는건 어떤가요?
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.
오 좋네요! 수정했습니다 9891808 바로 머지할게요!!
🚩 연관 이슈
close #965
📝 작업 내용
onFab()
함수가 어떤 건지 네이밍으로 와닿지 않기도 하고, 데이터 바인딩을 제대로 활용하지 않고 있는 것 같아서 수정했습니다.NavigateAction
을 통한 이동이 혼재되어 있어서, 모든 액티비티 이동이NavigateAction
을 거치도록 수정했습니다.MeetingUiModel
의position
을 통해 item 리스너를 호출했었는데, meeting id로 대체할 수 있을 것 같아 제거했습니다.🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
MeetingsActivity
관련 코드 수정 사항이 몇개 보여서 리팩터링했어요~위에 작업 내용에서
MeetingCatalog
을Meeting
으로 변경했다고 작성했는데, 도메인 객체명은 여전히MeetingCatalog
예요.도메인을
MeetingCatalog
에서Meeting
으로 변경하면, 약속 상세 화면에서 쓰이는 도메인명과 약속 리스트에서 쓰이는 도메인이 둘다Meeting
이 되어버리더라구요.그래서 약속 상세 화면에서 쓰이는 도메인명을
DetailMeeting
으로 변경하고, 약속 리스트 화면에서 쓰이는 도메인명을Meeting
으로 변경하려고 하는데 어떤가요?