-
Notifications
You must be signed in to change notification settings - Fork 1
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
[UI/YAF-74] 알람 설정 시 알람 미루기를 설정할 수 있다 #46
Conversation
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.
수고하셨습니다.
컴포넌트 단위로 State처리한거에 대한 생각:
지금은 기능단위로 처리된걸 Context
단위로도 분리할 수 있을 것 같다?
예를 들어서 알람 화면 전체에 관련된 상태를 하나의 AlarmState로 통합하고,
필요시 하위 컴포넌트에서 일부 상태를 가져다 쓰는 방식으로 설계할 수도 있을 것 같습니다.
is AlarmAddEditContract.Action.ToggleWeekendsChecked -> toggleWeekendsChecked() | ||
is AlarmAddEditContract.Action.ToggleDaySelection -> toggleDaySelection(action.day) | ||
is AlarmAddEditContract.Action.ToggleDisableHolidayChecked -> toggleDisableHolidayChecked() | ||
viewModelScope.launch { |
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.
p5
로컬 디비에 담아야되서 viewmodelscope.launch 넣으신 거죠??
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.
의도한 것은 아닌데 Copilot 쓰다가 저렇게 됐네요....
나중에 DB 연동 작업 시에 알맞게 수정할게요
import javax.inject.Inject | ||
|
||
@HiltViewModel | ||
class AlarmAddEditViewModel @Inject constructor() : BaseViewModel<AlarmAddEditContract.State, AlarmAddEditContract.SideEffect>( | ||
initialState = AlarmAddEditContract.State(), | ||
) { | ||
private var debounceJob: Job? = null // 디바운싱을 위한 Job |
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.
p4
Job을 사용하신 이유가 궁금합니다이
mutableStateFlow로 하면 아마 스트림 연산으로 dobounce를 처리할 수 있을거 같아서
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.
요거 디바운싱 처리했다가 알라미에서는 디바운싱 안했길래 제거했어요
} | ||
} | ||
|
||
private fun updateAlarmMessage() { |
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.
p3
시간계산 쪽은 뭔가 domain layer에서 처리해야될 것 같은 아이들이 있는거 같아요.
아니면 유틸함수로 빼도 될듯?
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.
시간 계산이 위에 n시간 n분 남았습니다
를 말씀하시는건가요?
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 duration = java.time.Duration.between(now, nextAlarmDateTime)
val totalMinutes = duration.toMinutes()
val days = totalMinutes / (24 * 60)
val hours = (totalMinutes % (24 * 60)) / 60
val minutes = totalMinutes % 60
// 출력 문구 생성
val newMessage = when {
days > 0 -> "${days}일 ${hours}시간 후에 울려요"
hours > 0 -> "${hours}시간 ${minutes}분 후에 울려요"
else -> "${minutes}분 후에 울려요"
}
this area.
사용하는지는 잘 모르지만 유틸이나 도메인으로 분리하면 다른 뷰에서도 참조 가능하고 우리가 나중에 테스트 코드를 짜게 된다면..? 더 이점이 있을듯 ㅋㅋㅋ
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.
이부분은 재사용하는 로직이 아니라서 따로 domain으로 안 빼둘게요
private fun convertTo24HourFormat(amPm: String, hour: Int): Int { | ||
return if (amPm == "오후" && hour != 12) { | ||
hour + 12 | ||
} else if (amPm == "오전" && hour == 12) { | ||
0 | ||
} else { | ||
hour | ||
} | ||
} |
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.
p3
private fun convertTo24HourFormat(amPm: String, hour: Int): Int = when {
amPm == "오후" && hour != 12 -> hour + 12
amPm == "오전" && hour == 12 -> 0
else -> hour
}
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.
p5
어우 👍👍👍
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.
호우
} | ||
} | ||
|
||
private fun updateAlarmMessage() { |
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 duration = java.time.Duration.between(now, nextAlarmDateTime)
val totalMinutes = duration.toMinutes()
val days = totalMinutes / (24 * 60)
val hours = (totalMinutes % (24 * 60)) / 60
val minutes = totalMinutes % 60
// 출력 문구 생성
val newMessage = when {
days > 0 -> "${days}일 ${hours}시간 후에 울려요"
hours > 0 -> "${hours}시간 ${minutes}분 후에 울려요"
else -> "${minutes}분 후에 울려요"
}
this area.
사용하는지는 잘 모르지만 유틸이나 도메인으로 분리하면 다른 뷰에서도 참조 가능하고 우리가 나중에 테스트 코드를 짜게 된다면..? 더 이점이 있을듯 ㅋㅋㅋ
Related issue 🛠
closed #45
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
default.mp4
Uncompleted Tasks 😅
To Reviewers 📢
화면에 표시할 정보가 너무 많아서 컴포넌트 단위로 상태 정의해서 관리하는데
나중에 다른 화면에도 써먹을 수 있을 것 같아요
이거보다 더 좋은 방식 있을 것 같음 코멘트 달아주세용