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

[refactor] AddSubjectScreen 코드정리 #83

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ data class SubjectListInfo(
subjectId = subjectId,
subjectName = subjectName,
studyList =
studyList.filter { data ->
data.examName == "중간고사"
}.map {
it.toSubjectCardDetailTodoInfoEntity()
},
studyList.filter { data ->
data.examName == "중간고사"
}.map {
it.toSubjectCardDetailTodoInfoEntity()
},
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fun BbangZipDatePickerBottomSheet(
onClick = onClickInputButton,
label = "시험 일자 입력하기",
isEnable = true,
modifier = Modifier.fillMaxWidth()
modifier = Modifier.fillMaxWidth(),
)
Spacer(modifier = Modifier.height(16.dp))
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.SnackbarHostState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.navigation.compose.NavHost
import org.android.bbangzip.presentation.ui.friend.navigation.friendNavGraph
Expand All @@ -19,9 +21,11 @@ import org.android.bbangzip.presentation.ui.onboarding.navigation.onboardingNavG
import org.android.bbangzip.presentation.ui.onboarding.navigation.onboardingStartNavGraph
import org.android.bbangzip.presentation.ui.splash.navigation.splashNavGraph
import org.android.bbangzip.presentation.ui.subject.addstudy.addStudyNavGraph
import org.android.bbangzip.presentation.ui.subject.addstudy.navigateAddStudy
import org.android.bbangzip.presentation.ui.subject.addsubject.addSubjectNavGraph
import org.android.bbangzip.presentation.ui.subject.modify.motivationmessage.modifyMotivationMessageNavGraph
import org.android.bbangzip.presentation.ui.subject.modify.subjectname.modifySubjectNameNavGraph
import org.android.bbangzip.presentation.ui.subject.splitstudy.navigateSplitStudy
import org.android.bbangzip.presentation.ui.subject.splitstudy.splitStudyNavGraph
import org.android.bbangzip.presentation.ui.subject.subjectNavGraph
import org.android.bbangzip.presentation.ui.subject.subjectdetail.subjectDetailNavGraph
Expand All @@ -39,10 +43,10 @@ fun MainNavHost(
) {
Box(
modifier =
modifier
.padding(top = padding.calculateTopPadding())
.fillMaxSize()
.background(BbangZipTheme.colors.backgroundNormal_FFFFFF),
modifier
.padding(top = padding.calculateTopPadding())
.fillMaxSize()
.background(BbangZipTheme.colors.backgroundNormal_FFFFFF),
) {
NavHost(
navController = navigator.navHostController,
Expand Down Expand Up @@ -127,12 +131,12 @@ fun MainNavHost(
modifyMotivationMessageNavGraph(
navigateToSubjectDetail = { id, name -> navigator.navigateToSubjectDetail(id, name) },
snackbarHostState = snackBarHostState,
popBackStack = { navigator.popBackStackIfNotSubject() }
popBackStack = { navigator.popBackStackIfNotSubject() },
)

addSubjectNavGraph(
navigateSubject = { navigator.navigateToSubject() },
navigateToBack = { navigator.popBackStackIfNotSubject() }
navigateToBack = { navigator.popBackStackIfNotSubject() },
navigateToSubject = { navigator.navigateToSubject() },
)

splitStudyNavGraph(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class MainNavigator(
inclusive = true
}
launchSingleTop = true
}
},
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.android.bbangzip.presentation.ui.onboarding.onboardingstart.Onboardin
fun NavController.navigateOnboardingStart(navOptions: NavOptions) {
navigate(
route = OnboardingStartRoute,
navOptions = navOptions
navOptions = navOptions,
)
}

Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

p5 : 디테일 와캬퍄

Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private fun DefaultCardView(
verticalAlignment = Alignment.CenterVertically,
) {
Text(
text = stringResource(R.string.add_subject_title),
text = stringResource(R.string.subject_select_subject_title),
style = BbangZipTheme.typography.headline2Bold,
color = BbangZipTheme.colors.labelAlternative_282119_61,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fun AddStudyRoute(
is AddStudyContract.AddStudySideEffect.PopBackStack -> popBackStack()
is AddStudyContract.AddStudySideEffect.NavigateSubjectDetail -> navigateSubjectDetail(it.subjectId, it.subjectName)

is AddStudyContract.AddStudySideEffect.ShowSnackBar ->{
is AddStudyContract.AddStudySideEffect.ShowSnackBar -> {
val job =
launch {
snackBarHostState.currentSnackbarData?.dismiss()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,41 @@ class AddSubjectContract {
data class AddSubjectState(
val subjectName: String = "",
val isTextFieldFocused: Boolean = false,
val isButtonEnable: Boolean = false,
val subjectTextFieldState: BbangZipTextFieldInputState = BbangZipTextFieldInputState.Default,
val isButtonEnabled: Boolean = false,
val subjectTextFieldInputState: BbangZipTextFieldInputState = BbangZipTextFieldInputState.Default,
) : BaseContract.State, Parcelable {
override fun toParcelable(): Parcelable = this
}

sealed interface AddSubjectEvent : BaseContract.Event {
data class OnFocusTextField(val isTextFieldFocused: Boolean) : AddSubjectEvent
data class OnTextFieldFocus(val isTextFieldFocused: Boolean) : AddSubjectEvent

data class OnChangeSubjectName(val subjectName: String) : AddSubjectEvent
data class OnSubjectNameChange(val subjectName: String) : AddSubjectEvent

data object OnClickBackBtn : AddSubjectEvent
data object OnAddBtnClick : AddSubjectEvent

data object OnClickAddBtn : AddSubjectEvent
data object OnBackIconClick : AddSubjectEvent

data object OnClickDeleteBtn : AddSubjectEvent
data object OnTextFieldDeleteIconClick : AddSubjectEvent
}

sealed interface AddSubjectReduce : BaseContract.Reduce {
data class UpdateSubjectName(val subjectName: String) : AddSubjectReduce

data object UpdateIsButtonEnabled : AddSubjectReduce

data object UpdateSubjectInputState : AddSubjectReduce

data class UpdateSubjectName(val subjectName: String) : AddSubjectReduce

data class UpdateIsTextFieldFocused(val isTextFieldFocused: Boolean) : AddSubjectReduce

data object ResetSubjectName : AddSubjectReduce
}

sealed interface AddSubjectSideEffect : BaseContract.SideEffect {
data object NavigateSubjectDetail : AddSubjectSideEffect

// 공부 추가 완료! 미룬이 탈출이 코앞이에요
data class ShowSnackBar(val message: String) : AddSubjectSideEffect
data object NavigateToSubject : AddSubjectSideEffect

data object NavigateToBack : AddSubjectSideEffect

data class ShowSuccessAddSubjectSnackBar(val message: String) : AddSubjectSideEffect
Copy link
Contributor

Choose a reason for hiding this comment

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

p5 : 저희가 snackBar 함수를 사용할때 동일한 snackBar함수를 사용합니다. 특정 이유가 아니라면 ShowSnackBar함수 네이밍을 바꿀 필요가 없지않을까요?

Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 Feb 21, 2025

Choose a reason for hiding this comment

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

p5
하나의 스크린에서 다른 스낵바가 여러개 띄워질 수 있는 경우에 이런 식으로 사이드이펙트를 분리하고 싶다면, message를 인자로 주는 것이 아니라 그냥 고정된 걸로 사용하는게 분리의 측면에서 더 맞는 게 아닐까 싶어요
그게 아니라면 승범오빠 말대로 공통된 스낵바 네이밍을 사용하면서 메세지만 갈아끼우는 걸로 하는게 좋을 듯합니당

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ fun NavController.navigateToAddSubject() {
}

fun NavGraphBuilder.addSubjectNavGraph(
navigateSubject: () -> Unit,
navigateToBack:()->Unit
navigateToBack: () -> Unit,
navigateToSubject: () -> Unit,
) {
composable<AddSubjectRoute> {
AddSubjectRoute(
navigateSubjectDetail = navigateSubject,
navigateToBack = navigateToBack
navigateToBack = navigateToBack,
navigateToSubject = navigateToSubject,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,34 @@ import kotlinx.coroutines.flow.collectLatest

@Composable
fun AddSubjectRoute(
navigateToBack: () -> Unit,
navigateToSubject: () -> Unit,
viewModel: AddSubjectViewModel = hiltViewModel(),
navigateToBack:()->Unit ={},
navigateSubjectDetail: () -> Unit,
) {
val addSubjectState by viewModel.uiState.collectAsStateWithLifecycle()

LaunchedEffect(Unit) {
viewModel.uiSideEffect.collectLatest { effect ->
when (effect) {
AddSubjectContract.AddSubjectSideEffect.NavigateSubjectDetail -> {
navigateSubjectDetail()
AddSubjectContract.AddSubjectSideEffect.NavigateToSubject -> {
navigateToSubject()
}

is AddSubjectContract.AddSubjectSideEffect.ShowSnackBar -> {}

AddSubjectContract.AddSubjectSideEffect.NavigateToBack -> {
navigateToBack()
}

is AddSubjectContract.AddSubjectSideEffect.ShowSuccessAddSubjectSnackBar -> {}
}
}
}

AddSubjectScreen(
subjectName = addSubjectState.subjectName,
isButtonEnable = addSubjectState.isButtonEnable,
isTextFieldFocused = addSubjectState.isTextFieldFocused,
textFieldInputState = addSubjectState.subjectTextFieldState,
onSubjectNameChanged = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnChangeSubjectName(it)) },
onTextFieldFocusChanged = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnFocusTextField(it)) },
onAddBtnClicked = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnClickAddBtn) },
onDeleteBtnClicked = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnClickDeleteBtn) },
onBackButtonClicked = {viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnClickBackBtn)}
state = addSubjectState,
onSubjectNameChange = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnSubjectNameChange(it)) },
onTextFieldFocusChange = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnTextFieldFocus(it)) },
onAddBtnClick = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnAddBtnClick) },
onTextFieldDeleteIconClick = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnTextFieldDeleteIconClick) },
onBackIconClick = { viewModel.setEvent(AddSubjectContract.AddSubjectEvent.OnBackIconClick) },
)
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.android.bbangzip.presentation.ui.subject.addsubject

import android.app.Activity
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
Expand All @@ -9,55 +10,57 @@ import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.toArgb
import androidx.compose.ui.platform.LocalFocusManager
import androidx.compose.ui.platform.LocalView
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import org.android.bbangzip.R
import org.android.bbangzip.presentation.component.button.BbangZipButton
import org.android.bbangzip.presentation.component.textfield.BbangZipBasicTextField
import org.android.bbangzip.presentation.component.topbar.BbangZipBaseTopBar
import org.android.bbangzip.presentation.model.BbangZipTextFieldInputState
import org.android.bbangzip.presentation.type.BbangZipButtonSize
import org.android.bbangzip.presentation.type.BbangZipButtonType
import org.android.bbangzip.presentation.util.modifier.addFocusCleaner
import org.android.bbangzip.ui.theme.BbangZipTheme

private const val TEXT_FIELD_MAX_CHARACTER = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

p3 : 이 상수를 screen이 아닌 어디에 적을지 고민해보면 좋을 것 같습니다


@Composable
fun AddSubjectScreen(
subjectName: String = "",
isButtonEnable: Boolean = false,
isTextFieldFocused: Boolean = false,
textFieldInputState: BbangZipTextFieldInputState = BbangZipTextFieldInputState.Default,
onSubjectNameChanged: (String) -> Unit = {},
onTextFieldFocusChanged: (Boolean) -> Unit = {},
onAddBtnClicked: () -> Unit = {},
onDeleteBtnClicked: () -> Unit = {},
onBackButtonClicked: () -> Unit = {}
state: AddSubjectContract.AddSubjectState,
onSubjectNameChange: (String) -> Unit = {},
onTextFieldFocusChange: (Boolean) -> Unit = {},
onAddBtnClick: () -> Unit = {},
onTextFieldDeleteIconClick: () -> Unit = {},
onBackIconClick: () -> Unit = {},
) {
(LocalView.current.context as Activity).window.statusBarColor = BbangZipTheme.colors.backgroundNormal_FFFFFF.toArgb()
Copy link
Contributor

Choose a reason for hiding this comment

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

p5 : route가 아닌 screen에 선언하는 이유는 무엇인가요?


val focusManager = LocalFocusManager.current

Column(
modifier =
Modifier
.fillMaxSize()
.addFocusCleaner(focusManager = focusManager),
Modifier
.fillMaxSize()
.addFocusCleaner(focusManager = focusManager),
) {
BbangZipBaseTopBar(
title = "과목 추가하기",
title = stringResource(R.string.add_subject_title),
leadingIcon = R.drawable.ic_chevronleft_thick_small_24,
onLeadingIconClick = onBackButtonClicked
onLeadingIconClick = onBackIconClick,
)

Column(
modifier =
Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp)
.padding(top = 48.dp, bottom = 20.dp),
Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp)
.padding(top = 48.dp, bottom = 20.dp),
) {
Text(
text = "과목명",
text = stringResource(R.string.add_subject_textfield_label),
style = BbangZipTheme.typography.headline1Bold,
color = BbangZipTheme.colors.labelNormal_282119,
)
Expand All @@ -68,13 +71,13 @@ fun AddSubjectScreen(
leadingIcon = R.drawable.ic_book_default_24,
placeholder = R.string.modify_subject_name_placeholder,
guideline = R.string.modify_subject_name_guideline,
value = subjectName,
value = state.subjectName,
modifier = Modifier.fillMaxWidth(),
bbangZipTextFieldInputState = textFieldInputState,
onValueChange = onSubjectNameChanged,
onFocusChange = onTextFieldFocusChanged,
onDeleteButtonClick = onDeleteBtnClicked,
maxCharacter = 10,
bbangZipTextFieldInputState = state.subjectTextFieldInputState,
onValueChange = onSubjectNameChange,
onFocusChange = onTextFieldFocusChange,
onDeleteButtonClick = onTextFieldDeleteIconClick,
maxCharacter = TEXT_FIELD_MAX_CHARACTER,
focusManager = focusManager,
)

Expand All @@ -84,11 +87,11 @@ fun AddSubjectScreen(
bbangZipButtonSize = BbangZipButtonSize.Large,
bbangZipButtonType = BbangZipButtonType.Solid,
onClick = {
onAddBtnClicked()
onAddBtnClick()
},
modifier = Modifier.fillMaxWidth(),
label = stringResource(R.string.btn_add_label),
isEnable = isButtonEnable,
isEnable = state.isButtonEnabled,
)
}
}
Expand All @@ -98,7 +101,6 @@ fun AddSubjectScreen(
@Composable
private fun AddSubjectScreenPreview() {
AddSubjectScreen(
"Preview",
false,
state = AddSubjectContract.AddSubjectState(),
)
}
Loading
Loading