-
Notifications
You must be signed in to change notification settings - Fork 5
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
현재 연결된 계정 표시 구현, 로그아웃/회원탈퇴 로직 구현, 캘린더 뷰 구현 #45
Conversation
@@ -14,7 +14,7 @@ buildscript { | |||
kotlinxCoroutinesVersion = "1.6.4" | |||
paging3Version = "3.1.1" | |||
navVersion = "2.5.3" | |||
playServicesAuthVersion = "20.3.0" | |||
playServicesAuthVersion = "20.4.0" |
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.
혹시 버전을 올리신 이유가 있나요??
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.
버전이 업그레이드 됐다고 표시되더라고요! 며칠 전에 릴리즈 된 것 같습니다
@@ -51,6 +66,15 @@ class AccountDataSourceImpl @Inject constructor( | |||
newNickName | |||
} | |||
|
|||
override suspend fun signOut(): Result<Boolean> = runCatching { | |||
signOut() |
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.
헉.. 다른 코드가 안보여서 그런건지... 무한 재귀가 되는 것으로 보이는데 혹시 어떻게 동작하나요?
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.
오.. 그렇네요?
val totalRunningTime: Int, | ||
val pace: Double, | ||
val totalDistance: Double | ||
@ColumnInfo(name = "started_at") val startedAt: Long, |
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.
이건 스타일이겠지만 저는 PrimaryKey 어노테이션처럼 ColumnInfo 어노테이션도 변수 위에 붙여줬어요!
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.
이거는 kt lint를 돌렸을 때 위 코드처럼 됐던 것 같아요!
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.
네 저도 ktlint 돌렸습니다!
class CalendarDayBinder( | ||
private val calendarView: CalendarView | ||
) : DayBinder<CalendarDayBinder.DayContainer> { | ||
private var calendar: Pair<LocalDate?, LocalDate?> = null to null |
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.
Pair 대신 간단한 데이터 클래스를 만드는 것은 어떨까요??
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.
동의합니다!
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.
정확히 무슨 뜻인지 파악하고 데이터 클래스로 만들어 보겠습니다!
viewModel.updateNickName(viewModel.uid.value, dialogView.findViewById<EditText>(R.id.et_change_nick_name).text.toString()) | ||
viewModel.updateNickName( | ||
viewModel.uid.value, | ||
dialogView.findViewById<EditText>(R.id.et_change_nick_name).text.toString() |
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.
바인딩을 안 쓰신 이유가 있나요?
@@ -26,4 +29,10 @@ interface AccountRepository { | |||
|
|||
// 프로필 사진 서버에 업데이트 | |||
suspend fun updateProfileUrl(newProfileUrl: String): 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.
이친구만 Result가 아닌 게 궁금합니다!! updateNickname은 반환값이 있고 얘는 반환값이 없나요?!
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.
엇 지금 프로필 사진 업데이트 기능은 나중에 구현하기로 해서 지금 안쓰는 함수입니다! 예전에 다같이 작성했던 코드에요
@@ -23,6 +25,24 @@ class MyRunningHistoryAdapter : | |||
override fun onBindViewHolder(holder: RunningHistoryViewHolder, position: Int) { | |||
holder.bind(getItem(position)) | |||
} | |||
|
|||
companion object { | |||
class MyRunningHistoryDiffCallback : DiffUtil.ItemCallback<RunningHistory>() { |
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.
RunningHistoryUiModel을 쓰기로 했던 것 같아서 남겨봅니다...!
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.
UiModel을 쓰면 RunningHistory에 대한 id값을 몰라서 diffutil을 사용할 수 없지 않나 해서 RunningHistory를 썼습니다..!
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.
uiModel에 id를 넣는 방향으로 수정
private val firstMonth = currentMonth.minusMonths(5) | ||
private val lastMonth = currentMonth.plusMonths(5) |
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.
상수는 임시로 넣으신건가요!
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.
넹 테스트용이고 다같이 의논해봐야될 사항인 것 같아요! 저 숫자로 인해 달력의 범위가 결정됩니다!
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.
10년 단위인 120으로 결정
private val fireBaseDb: FirebaseFirestore | ||
) : AccountDataSource { | ||
|
||
private val auth = FirebaseAuth.getInstance() | ||
private val currentUser = auth.currentUser | ||
|
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.
파이어 베이스를 모듈안에서 만들어주기 보다는 di를 통해 주입해보는건 어떻게 생각하시나요?
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.
넵 동의합니다!
val totalRunningTime: Int, | ||
val pace: Double, | ||
val totalDistance: Double | ||
@ColumnInfo(name = "started_at") val startedAt: Long, |
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.
이거는 kt lint를 돌렸을 때 위 코드처럼 됐던 것 같아요!
@ColumnInfo(name = "finished_at") val finishedAt: Long, | ||
@ColumnInfo(name = "total_running_time") val totalRunningTime: Int, | ||
@ColumnInfo(name = "pace") val pace: Double, | ||
@ColumnInfo(name = "total_distance") val totalDistance: Double | ||
) |
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.
컬럼 네임 작성해주신거 좋았습니다!
class RunningHistoryLocalDataSourceImpl @Inject constructor( | ||
private val roomDb: RunningHistoryLocalDataBase | ||
private val runningHistoryDao: RunningHistoryDao |
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.
피드백 적용해주신거 좋았습니다!
class CalendarDayBinder( | ||
private val calendarView: CalendarView | ||
) : DayBinder<CalendarDayBinder.DayContainer> { | ||
private var calendar: Pair<LocalDate?, LocalDate?> = null to null |
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.
동의합니다!
R.id.my_run_setting -> { | ||
val direction = MyRunFragmentDirections.actionMyRunFragmentToSettingFragment() | ||
findNavController().navigate(direction) | ||
} | ||
} | ||
true | ||
} | ||
|
||
calendarView.apply { | ||
itemAnimator = null | ||
dayBinder = CalendarDayBinder(this) | ||
monthScrollListener = { calendarMonth -> | ||
onMonthScrolled(calendarMonth.yearMonth) | ||
} | ||
// 모든 달력 범위 설정 | ||
setup(firstMonth, lastMonth, firstDayOfWeek) | ||
// 첫 화면에서 보일 달 설정 | ||
scrollToMonth(currentMonth) | ||
} |
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.
컨벤션으로 합의했던 initViews(), observeState() 적용이 안되어있습니다!
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.
다음 커밋에 반영하겠습니다 ㅠㅠ
class SettingViewModel @Inject constructor( | ||
private val getEmailUseCase: GetEmailUseCase, | ||
private val signOutUseCase: SignOutUseCase | ||
) : ViewModel() { | ||
|
||
init { | ||
getEmail() | ||
} | ||
|
||
private val _emailState = MutableStateFlow<UiState<String>>(UiState.UnInitialized) | ||
val emailState: StateFlow<UiState<String>> | ||
get() = _emailState.asStateFlow() | ||
|
||
private fun getEmail() { | ||
viewModelScope.launch { | ||
_emailState.value = UiState.Loading | ||
|
||
getEmailUseCase().collect { emailResult -> | ||
emailResult.onSuccess { email -> | ||
_emailState.value = UiState.Success(email) | ||
}.onFailure { throwable -> | ||
_emailState.value = UiState.Failure(throwable) | ||
} | ||
} | ||
} | ||
} | ||
|
||
fun signOut() { | ||
viewModelScope.launch { | ||
signOutUseCase.invoke() | ||
} | ||
} | ||
} |
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.
UiState 적용해주신거 좋았습니다!
signOut도 eventFlow를 활용한 패턴으로 적용해보면 어떨까요? ㅎㅎ
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.
넵 ㅎㅎ 이번주에 구현이 빨리 끝나면 리팩토링 때 해보겠습니다
😎 작업 내용
🧐 변경된 내용
🥳 동작 화면
🤯 이슈 번호