Skip to content
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

πŸ”€ :: (#218) - λ””μžμΈ 변동사항을 λ”°λ¦…λ‹ˆλ‹€. #219

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,15 @@ fun ImageIcon(
}

@Composable
fun CheckIcon(modifier: Modifier = Modifier) {
fun CheckIcon(
modifier: Modifier = Modifier,
tint: Color = Color.Unspecified
) {
Icon(
painter = painterResource(id = R.drawable.ic_check),
contentDescription = stringResource(id = R.string.check_description),
modifier = modifier
modifier = modifier,
tint = tint
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ package com.school_of_company.model.model.training
data class TrainingDtoModel(
val title: String,
val startedAt: String,
val finishedAt: String,
val endedAt: String,
val category: String,
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ import com.squareup.moshi.JsonClass
data class TrainingDto(
@Json(name = "title") val title: String,
@Json(name = "startedAt") val startedAt: String,
@Json(name = "finishedAt") val finishedAt: String,
@Json(name = "endedAt") val endedAt: String,
@Json(name = "category") val category: String,
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ fun TrainingDtoModel.toDto(): TrainingDto =
TrainingDto(
title = this.title,
startedAt = this.startedAt,
finishedAt = this.finishedAt,
endedAt = this.endedAt,
category = this.category,
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ fun TrainingDto.toModel(): TrainingDtoModel =
TrainingDtoModel(
title = this.title,
startedAt = this.startedAt,
finishedAt = this.finishedAt,
endedAt = this.endedAt,
category = this.category
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.school_of_company.expo.enum

enum class TrainingCategory {
ESSENTIAL,
CHOICE
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import androidx.compose.ui.platform.LocalFocusManager
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.Dialog
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import coil.compose.rememberAsyncImagePainter
Expand All @@ -55,13 +56,14 @@ import com.school_of_company.design_system.component.button.ExpoStateButton
import com.school_of_company.design_system.component.button.state.ButtonState
import com.school_of_company.design_system.component.modifier.clickable.expoClickable
import com.school_of_company.design_system.component.modifier.padding.paddingHorizontal
import com.school_of_company.design_system.component.textfield.ExpoAddTextField
import com.school_of_company.design_system.component.textfield.ExpoLocationIconTextField
import com.school_of_company.design_system.component.textfield.LimitedLengthTextField
import com.school_of_company.design_system.component.textfield.NoneLimitedLengthTextField
import com.school_of_company.design_system.icon.ImageIcon
import com.school_of_company.design_system.icon.WarnIcon
import com.school_of_company.design_system.theme.ExpoAndroidTheme
import com.school_of_company.expo.view.component.ExpoAddTextField
import com.school_of_company.expo.view.component.ExpoTrainingSettingBottomSheet
import com.school_of_company.expo.viewmodel.ExpoViewModel
import com.school_of_company.expo.viewmodel.uistate.ImageUpLoadUiState
import com.school_of_company.expo.viewmodel.uistate.RegisterExpoInformationUiState
Expand All @@ -83,6 +85,8 @@ internal fun ExpoCreateRoute(
val addressState by viewModel.address.collectAsStateWithLifecycle()
val locationState by viewModel.location.collectAsStateWithLifecycle()
val coverImageState by viewModel.cover_image.collectAsStateWithLifecycle()
val trainingProgramTextState by viewModel.trainingProgramTextState.collectAsStateWithLifecycle()
val standardProgramTextState by viewModel.standardProgramTextState.collectAsStateWithLifecycle()

var selectedImageUri by remember { mutableStateOf<Uri?>(null) }

Expand Down Expand Up @@ -168,6 +172,14 @@ internal fun ExpoCreateRoute(
onErrorToast(null, R.string.expo_image_fail)
}
},
trainingProgramTextState = trainingProgramTextState,
onTrainingProgramChange = viewModel::updateTrainingProgramText,
onAddTrainingProgram = viewModel::addTrainingProgramText,
onRemoveTrainingProgram = viewModel::removeTrainingProgramText,
standardProgramTextState = standardProgramTextState,
onStandardProgramChange = viewModel::updateStandardProgramText,
onAddStandardProgram = viewModel::addStandardProgramText,
onRemoveStandardProgram = viewModel::removeStandardProgramText
)
}

Expand All @@ -191,8 +203,16 @@ internal fun ExpoCreateScreen(
onAddressChange: (String) -> Unit,
onLocationChange: (String) -> Unit,
onExpoCreateCallBack: () -> Unit,
trainingProgramTextState: List<String>,
onTrainingProgramChange: (Int, String) -> Unit,
onAddTrainingProgram: () -> Unit,
onRemoveTrainingProgram: (Int) -> Unit,
standardProgramTextState: List<String>,
onStandardProgramChange: (Int, String) -> Unit,
onAddStandardProgram: () -> Unit,
onRemoveStandardProgram: (Int) -> Unit
) {
var trainingTextState by rememberSaveable { mutableStateOf(listOf("")) }
val (openTrainingSettingBottomSheet, isOpenTrainingSettingBottomSheet) = rememberSaveable { mutableStateOf(false) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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 = false

Also applies to: 421-421, 444-444, 500-500


ExpoAndroidTheme { colors, typography ->
Column(
Expand Down Expand Up @@ -375,27 +395,45 @@ internal fun ExpoCreateScreen(

Column(verticalArrangement = Arrangement.spacedBy(8.dp, Alignment.Top)) {
Text(
text = "μ—°μˆ˜ μ’…λ₯˜",
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 = "μ—°μˆ˜ μ’…λ₯˜λ₯Ό μž…λ ₯ν•΄μ£Όμ„Έμš”."
)
Comment on lines +398 to +416
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ› οΈ 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
)


Spacer(modifier = Modifier.padding(top = 28.dp))

Text(
text = "μ—°μˆ˜μž μ—°μˆ˜ μ’…λ₯˜",
style = typography.bodyBold2,
color = colors.black,
)

ExpoAddTextField(
trainingTextFieldList = trainingProgramTextState,
onValueChange = { index, newState ->
onTrainingProgramChange(index, newState)
},
onAddTextField = { onAddTrainingProgram() },
onRemoveTextField = { index ->
onRemoveTrainingProgram(index)
},
onTrainingSetting = {
isOpenTrainingSettingBottomSheet(true)
},
placeHolder = "μ—°μˆ˜ μ’…λ₯˜λ₯Ό μž…λ ₯ν•΄μ£Όμ„Έμš”."
)
Expand Down Expand Up @@ -432,7 +470,8 @@ internal fun ExpoCreateScreen(
introduceTitleState.isNotEmpty() &&
addressState.isNotEmpty() &&
locationState.isNotEmpty() &&
trainingTextState.isNotEmpty()
trainingProgramTextState.isNotEmpty() &&
standardProgramTextState.isNotEmpty()
) {
ButtonState.Enable
} else {
Expand All @@ -448,6 +487,20 @@ internal fun ExpoCreateScreen(
}
}
}

if (openTrainingSettingBottomSheet) {
Dialog(onDismissRequest = { isOpenTrainingSettingBottomSheet(false) }) {
ExpoTrainingSettingBottomSheet(
onCancelClick = { isOpenTrainingSettingBottomSheet(false) },
startedTextState = "",
endedTextState = "",
onStartedTextChange = {},
onEndedTextChange = {},
onCategoryChange = {},
onButtonClick = { /*TODO*/ }
)
}
}
Comment on lines +491 to +503
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Dialog μƒνƒœ 관리 및 κΈ°λŠ₯ κ΅¬ν˜„ λ―Έμ™„λ£Œ

  1. Dialog의 μƒνƒœ 관리가 이전 리뷰 μ½”λ©˜νŠΈλŒ€λ‘œ μˆ˜μ •λ˜μ–΄μ•Ό ν•©λ‹ˆλ‹€.
  2. 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.

Suggested change
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*/ }
)
}
}

}

@Preview
Expand All @@ -469,5 +522,13 @@ private fun ExpoCreateScreenPreview() {
onEndedDateChange = {},
onIntroduceTitleChange = {},
onExpoCreateCallBack = {},
trainingProgramTextState = emptyList(),
onTrainingProgramChange = { _, _ -> },
onAddTrainingProgram = {},
onRemoveTrainingProgram = {},
standardProgramTextState = emptyList(),
onStandardProgramChange = { _, _ -> },
onAddStandardProgram = {},
onRemoveStandardProgram = {}
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ internal fun ExpoDetailScreen(
}
}
}

if (openDialog) {
Dialog(onDismissRequest = { isOpenDialog(false) }) {
com.school_of_company.expo.view.component.MessageDialog(
Expand Down
Loading
Loading