🔀 :: (#218) - 디자인 변동사항을 따릅니다. #219
Conversation
Walkthrough이 변경 사항은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (20)
core/model/src/main/java/com/school_of_company/model/model/training/TrainingDtoModel.kt (1)
6-6: 속성에 대한 문서화 추가를 제안합니다.
endedAt속성의 의미와 예상되는 날짜 형식을 명확히 하기 위해 KDoc 문서화를 추가하는 것이 좋겠습니다.data class TrainingDtoModel( val title: String, val startedAt: String, + /** 트레이닝이 종료된 날짜 (ISO 8601 형식: yyyy-MM-dd'T'HH:mm:ss.SSS'Z') */ val endedAt: String, val category: String, )core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingDtoResponseMapper.kt (1)
7-12: 매핑 함수에 대한 문서화를 추가하는 것이 좋겠습니다.매핑 로직의 목적과 변환 규칙을 명확히 하기 위해 KDoc 문서를 추가하는 것을 제안합니다.
다음과 같이 문서를 추가해보세요:
+/** + * TrainingDto를 TrainingDtoModel로 변환합니다. + * + * @return 변환된 TrainingDtoModel 인스턴스 + */ fun TrainingDto.toModel(): TrainingDtoModel = TrainingDtoModel( title = this.title, startedAt = this.startedAt, endedAt = this.endedAt, category = this.category )feature/expo/src/main/java/com/school_of_company/expo/view/component/TrainingSettingButton.kt (2)
18-43: 컴포넌트의 접근성과 재사용성 개선이 필요합니다.현재 구현은 기본적인 기능을 잘 수행하고 있지만, 다음과 같은 개선사항을 제안드립니다:
- 접근성을 위한 contentDescription 추가
- 버튼의 크기 제한이 없어 일관성 있는 UI를 위해 기본 크기 지정 필요
- 비활성화 상태 지원 추가
다음과 같이 수정하는 것을 제안드립니다:
@Composable fun TrainingSettingButton( modifier: Modifier = Modifier, text: String, + enabled: Boolean = true, + contentDescription: String? = null, onClick: () -> Unit ) { ExpoAndroidTheme { colors, typography -> Box( modifier = modifier + .defaultMinSize(minWidth = 80.dp, minHeight = 36.dp) .border( width = 1.dp, - color = colors.gray200, + color = if (enabled) colors.gray200 else colors.gray100, shape = RoundedCornerShape(size = 6.dp) ) .padding(all = 8.dp) - .expoClickable(onClick = onClick) + .expoClickable( + onClick = onClick, + enabled = enabled, + role = Role.Button + ), + contentDescription = contentDescription ) { Text( text = text, style = typography.captionRegular2, - color = colors.gray300, + color = if (enabled) colors.gray300 else colors.gray200, ) } } }
45-49: 프리뷰 함수 개선이 필요합니다.프리뷰 함수가 다양한 상태를 보여주지 않아 컴포넌트의 모든 상태를 확인하기 어렵습니다.
다음과 같이 수정하는 것을 제안드립니다:
-@Preview -@Composable -private fun TrainingSettingButtonPreview() { - TrainingSettingButton(text = "수정하기") {} -} +@Preview(name = "기본 상태") +@Composable +private fun TrainingSettingButtonPreview() { + ExpoAndroidTheme { _, _ -> + TrainingSettingButton( + text = "수정하기", + onClick = {} + ) + } +} + +@Preview(name = "비활성화 상태") +@Composable +private fun DisabledTrainingSettingButtonPreview() { + ExpoAndroidTheme { _, _ -> + TrainingSettingButton( + text = "수정하기", + enabled = false, + onClick = {} + ) + } +}feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoNoneLineTextField.kt (3)
13-13: 불필요한 import 제거 필요
TextFieldimport가 코드에서 사용되지 않고 있습니다. 사용하지 않는 import는 제거하는 것이 좋습니다.-import androidx.compose.material3.TextField
26-38: KDoc 문서화 추가 제안함수의 목적과 각 매개변수의 역할을 명확히 하기 위해 KDoc 문서화를 추가하는 것이 좋습니다.
다음과 같은 문서화를 추가해보세요:
+/** + * 밑줄이 없는 커스텀 텍스트 필드를 생성합니다. + * + * @param modifier 컴포저블의 수정자 + * @param textState 현재 텍스트 상태 + * @param placeHolder 텍스트가 비어있을 때 표시될 플레이스홀더 + * @param singleLine 한 줄 입력 여부 + * @param maxLines 최대 줄 수 + * @param minLines 최소 줄 수 + * @param visualTransformation 텍스트 변환 설정 + * @param keyboardOptions 키보드 옵션 + * @param keyboardActions 키보드 액션 + * @param onTextChange 텍스트 변경 시 호출될 콜백 + */
76-90: 프리뷰 기능 개선 제안현재 프리뷰는 기본적인 사용 사례만 보여주고 있습니다. 다양한 상태와 테마에서의 모습을 확인할 수 있도록 프리뷰를 확장하면 좋겠습니다.
다음과 같은 프리뷰들을 추가해보세요:
@Preview(name = "다크 테마") @Composable private fun ExpoNoneLineTextFieldDarkPreview() { ExpoAndroidTheme(darkTheme = true) { ExpoNoneLineTextField( textState = "샘플 텍스트", placeHolder = { Text( text = "yyyy-MM-dd HH:mm", style = ExpoTypography.titleBold2, color = ExpoColor.gray300 ) }, onTextChange = {} ) } } @Preview(name = "텍스트 입력 상태") @Composable private fun ExpoNoneLineTextFieldWithTextPreview() { ExpoNoneLineTextField( textState = "2024-01-01 12:00", placeHolder = { Text( text = "yyyy-MM-dd HH:mm", style = ExpoTypography.titleBold2, color = ExpoColor.gray300 ) }, onTextChange = {} ) }feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoAddTextField.kt (1)
130-141: Preview 함수를 개선해주세요.현재 Preview 함수에 다음과 같은 개선사항이 필요합니다:
- TODO 주석 대신 실제 동작하는 코드 구현
- 다양한 상태를 보여주는 추가 Preview 함수 구현 (예: 여러 개의 텍스트 필드가 있는 상태)
다음과 같이 개선해보세요:
@Preview +@Preview(name = "Empty State") @Composable private fun ExpoAddTextFieldPreview() { ExpoAddTextField( placeHolder = "연수를 입력하세요.", trainingTextFieldList = listOf(""), - onValueChange = { _, _ ->}, - onAddTextField = { /*TODO*/ }, - onRemoveTextField = { /*TODO*/ }, - onTrainingSetting = { /*TODO*/ } + onValueChange = { _, _ -> }, + onAddTextField = {}, + onRemoveTextField = {}, + onTrainingSetting = {} ) } + +@Preview(name = "Multiple Items") +@Composable +private fun ExpoAddTextFieldWithItemsPreview() { + ExpoAddTextField( + placeHolder = "연수를 입력하세요.", + trainingTextFieldList = listOf("연수 1", "연수 2", "연수 3"), + onValueChange = { _, _ -> }, + onAddTextField = {}, + onRemoveTextField = {}, + onTrainingSetting = {} + ) +}core/design-system/src/main/java/com/school_of_company/design_system/icon/ExpoIcon.kt (1)
Line range hint
1-300: 아이콘 컴포넌트들의 일관성 개선이 필요합니다.현재 일부 아이콘 컴포넌트들만 tint 매개변수를 지원하고 있습니다. 디자인 시스템의 일관성을 위해 모든 아이콘 컴포넌트에 tint 지원을 추가하는 것이 좋을 것 같습니다.
다음 아이콘들에 tint 매개변수 추가가 필요합니다:
- RightArrowIcon
- UpArrowIcon
- TrashIcon
- UserIcon
- WonderIcon
- CopyIcon
- BellIcon
- LogoutIcon
- SettingIcon
- CheckBoxIcon
- DropDownIcon
- TwoCircleIcon
- LongContentIcon
- ShortContentIcon
- HomeIcon
- AppendIcon
- ProgramIcon
예시 리팩토링:
@Composable -fun RightArrowIcon(modifier: Modifier = Modifier) { +fun RightArrowIcon( + modifier: Modifier = Modifier, + tint: Color = Color.Unspecified +) { Icon( painter = painterResource(id = R.drawable.ic_right_arrow), contentDescription = stringResource(id = R.string.right_arrow_description), - modifier = modifier + modifier = modifier, + tint = tint ) }feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoTrainingSettingBottomSheet.kt (3)
51-62: 공개 API에 대한 KDoc 문서화가 필요합니다.이 컴포저블은 공개 API이므로, 각 매개변수의 용도와 동작을 설명하는 문서화가 필요합니다.
다음과 같은 KDoc 문서를 추가하는 것을 제안합니다:
+/** + * 연수 설정을 위한 바텀 시트 컴포저블 + * + * @param modifier 컴포저블의 수정자 + * @param onCancelClick 취소 버튼 클릭 시 호출될 콜백 + * @param startedTextState 시작 시간 텍스트 상태 + * @param endedTextState 종료 시간 텍스트 상태 + * @param onStartedTextChange 시작 시간 변경 시 호출될 콜백 + * @param onEndedTextChange 종료 시간 변경 시 호출될 콜백 + * @param onButtonClick 확인 버튼 클릭 시 호출될 콜백 + * @param categoryState 현재 선택된 카테고리 상태 + * @param onCategoryChange 카테고리 변경 시 호출될 콜백 + * @param focusManager 포커스 관리자 + */
104-104: 문자열 리소스 추출이 필요합니다.하드코딩된 문자열들을 리소스로 추출하여 다국어 지원 및 유지보수성을 개선해야 합니다.
다음 문자열들을 리소스로 추출하는 것을 제안합니다:
- "연수 설정"
- "필수"
- "확인"
Also applies to: 151-151, 166-166
220-243: 프리뷰 함수의 다양한 상태 테스트가 필요합니다.현재 프리뷰 함수들은 단일 상태만 보여주고 있습니다. 다양한 상태에서의 UI를 확인할 수 있도록 개선이 필요합니다.
다음과 같은 추가 프리뷰 함수들을 제안합니다:
@Preview(name = "빈 입력 상태") @Composable private fun EmptyStatePreview() { ExpoTrainingSettingBottomSheet( onCancelClick = {}, startedTextState = "", endedTextState = "", onStartedTextChange = {}, onEndedTextChange = {}, onButtonClick = {}, categoryState = TrainingCategory.CHOICE, onCategoryChange = {} ) } @Preview(name = "입력 완료 상태") @Composable private fun FilledStatePreview() { ExpoTrainingSettingBottomSheet( onCancelClick = {}, startedTextState = "2024-11-20 09:00", endedTextState = "2024-11-20 18:00", onStartedTextChange = {}, onEndedTextChange = {}, onButtonClick = {}, categoryState = TrainingCategory.ESSENTIAL, onCategoryChange = {} ) }feature/expo/src/main/java/com/school_of_company/expo/viewmodel/ExpoViewModel.kt (1)
318-334: 상태 업데이트 성능 최적화가 필요합니다.각 함수에서 toMutableList()를 호출하여 새로운 리스트를 생성하는 것은 불필요한 객체 생성을 유발할 수 있습니다. 다음과 같이 최적화하는 것을 추천드립니다:
- internal fun updateTrainingProgramText(index: Int, newText: String) { - _trainingProgramTextState.value = _trainingProgramTextState.value.toMutableList().apply { - set(index, newText) - } - } + internal fun updateTrainingProgramText(index: Int, newText: String) { + val currentList = _trainingProgramTextState.value + _trainingProgramTextState.value = buildList(currentList.size) { + addAll(currentList) + set(index, newText) + } + }feature/expo/src/main/java/com/school_of_company/expo/view/ExpoDetailScreen.kt (3)
Line range hint
383-407: 다이얼로그 구현 개선 제안다이얼로그 구현에서 다음과 같은 개선사항을 제안드립니다:
- 완전한 경로명(fully qualified path) 대신 import 문을 사용하면 코드가 더 깔끔해질 것 같습니다.
- 다이얼로그 상태 관리를 더 간결하게 할 수 있습니다.
+import com.school_of_company.expo.view.component.MessageDialog +import com.school_of_company.expo.view.component.QrDialog - com.school_of_company.expo.view.component.MessageDialog( + MessageDialog( - com.school_of_company.expo.view.component.QrDialog( + QrDialog(
Line range hint
389-396: SMS 로직 구현 필요SMS 로직이 아직 구현되지 않았습니다. 참가자와 연수생을 위한 SMS 기능이 필요해 보입니다.
이 기능 구현을 위한 이슈를 생성하고 구현 방안을 제안드릴까요?
Line range hint
293-321: 에러 처리 개선 제안네트워크 오류 발생 시 사용자가 다시 시도할 수 있는 방법이 없습니다. 재시도 버튼을 추가하면 사용자 경험이 개선될 것 같습니다.
다음과 같은 개선을 제안드립니다:
Text( text = "네트워크가 불안정해요..", style = typography.bodyRegular2, color = colors.gray400 ) + ExpoButton( + text = "다시 시도", + color = colors.main, + onClick = { viewModel.getExpoInformation(expoId = id) } + )feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt (3)
423-423:placeholder매개변수 철자 오류 수정 필요
ExpoAddTextField컴포넌트에서placeHolder로 표기되어 있습니다. 일반적으로placeholder로 소문자h를 사용하여 표기합니다.수정된 코드는 다음과 같습니다:
- placeHolder = "연수 종류를 입력해주세요." + placeholder = "연수 종류를 입력해주세요."Also applies to: 443-443
481-482: 생성하기 버튼 활성화 조건 개선 제안현재 버튼 활성화 조건에서
trainingProgramTextState와standardProgramTextState가 빈 리스트가 아닌지만 확인하고 있습니다. 리스트 항목들이 실제로 내용이 있는지 확인하여 사용자의 의도치 않은 빈 입력을 방지하는 것이 좋습니다.예를 들어, 리스트의 모든 항목이 비어있지 않은지 확인하는 로직을 추가할 수 있습니다.
val isAllTrainingProgramFilled = trainingProgramTextState.all { it.isNotEmpty() } val isAllStandardProgramFilled = standardProgramTextState.all { it.isNotEmpty() } state = if ( modifyTitleState.isNotEmpty() && startedDateState.isNotEmpty() && endedDateState.isNotEmpty() && introduceTitleState.isNotEmpty() && addressState.isNotEmpty() && locationState.isNotEmpty() && isAllTrainingProgramFilled && isAllStandardProgramFilled ) { ButtonState.Enable } else { ButtonState.Disable }
406-432: 텍스트 라벨의 표현 통일성 개선"참가자 연수 종류"와 "연수자 연수 종류"라는 라벨이 사용되고 있습니다. 용어가 비슷하여 사용자가 혼동할 수 있으므로, 명확한 구분을 위해 표현을 통일하거나 수정하는 것이 좋습니다.
예를 들어, "참가자 프로그램 종류"와 "교육자 프로그램 종류"로 변경하면 더 명확하게 전달될 수 있습니다.
Text( - text = "참가자 연수 종류", + text = "참가자 프로그램 종류", ... ) Text( - text = "연수자 연수 종류", + text = "교육자 프로그램 종류", ... )feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt (1)
581-588: [확인요청] Preview 함수의 매개변수 기본값 검토
HomeDetailModifyScreenPreview에서 추가된 매개변수들의 기본값이 실제 사용 사례를 잘 반영하는지 확인해주세요. 미리보기 화면에서 의미 있는 데이터가 표시되도록 기본값을 설정하면 개발 및 디버깅에 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
core/design-system/src/main/java/com/school_of_company/design_system/icon/ExpoIcon.kt(1 hunks)core/model/src/main/java/com/school_of_company/model/model/training/TrainingDtoModel.kt(1 hunks)core/network/src/main/java/com/school_of_company/network/dto/training/all/TrainingDto.kt(1 hunks)core/network/src/main/java/com/school_of_company/network/mapper/training/request/TrainingDtoRequestMapper.kt(1 hunks)core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingDtoResponseMapper.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/enum/TrainingCategory.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt(9 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/ExpoDetailScreen.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt(9 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoAddTextField.kt(5 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoNoneLineTextField.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoTrainingSettingBottomSheet.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/component/TrainingSettingButton.kt(1 hunks)feature/expo/src/main/java/com/school_of_company/expo/viewmodel/ExpoViewModel.kt(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- feature/expo/src/main/java/com/school_of_company/expo/enum/TrainingCategory.kt
🔇 Additional comments (13)
core/model/src/main/java/com/school_of_company/model/model/training/TrainingDtoModel.kt (1)
6-6: 속성 이름 변경이 올바르게 적용되었습니다.
finishedAt에서 endedAt으로의 변경이 확인되었습니다. 관련 파일들과의 일관성을 검증하겠습니다.
✅ Verification successful
Let me verify the actual usage of endedAt in these files to ensure consistency.
속성 이름 변경이 전체 코드베이스에서 일관되게 적용되었습니다.
검증 결과, finishedAt에서 endedAt으로의 변경이 모든 관련 파일에서 완전히 이루어졌음을 확인했습니다. 다음 파일들에서 endedAt 속성이 일관되게 사용되고 있습니다:
- DTO 모델:
TrainingDtoModel.kt - 네트워크 DTO:
TrainingDto.kt,TrainingProgramListResponse.kt - 매퍼 클래스들:
TrainingDtoRequestMapper.kt,TrainingDtoResponseMapper.kt,TrainingProgramListResponseMapper.kt - 엔티티:
TrainingProgramListResponseEntity.kt
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistency of the property rename across related files
# Check for any remaining instances of 'finishedAt'
echo "Checking for remaining 'finishedAt' references:"
rg -l "finishedAt" --type kotlin
# Check if endedAt is consistently used in related files
echo "Verifying 'endedAt' usage in related files:"
rg -l "endedAt" --type kotlin
Length of output: 1008
Script:
#!/bin/bash
# Check the actual usage context of endedAt in the related files
echo "Checking usage context of endedAt in related files:"
rg "endedAt" --type kotlin -C 2
Length of output: 4639
core/network/src/main/java/com/school_of_company/network/dto/training/all/TrainingDto.kt (2)
Line range hint 6-11: API 계약 변경 사항 확인 필요
@Json 어노테이션이 사용된 것으로 보아 API 통신에 사용되는 DTO입니다. 이 변경으로 인한 API 호환성 문제가 없는지 확인해 주세요.
다음 사항들을 확인해 주세요:
- API 문서가 업데이트되었는지 확인
- 서버 측에서도 동일한 필드명 변경이 적용되었는지 확인
- 이전 버전과의 호환성 처리가 필요한지 검토
10-10: 속성 이름 변경에 대한 검증이 필요합니다
finishedAt에서 endedAt으로의 변경이 확인되었습니다. 이 변경이 전체 코드베이스에서 일관되게 적용되었는지 확인이 필요합니다.
✅ Verification successful
Let me gather more information about the implementation details to ensure the property name change is consistent.
속성 이름 변경이 일관되게 적용되었음을 확인했습니다
코드베이스 전반에 걸쳐 finishedAt에서 endedAt으로의 변경이 모든 관련 파일에서 일관되게 적용되었습니다:
- DTO 모델 (
TrainingDtoModel.kt) - 네트워크 DTO (
TrainingDto.kt) - 매퍼 구현체들 (
TrainingDtoRequestMapper.kt,TrainingDtoResponseMapper.kt)
모든 관련 코드가 새로운 endedAt 명명 규칙을 따르고 있으며, finishedAt의 잔여 사용이 발견되지 않았습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistency of the property name change across the codebase
# Check for any remaining usage of 'finishedAt'
echo "Checking for remaining 'finishedAt' references..."
rg -l "finishedAt"
# Check for proper implementation of 'endedAt'
echo "Checking 'endedAt' implementation..."
rg -l "endedAt"
# Check for related mapping functions
echo "Checking mapping implementations..."
ast-grep --pattern 'fun toDto($$$) {
$$$
endedAt = $$$
$$$
}'
Length of output: 1121
Script:
#!/bin/bash
# Check the implementation details of files using endedAt
echo "Checking TrainingDtoModel implementation..."
rg "endedAt" core/model/src/main/java/com/school_of_company/model/model/training/TrainingDtoModel.kt -A 2 -B 2
echo "\nChecking TrainingDto implementation..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/dto/training/all/TrainingDto.kt -A 2 -B 2
echo "\nChecking mapper implementations..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingDtoResponseMapper.kt -A 2 -B 2
rg "endedAt" core/network/src/main/java/com/school_of_company/network/mapper/training/request/TrainingDtoRequestMapper.kt -A 2 -B 2
Length of output: 1333
core/network/src/main/java/com/school_of_company/network/mapper/training/request/TrainingDtoRequestMapper.kt (1)
10-10: 속성 이름 변경이 올바르게 적용되었습니다.
finishedAt에서 endedAt으로의 변경이 매퍼에 정확하게 반영되었습니다.
다른 관련 파일들에서도 이 변경사항이 일관되게 적용되었는지 확인하겠습니다:
✅ Verification successful
Let me check the content of these files to ensure the property renaming is consistent.
속성 이름 변경이 전체 코드베이스에서 일관되게 적용되었습니다.
검증 결과, finishedAt에서 endedAt으로의 변경이 다음 파일들에서 모두 올바르게 적용되었음을 확인했습니다:
- DTO:
TrainingDto.kt,TrainingProgramListResponse.kt - 모델:
TrainingDtoModel.kt,TrainingProgramListResponseEntity.kt - 매퍼:
TrainingDtoRequestMapper.kt,TrainingDtoResponseMapper.kt,TrainingProgramListResponseMapper.kt
또한 기존 finishedAt 속성명이 코드베이스 어디에서도 사용되지 않는 것을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the property rename across the codebase
# Expected: All occurrences should use 'endedAt' instead of 'finishedAt'
echo "Checking for any remaining uses of 'finishedAt'..."
rg -l "finishedAt"
echo "Verifying the usage of 'endedAt' in related files..."
rg -l "endedAt" --type kotlin
Length of output: 1018
Script:
#!/bin/bash
# Check the context of endedAt usage in the related files
echo "Checking TrainingDto.kt..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/dto/training/all/TrainingDto.kt -C 2
echo "\nChecking TrainingDtoModel.kt..."
rg "endedAt" core/model/src/main/java/com/school_of_company/model/model/training/TrainingDtoModel.kt -C 2
echo "\nChecking TrainingProgramListResponseEntity.kt..."
rg "endedAt" core/model/src/main/java/com/school_of_company/model/entity/training/TrainingProgramListResponseEntity.kt -C 2
echo "\nChecking TrainingProgramListResponse.kt..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/dto/training/response/TrainingProgramListResponse.kt -C 2
echo "\nChecking TrainingDtoResponseMapper.kt..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingDtoResponseMapper.kt -C 2
echo "\nChecking TrainingProgramListResponseMapper.kt..."
rg "endedAt" core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingProgramListResponseMapper.kt -C 2
Length of output: 2086
core/network/src/main/java/com/school_of_company/network/mapper/training/response/TrainingDtoResponseMapper.kt (1)
10-10: 변경된 속성 이름의 일관성을 확인하세요.
finishedAt에서 endedAt으로의 변경이 확인되었습니다. 이 변경이 올바르게 이루어졌습니다.
관련 파일들에서 일관성 있게 변경되었는지 확인하겠습니다:
✅ Verification successful
속성 이름 변경이 일관되게 적용되었습니다.
코드베이스 전체에서 finishedAt이 완전히 제거되고 endedAt으로 일관되게 변경되었음을 확인했습니다. 다음 파일들에서 모두 올바르게 적용되었습니다:
- DTO 클래스들 (
TrainingDto,TrainingProgramListResponse) - 매퍼 클래스들 (
TrainingDtoResponseMapper,TrainingDtoRequestMapper,TrainingProgramListResponseMapper) - 모델 클래스들 (
TrainingDtoModel,TrainingProgramListResponseEntity)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistent renaming from finishedAt to endedAt
# Expected: No occurrences of finishedAt in related files
# Search for any remaining instances of finishedAt
rg -l 'finishedAt' --type kotlin
# Search for the new property name endedAt to confirm proper usage
rg -l 'endedAt' --type kotlin
Length of output: 794
Script:
#!/bin/bash
# Let's check the actual content of these files to verify the consistency
# and ensure there are no remaining 'finishedAt' references
# Check for any remaining finishedAt in the found files
rg 'finishedAt' --type kotlin
# Get context around endedAt usage to verify proper implementation
rg 'endedAt' -B 2 -A 2 --type kotlin
Length of output: 4561
feature/expo/src/main/java/com/school_of_company/expo/view/component/ExpoAddTextField.kt (3)
Line range hint 28-37: 함수 시그니처 변경이 적절합니다!
콜백 함수 onTrainingSetting의 추가는 컴포넌트의 재사용성을 높이고 디자인 요구사항을 잘 반영하고 있습니다.
101-104: 아이콘 색상 변경이 디자인 변경사항과 일치합니다!
XIcon의 색상을 gray500에서 black으로 변경한 것이 디자인 요구사항을 잘 반영하고 있습니다.
96-100: TrainingSettingButton 컴포넌트 구현을 확인해주세요.
TrainingSettingButton 컴포넌트가 프로젝트 내에 구현되어 있는지 확인이 필요합니다.
✅ Verification successful
TrainingSettingButton 컴포넌트가 정상적으로 구현되어 있습니다.
TrainingSettingButton 컴포넌트는 feature/expo/src/main/java/com/school_of_company/expo/view/component/TrainingSettingButton.kt 파일에 적절하게 구현되어 있으며, 필요한 모든 속성(text, onClick)을 포함하고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# TrainingSettingButton 컴포넌트 구현 확인
rg -t kotlin "TrainingSettingButton"
Length of output: 570
Script:
#!/bin/bash
# TrainingSettingButton 컴포넌트 구현 내용 확인
cat feature/expo/src/main/java/com/school_of_company/expo/view/component/TrainingSettingButton.kt
Length of output: 1614
core/design-system/src/main/java/com/school_of_company/design_system/icon/ExpoIcon.kt (1)
118-126: CheckIcon 함수의 변경사항이 적절합니다.
기존 아이콘 컴포넌트들의 패턴을 잘 따르고 있으며, tint 매개변수 추가는 적절해 보입니다.
feature/expo/src/main/java/com/school_of_company/expo/viewmodel/ExpoViewModel.kt (1)
68-72: 상태 관리가 적절히 구현되었습니다!
private MutableStateFlow와 public StateFlow를 적절히 분리하여 상태 캡슐화를 잘 구현하였습니다.
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt (3)
93-94: [확인요청] viewModel 상태와 로컬 상태의 동기화 확인
trainingTextState와 standardTextState 변수를 rememberSaveable로 로컬 상태로 저장하고 있습니다. 하지만 초기값을 viewModel.trainingProgramTextState.value와 viewModel.standardProgramTextState.value로 설정하여 viewModel의 상태와 로컬 상태 사이에 데이터 불일치가 발생할 수 있습니다. 로컬 상태 대신 viewModel의 상태를 직접 사용하거나, 데이터 변경 시 양쪽 상태가 동기화되도록 확인해주세요.
452-452: [확인요청] '참가자 연수 종류' 라벨의 의미 검토
라벨로 사용된 "참가자 연수 종류"가 사용자에게 명확하게 전달되는지 확인이 필요합니다. "연수 프로그램 목록" 또는 "참가자 대상 연수"와 같이 더 명확한 표현이 있는지 검토해주세요.
475-475: [확인요청] '연수자 연수 종류' 라벨의 명확성 확인
"연수자 연수 종류"라는 라벨이 사용자에게 혼란을 줄 수 있습니다. 특히 앞의 "참가자 연수 종류"와의 차이를 명확히 구분할 수 있도록 라벨을 수정하거나 추가 설명을 제공하는 것이 좋습니다.
| decorationBox = { innerTextField -> | ||
| Column(verticalArrangement = Arrangement.Center) { | ||
| if (textState.isEmpty()) { | ||
| placeHolder() | ||
| } | ||
| } | ||
| innerTextField() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
레이아웃 안정성 개선 필요
현재 구현은 플레이스홀더가 나타나거나 사라질 때 레이아웃 변화를 일으킬 수 있습니다. Box를 사용하여 레이아웃 안정성을 개선할 수 있습니다.
다음과 같이 수정해보세요:
- decorationBox = { innerTextField ->
- Column(verticalArrangement = Arrangement.Center) {
- if (textState.isEmpty()) {
- placeHolder()
- }
- }
- innerTextField()
- }
+ decorationBox = { innerTextField ->
+ Box(
+ contentAlignment = Alignment.CenterStart,
+ modifier = Modifier.padding(horizontal = 16.dp)
+ ) {
+ if (textState.isEmpty()) {
+ placeHolder()
+ }
+ innerTextField()
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| decorationBox = { innerTextField -> | |
| Column(verticalArrangement = Arrangement.Center) { | |
| if (textState.isEmpty()) { | |
| placeHolder() | |
| } | |
| } | |
| innerTextField() | |
| } | |
| decorationBox = { innerTextField -> | |
| Box( | |
| contentAlignment = Alignment.CenterStart, | |
| modifier = Modifier.padding(horizontal = 16.dp) | |
| ) { | |
| if (textState.isEmpty()) { | |
| placeHolder() | |
| } | |
| innerTextField() | |
| } | |
| } |
| ExpoNoneLineTextField( | ||
| textState = startedTextState, | ||
| placeHolder = { | ||
| Text( | ||
| text = "yyyy-MM-dd HH:mm", | ||
| style = typography.titleBold2, | ||
| color = colors.gray300 | ||
| ) | ||
| }, | ||
| onTextChange = onStartedTextChange | ||
| ) | ||
|
|
||
| ExpoNoneLineTextField( | ||
| textState = endedTextState, | ||
| placeHolder = { | ||
| Text( | ||
| text = "yyyy-MM-dd HH:mm", | ||
| style = typography.titleBold2, | ||
| color = colors.gray300 | ||
| ) | ||
| }, | ||
| onTextChange = onEndedTextChange | ||
| ) |
There was a problem hiding this comment.
날짜 및 시간 입력에 대한 유효성 검사가 필요합니다.
현재 구현에서는 사용자가 입력한 날짜와 시간 형식의 유효성을 검사하지 않고 있습니다. 잘못된 형식의 입력이 들어올 경우 문제가 발생할 수 있습니다.
다음과 같은 개선사항을 제안합니다:
- 날짜 형식 검증 함수 추가
- 시작 시간이 종료 시간보다 이후인 경우 처리
- 사용자 친화적인 날짜 선택기 도입 고려
이러한 기능을 구현하는데 도움이 필요하시다면 말씀해 주세요.
| @Composable | ||
| fun CustomCheckBox( | ||
| modifier: Modifier = Modifier, | ||
| primaryColor: ExpoColor = ExpoColor, | ||
| categoryState: TrainingCategory = TrainingCategory.CHOICE, | ||
| onCategoryChange: (TrainingCategory) -> Unit | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
체크박스는 접근성이 중요한 UI 요소입니다. 현재 구현에서는 접근성 속성이 누락되어 있습니다.
다음과 같은 수정을 제안합니다:
fun CustomCheckBox(
modifier: Modifier = Modifier,
primaryColor: ExpoColor = ExpoColor,
categoryState: TrainingCategory = TrainingCategory.CHOICE,
- onCategoryChange: (TrainingCategory) -> Unit
+ onCategoryChange: (TrainingCategory) -> Unit,
+ contentDescription: String? = null
) {
var category by remember { mutableStateOf(categoryState) }
+ val semantics = Modifier.semantics {
+ role = Role.Checkbox
+ selected = category == TrainingCategory.ESSENTIAL
+ contentDescription?.let { this.contentDescription = it }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Composable | |
| fun CustomCheckBox( | |
| modifier: Modifier = Modifier, | |
| primaryColor: ExpoColor = ExpoColor, | |
| categoryState: TrainingCategory = TrainingCategory.CHOICE, | |
| onCategoryChange: (TrainingCategory) -> Unit | |
| ) { | |
| @Composable | |
| fun CustomCheckBox( | |
| modifier: Modifier = Modifier, | |
| primaryColor: ExpoColor = ExpoColor, | |
| categoryState: TrainingCategory = TrainingCategory.CHOICE, | |
| onCategoryChange: (TrainingCategory) -> Unit, | |
| contentDescription: String? = null | |
| ) { |
| internal fun updateStandardProgramText(index: Int, newText: String) { | ||
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | ||
| set(index, newText) | ||
| } | ||
| } | ||
|
|
||
| internal fun addStandardProgramText() { | ||
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | ||
| add("") | ||
| } | ||
| } | ||
|
|
||
| internal fun removeStandardProgramText(index: Int) { | ||
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | ||
| removeAt(index) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 중복을 제거하여 유지보수성을 향상시킬 수 있습니다.
훈련 프로그램과 표준 프로그램 텍스트 관리 로직이 거의 동일합니다. 다음과 같이 공통 로직을 추출하여 코드 중복을 제거하는 것을 추천드립니다:
+ private fun updateTextState(
+ state: MutableStateFlow<List<String>>,
+ index: Int,
+ newText: String
+ ) {
+ val currentList = state.value
+ state.value = buildList(currentList.size) {
+ addAll(currentList)
+ set(index, newText)
+ }
+ }
+
+ private fun addTextState(state: MutableStateFlow<List<String>>) {
+ val currentList = state.value
+ state.value = buildList(currentList.size + 1) {
+ addAll(currentList)
+ add("")
+ }
+ }
+
+ private fun removeTextState(
+ state: MutableStateFlow<List<String>>,
+ index: Int
+ ) {
+ val currentList = state.value
+ state.value = buildList(currentList.size - 1) {
+ addAll(currentList.take(index))
+ addAll(currentList.drop(index + 1))
+ }
+ }
- internal fun updateStandardProgramText(index: Int, newText: String) {
- _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply {
- set(index, newText)
- }
- }
+ internal fun updateStandardProgramText(index: Int, newText: String) =
+ updateTextState(_standardProgramTextState, index, newText)
- internal fun addStandardProgramText() {
- _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply {
- add("")
- }
- }
+ internal fun addStandardProgramText() =
+ addTextState(_standardProgramTextState)
- internal fun removeStandardProgramText(index: Int) {
- _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply {
- removeAt(index)
- }
- }
+ internal fun removeStandardProgramText(index: Int) =
+ removeTextState(_standardProgramTextState, index)이렇게 수정하면:
- 코드 중복이 제거됩니다
- 성능이 개선됩니다
- 유지보수가 더 쉬워집니다
- 버그 발생 가능성이 줄어듭니다
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| internal fun updateStandardProgramText(index: Int, newText: String) { | |
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | |
| set(index, newText) | |
| } | |
| } | |
| internal fun addStandardProgramText() { | |
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | |
| add("") | |
| } | |
| } | |
| internal fun removeStandardProgramText(index: Int) { | |
| _standardProgramTextState.value = _standardProgramTextState.value.toMutableList().apply { | |
| removeAt(index) | |
| } | |
| } | |
| private fun updateTextState( | |
| state: MutableStateFlow<List<String>>, | |
| index: Int, | |
| newText: String | |
| ) { | |
| val currentList = state.value | |
| state.value = buildList(currentList.size) { | |
| addAll(currentList) | |
| set(index, newText) | |
| } | |
| } | |
| private fun addTextState(state: MutableStateFlow<List<String>>) { | |
| val currentList = state.value | |
| state.value = buildList(currentList.size + 1) { | |
| addAll(currentList) | |
| add("") | |
| } | |
| } | |
| private fun removeTextState( | |
| state: MutableStateFlow<List<String>>, | |
| index: Int | |
| ) { | |
| val currentList = state.value | |
| state.value = buildList(currentList.size - 1) { | |
| addAll(currentList.take(index)) | |
| addAll(currentList.drop(index + 1)) | |
| } | |
| } | |
| internal fun updateStandardProgramText(index: Int, newText: String) = | |
| updateTextState(_standardProgramTextState, index, newText) | |
| internal fun addStandardProgramText() = | |
| addTextState(_standardProgramTextState) | |
| internal fun removeStandardProgramText(index: Int) = | |
| removeTextState(_standardProgramTextState, index) |
| ) { | ||
| var trainingTextState by rememberSaveable { mutableStateOf(listOf("")) } | ||
| val (openTrainingSettingBottomSheet, isOpenTrainingSettingBottomSheet) = rememberSaveable { mutableStateOf(false) } |
There was a problem hiding this comment.
mutableStateOf 사용법 수정 필요
현재 코드에서 mutableStateOf를 사용하여 openTrainingSettingBottomSheet와 isOpenTrainingSettingBottomSheet를 분해하고 있습니다. 그러나 mutableStateOf는 단일 MutableState 객체를 반환하므로 분해할 수 없습니다. 대신 by 키워드를 사용하여 상태 변수를 선언하고 직접 값을 변경하는 것이 좋습니다.
수정된 코드는 다음과 같습니다:
- val (openTrainingSettingBottomSheet, isOpenTrainingSettingBottomSheet) = rememberSaveable { mutableStateOf(false) }
+ var openTrainingSettingBottomSheet by rememberSaveable { mutableStateOf(false) }그리고 상태 변경 부분을 다음과 같이 수정하세요:
- isOpenTrainingSettingBottomSheet(true)
+ openTrainingSettingBottomSheet = true
- isOpenTrainingSettingBottomSheet(false)
+ openTrainingSettingBottomSheet = falseAlso applies to: 421-421, 444-444, 500-500
| ) { | ||
| var trainingTextState by rememberSaveable { mutableStateOf(listOf("")) } | ||
| val (openTrainingSettingBottomSheet, isOpenTrainingSettingBottomSheet) = rememberSaveable { mutableStateOf(false) } |
There was a problem hiding this comment.
[수정 필요] 상태 변수의 선언 방식 수정
rememberSaveable { mutableStateOf(false) }은 MutableState<Boolean>을 반환하며, 이를 두 개의 변수로 분해할 수 없습니다. 대신 다음과 같이 변수 하나로 선언하고 사용하는 것이 좋습니다:
var openTrainingSettingBottomSheet by rememberSaveable { mutableStateOf(false) }이를 통해 상태 변경 시 openTrainingSettingBottomSheet = true 또는 false로 직접 할당할 수 있습니다.
| if (openTrainingSettingBottomSheet) { | ||
| Dialog(onDismissRequest = { isOpenTrainingSettingBottomSheet(false) }) { | ||
| ExpoTrainingSettingBottomSheet( | ||
| onCancelClick = { isOpenTrainingSettingBottomSheet(false) }, | ||
| startedTextState = "", | ||
| endedTextState = "", | ||
| onStartedTextChange = {}, | ||
| onEndedTextChange = {}, | ||
| onCategoryChange = {}, | ||
| onButtonClick = { /*TODO*/ } | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
[수정 필요] openTrainingSettingBottomSheet 상태 관리 개선
현재 isOpenTrainingSettingBottomSheet를 함수처럼 사용하고 있지만, 이는 올바른 방법이 아닙니다. 앞서 제안한 대로 상태 변수를 하나로 선언하고 직접 값을 변경하는 방식으로 수정해주세요:
var openTrainingSettingBottomSheet by rememberSaveable { mutableStateOf(false) }상태 변경 시에는 다음과 같이 사용합니다:
openTrainingSettingBottomSheet = true // 다이얼로그 열기
openTrainingSettingBottomSheet = false // 다이얼로그 닫기
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt (1)
88-89: 상태 관리 개선 필요
trainingProgramTextState와standardProgramTextState를 별도의 상태로 관리하는 것이 좋습니다. 두 상태가 매우 유사한 구조를 가지고 있으므로, 공통 상태 관리 로직을 만들어 재사용하는 것이 좋습니다.sealed class ProgramType { object Training : ProgramType() object Standard : ProgramType() } @Composable fun rememberProgramState(type: ProgramType) { // 공통 상태 관리 로직 }feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt (2)
537-549: TODO 항목 구현 필요
ExpoTrainingSettingBottomSheet의 콜백 함수들이 구현되지 않았습니다. 특히onButtonClick에 TODO 주석이 있습니다.이 기능의 구현을 도와드릴까요? GitHub 이슈를 생성하여 추적할 수 있습니다.
또한, 다음과 같은 개선사항을 제안드립니다:
ExpoTrainingSettingBottomSheet( onCancelClick = { isOpenTrainingSettingBottomSheet(false) }, - startedTextState = "", - endedTextState = "", - onStartedTextChange = {}, - onEndedTextChange = {}, - onCategoryChange = {}, - onButtonClick = { /*TODO*/ } + startedTextState = viewModel.trainingStartDate.value, + endedTextState = viewModel.trainingEndDate.value, + onStartedTextChange = viewModel::updateTrainingStartDate, + onEndedTextChange = viewModel::updateTrainingEndDate, + onCategoryChange = viewModel::updateTrainingCategory, + onButtonClick = { + viewModel.saveTrainingSettings() + isOpenTrainingSettingBottomSheet(false) + } )
573-580: Preview 함수 개선 필요Preview 함수에서 의미 있는 샘플 데이터를 사용하면 UI를 더 효과적으로 테스트할 수 있습니다.
다음과 같이 수정하는 것을 추천드립니다:
- trainingProgramTextState = emptyList(), + trainingProgramTextState = listOf("프로그램 A", "프로그램 B"), - standardProgramTextState = emptyList(), + standardProgramTextState = listOf("기본 과정", "심화 과정"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt(9 hunks)feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt(9 hunks)
🔇 Additional comments (3)
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoCreateScreen.kt (2)
215-215:
상태 관리 방식 수정 필요
mutableStateOf를 분해하여 사용하는 것은 권장되지 않습니다. 이전 리뷰 코멘트에서 지적된 대로 수정이 필요합니다.
- val (openTrainingSettingBottomSheet, isOpenTrainingSettingBottomSheet) = rememberSaveable { mutableStateOf(false) }
+ var openTrainingSettingBottomSheet by rememberSaveable { mutableStateOf(false) }Likely invalid or redundant comment.
473-474: 버튼 활성화 조건 개선 필요
버튼 활성화 조건이 단순히 리스트가 비어있지 않은지만 확인합니다. 각 입력 필드의 유효성도 검증해야 합니다.
feature/expo/src/main/java/com/school_of_company/expo/view/ExpoModifyScreen.kt (1)
228-228: [중복 코멘트] 상태 변수의 선언 방식 수정
| text = "참가자 연수 종류", | ||
| style = typography.bodyBold2, | ||
| color = colors.black, | ||
| ) | ||
|
|
||
| ExpoAddTextField( | ||
| trainingTextFieldList = trainingTextState, | ||
| trainingTextFieldList = standardProgramTextState, | ||
| onValueChange = { index, newState -> | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| set(index, newState) | ||
| } | ||
| onStandardProgramChange(index, newState) | ||
| }, | ||
| onAddTextField = { | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| add("") | ||
| } | ||
| onAddTextField = { onAddStandardProgram() }, | ||
| onRemoveTextField = { index -> | ||
| onRemoveStandardProgram(index) | ||
| }, | ||
| onRemoveTextField = { | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| removeAt(it) | ||
| } | ||
| onTrainingSetting = { | ||
| isOpenTrainingSettingBottomSheet(true) | ||
| }, | ||
| placeHolder = "연수 종류를 입력해주세요." | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
UI 컴포넌트 재사용성 개선 필요
ExpoAddTextField 컴포넌트가 두 번 사용되며, 매우 유사한 로직을 가지고 있습니다. 컴포넌트의 재사용성을 높이기 위해 공통 속성을 추출하는 것이 좋습니다.
data class ExpoAddTextFieldProps(
val title: String,
val textFieldList: List<String>,
val onValueChange: (Int, String) -> Unit,
val onAddTextField: () -> Unit,
val onRemoveTextField: (Int) -> Unit,
val onTrainingSetting: () -> Unit,
val placeHolder: String
)| if (openTrainingSettingBottomSheet) { | ||
| Dialog(onDismissRequest = { isOpenTrainingSettingBottomSheet(false) }) { | ||
| ExpoTrainingSettingBottomSheet( | ||
| onCancelClick = { isOpenTrainingSettingBottomSheet(false) }, | ||
| startedTextState = "", | ||
| endedTextState = "", | ||
| onStartedTextChange = {}, | ||
| onEndedTextChange = {}, | ||
| onCategoryChange = {}, | ||
| onButtonClick = { /*TODO*/ } | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Dialog 상태 관리 및 기능 구현 미완료
- Dialog의 상태 관리가 이전 리뷰 코멘트대로 수정되어야 합니다.
- TODO 주석이 있는 미구현 기능이 있습니다.
- Dialog(onDismissRequest = { isOpenTrainingSettingBottomSheet(false) }) {
+ Dialog(onDismissRequest = { openTrainingSettingBottomSheet = false }) {미구현된 기능에 대한 구현을 도와드릴까요? GitHub 이슈를 생성하여 추적할 수 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (openTrainingSettingBottomSheet) { | |
| Dialog(onDismissRequest = { isOpenTrainingSettingBottomSheet(false) }) { | |
| ExpoTrainingSettingBottomSheet( | |
| onCancelClick = { isOpenTrainingSettingBottomSheet(false) }, | |
| startedTextState = "", | |
| endedTextState = "", | |
| onStartedTextChange = {}, | |
| onEndedTextChange = {}, | |
| onCategoryChange = {}, | |
| onButtonClick = { /*TODO*/ } | |
| ) | |
| } | |
| } | |
| if (openTrainingSettingBottomSheet) { | |
| Dialog(onDismissRequest = { openTrainingSettingBottomSheet = false }) { | |
| ExpoTrainingSettingBottomSheet( | |
| onCancelClick = { isOpenTrainingSettingBottomSheet(false) }, | |
| startedTextState = "", | |
| endedTextState = "", | |
| onStartedTextChange = {}, | |
| onEndedTextChange = {}, | |
| onCategoryChange = {}, | |
| onButtonClick = { /*TODO*/ } | |
| ) | |
| } | |
| } |
| val trainingProgramTextState by rememberSaveable { mutableStateOf(viewModel.trainingProgramTextState.value) } | ||
| val standardProgramTextState by rememberSaveable { mutableStateOf(viewModel.standardProgramTextState.value) } |
There was a problem hiding this comment.
상태 관리 개선 필요
rememberSaveable을 사용하여 초기값을 설정하는 방식은 ViewModel의 상태 변경을 반영하지 못할 수 있습니다. 대신 ViewModel의 상태를 직접 구독하는 것이 좋습니다.
다음과 같이 수정하는 것을 추천드립니다:
-val trainingProgramTextState by rememberSaveable { mutableStateOf(viewModel.trainingProgramTextState.value) }
-val standardProgramTextState by rememberSaveable { mutableStateOf(viewModel.standardProgramTextState.value) }
+val trainingProgramTextState by viewModel.trainingProgramTextState.collectAsStateWithLifecycle()
+val standardProgramTextState by viewModel.standardProgramTextState.collectAsStateWithLifecycle()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val trainingProgramTextState by rememberSaveable { mutableStateOf(viewModel.trainingProgramTextState.value) } | |
| val standardProgramTextState by rememberSaveable { mutableStateOf(viewModel.standardProgramTextState.value) } | |
| val trainingProgramTextState by viewModel.trainingProgramTextState.collectAsStateWithLifecycle() | |
| val standardProgramTextState by viewModel.standardProgramTextState.collectAsStateWithLifecycle() |
| text = "참가자 연수 종류", | ||
| style = typography.bodyBold2, | ||
| color = colors.black, | ||
| ) | ||
|
|
||
| ExpoAddTextField( | ||
| trainingTextFieldList = trainingTextState, | ||
| trainingTextFieldList = standardProgramTextState, | ||
| onValueChange = { index, newState -> | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| set(index, newState) | ||
| } | ||
| onStandardProgramChange(index, newState) | ||
| }, | ||
| onAddTextField = { | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| add("") | ||
| } | ||
| onAddTextField = { onAddStandardProgram() }, | ||
| onRemoveTextField = { index -> | ||
| onRemoveStandardProgram(index) | ||
| }, | ||
| onRemoveTextField = { | ||
| trainingTextState = trainingTextState.toMutableList().apply { | ||
| removeAt(it) | ||
| } | ||
| onTrainingSetting = { | ||
| isOpenTrainingSettingBottomSheet(true) | ||
| }, | ||
| placeHolder = "연수 종류를 입력해주세요." | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선 필요
연수 종류 입력 필드에 대한 접근성 레이블이 누락되었습니다. 시각 장애인 사용자를 위해 적절한 설명을 추가해야 합니다.
다음과 같이 수정하는 것을 추천드립니다:
ExpoAddTextField(
trainingTextFieldList = standardProgramTextState,
onValueChange = { index, newState ->
onStandardProgramChange(index, newState)
},
onAddTextField = { onAddStandardProgram() },
onRemoveTextField = { index ->
onRemoveStandardProgram(index)
},
onTrainingSetting = {
isOpenTrainingSettingBottomSheet(true)
},
- placeHolder = "연수 종류를 입력해주세요."
+ placeHolder = "연수 종류를 입력해주세요.",
+ modifier = Modifier.semantics {
+ contentDescription = "참가자 연수 종류 입력 필드"
+ }
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| text = "참가자 연수 종류", | |
| style = typography.bodyBold2, | |
| color = colors.black, | |
| ) | |
| ExpoAddTextField( | |
| trainingTextFieldList = trainingTextState, | |
| trainingTextFieldList = standardProgramTextState, | |
| onValueChange = { index, newState -> | |
| trainingTextState = trainingTextState.toMutableList().apply { | |
| set(index, newState) | |
| } | |
| onStandardProgramChange(index, newState) | |
| }, | |
| onAddTextField = { | |
| trainingTextState = trainingTextState.toMutableList().apply { | |
| add("") | |
| } | |
| onAddTextField = { onAddStandardProgram() }, | |
| onRemoveTextField = { index -> | |
| onRemoveStandardProgram(index) | |
| }, | |
| onRemoveTextField = { | |
| trainingTextState = trainingTextState.toMutableList().apply { | |
| removeAt(it) | |
| } | |
| onTrainingSetting = { | |
| isOpenTrainingSettingBottomSheet(true) | |
| }, | |
| placeHolder = "연수 종류를 입력해주세요." | |
| ) | |
| text = "참가자 연수 종류", | |
| style = typography.bodyBold2, | |
| color = colors.black, | |
| ) | |
| ExpoAddTextField( | |
| trainingTextFieldList = standardProgramTextState, | |
| onValueChange = { index, newState -> | |
| onStandardProgramChange(index, newState) | |
| }, | |
| onAddTextField = { onAddStandardProgram() }, | |
| onRemoveTextField = { index -> | |
| onRemoveStandardProgram(index) | |
| }, | |
| onTrainingSetting = { | |
| isOpenTrainingSettingBottomSheet(true) | |
| }, | |
| placeHolder = "연수 종류를 입력해주세요.", | |
| modifier = Modifier.semantics { | |
| contentDescription = "참가자 연수 종류 입력 필드" | |
| } | |
| ) |
audgns10
deleted the
refactor/218-apply-expo-training-stardard-design-change
branch
💡 개요
📃 작업내용
디자인에 변동사항이 있어 이를 수정하였습니다.
2024-11-26.12.10.07.mov
🔀 변경사항
🙋♂️ 질문사항
🍴 사용방법
🎸 기타
Summary by CodeRabbit
New Features
CheckIcon에 색상 조정 기능 추가.TrainingCategory열거형 추가로 교육 유형 분류 가능.ExpoTrainingSettingBottomSheet다이얼로그 추가로 교육 설정 관리 기능 제공.ExpoAddTextField및TrainingSettingButton컴포넌트 추가로 사용자 인터페이스 개선.ExpoCreateScreen및ExpoModifyScreen에서 교육 프로그램 관리 기능 향상.ExpoNoneLineTextField로 사용자 정의 텍스트 입력 필드 제공.ExpoDetailScreen에 메시지 전송 및 QR 코드 표시를 위한 다이얼로그 기능 추가.Bug Fixes
Documentation