-
Notifications
You must be signed in to change notification settings - Fork 9
[attendance-compose] 출석 뷰 Compose로 마이그레이션 #1292
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
base: develop
Are you sure you want to change the base?
Conversation
|
|
와우... |
5453d29 to
a2a62ff
Compare
feature/attendance/build.gradle.kts
Outdated
| implementation(projects.domain.attendance) | ||
| implementation(projects.core.common) | ||
| implementation(projects.core.designsystem) | ||
| implementation("androidx.compose.runtime:runtime-livedata:1.5.4") |
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.
?
| fun NavGraphBuilder.attendanceScreen( | ||
| onBackClick: () -> Unit | ||
| ) { | ||
| composable(route = AttendanceRoute) { |
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.
type-safe
|
이거 #990 작업이랑은 별개인건가요?? 완전 새로 작업한건가? |
|
@angryPodo 얍얍....이거 완전히 새롭게 코드 판 짠거 |
Hyobeen-Park
left a comment
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.
리뷰가 너무 늦어져서 죄송합니다😭😭 이렇게 또 하나의 xml 뷰가 사라졌네요!! 정말 감사합니다😊😊
| ) | ||
| } else { | ||
| AttendanceCodeResponse.ERROR | ||
| DomainAttendanceCodeResponse(UNKNOWN_ERROR_CODE) |
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.
기존에 AttendanceodeReponse.ERROR를 사용하던 방식에서 지금과 같이 repositoryImpl 내부에서 UNKNOWN_ERROR_CODE 사용하는 방식으로 수정하신 이유가 궁금해요!
제 생각에는 서버 응답에 따라 subLectureId를 넣어주는 것은 repository의 역할이기 때문에 지금 구조가 더 명확하게 책임이 분리되어 있다고 생각했는데 이 의도로 수정하신게 맞을까요..??
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(Color.Black) | ||
| ) { |
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.
이 Box는 없애도 될 것 같은데 한 번 확인 부탁드려요..!!
| */ | ||
| @Stable | ||
| private class CodeInputState( | ||
| private val keyboardController: androidx.compose.ui.platform.SoftwareKeyboardController? |
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.
여기 import 위로 올려주면 좋을 것 같아요!
| Card( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .clip(RoundedCornerShape(16.dp)), | ||
| colors = CardDefaults.cardColors(containerColor = Gray800) | ||
| ) { |
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.
Card 사용 없이 구현 가능해보이는데 사용하신 이유가 있으실까요?
| } | ||
| }, | ||
| colors = TopAppBarDefaults.topAppBarColors( | ||
| containerColor = androidx.compose.ui.graphics.Color.Black |
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.
여기도 import 위로 올려주시면 감사하겠습니다..!!
| return try { | ||
| attendanceRepository.fetchAttendanceRound(eventId).getOrNull() | ||
| } catch (error: Throwable) { | ||
| null // AttendanceRound 실패는 전체 프로세스에 영향 주지 않음 | ||
| } |
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.
이미 getOrNull 하고 있는데 catch가 실행될 일이 있나요??
| modifier: Modifier = Modifier | ||
| ) { | ||
| Box(modifier = modifier) { | ||
| LazyColumn( |
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.
내부에 item밖에 없는데 LazyColumn 사용하신 이유가 궁금해요!
| init { | ||
| loadAttendanceData() | ||
| } |
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.
LaunchedEffect(Unit) {
viewModel.loadAttendanceData()
}
스크린쪽에서 이렇게 로드해주고 있는데 init에서도 로드하면 처음 진입했을 때 loadAttendanceData()가 두 번 호출되지 않나요??
| */ | ||
| fun loadAttendanceData() { | ||
| viewModelScope.launch { | ||
| _uiState.value = AttendanceUiState.Initial |
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.
혹시 .update를 사용하지 않으신 이유가 있으신가요??
| return suspendRunCatching { | ||
| attendanceRepository.fetchAttendanceRound(eventId) | ||
| }.mapCatching { result -> | ||
| result.getOrThrow() | ||
| }.getOrNull() | ||
| } |
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.
suspendRunCatching을 repository에서 적용해서 한 곳에서만 에러 처리를 하는 것이 더 좋다고 생각했는데 이 부분 뷰모델에서 적용하신 이유가 있으신가요??
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.
| return suspendRunCatching { | |
| attendanceRepository.fetchAttendanceRound(eventId) | |
| }.mapCatching { result -> | |
| result.getOrThrow() | |
| }.getOrNull() | |
| } | |
| return suspendRunCatching { | |
| attendanceRepository.fetchAttendanceRound(eventId).getOrThrow() | |
| }.getOrNull() | |
| } |
이렇게 해도 좋을 것 같아요!
|
헉 @Hyobeen-Park 감사합니당! 이거 리뷰 반영해서 수정할게요 |
2101021 to
80aa1da
Compare
- domain/attendance: 도메인 레이어 모듈 생성 - data/attendance: 데이터 레이어 모듈 생성 - feature/attendance: 프레젠테이션 레이어 모듈 생성 - 각 모듈 gradle 설정 및 의존성 구성
- Entity 클래스들을 domain 모듈로 이동 - Repository 인터페이스를 domain 모듈로 이동 - 패키지 구조 정리 (org.sopt.official.domain.attendance)
- AttendanceService, MockAttendanceService 이동 - AttendanceRepositoryImpl 이동 - Response/Request 모델 클래스들 이동 - DI 모듈을 AttendanceDataModule로 리팩토링
- AttendanceActivity를 Compose 기반으로 재작성 - AttendanceScreen 및 컴포넌트 구현 - XML 레이아웃 제거 및 Compose UI로 대체 - AttendanceViewModel 초기 이동
- AttendanceRepositoryImpl에서 DomainAttendanceCodeResponse 별칭 사용 - UNKNOWN_ERROR_CODE 상수 추가로 매직 넘버 제거 - AttendanceHistoryResponse의 와일드카드 import를 명시적 import로 변경
- try-catch를 suspendRunCatching으로 리팩토링 - AttendanceViewModelRefactored 제거 및 코드 정리 - AttendanceCodeDialog 컴포넌트 분리 및 가독성 개선 - CodeInputState 클래스로 상태 관리 로직 분리 - 각 UI 영역별로 함수 분리 (Header, Title, InputFields 등) - 역할과 책임이 명확한 구조로 개선 - deprecated 메소드 제거
80aa1da to
ac3c0a0
Compare
What is this issue?
프리뷰