[FEAT/#254] 리뷰 수정 UI만들었어요 by angryPodo · Pull Request #257 · spooooony/Spoony-Android

angryPodo · 2025-04-29T16:16:31Z

Related issue 🛠

closed [FEAT] 리뷰 수정 만들어요 #254

Work Description ✏️

등록하기, 리뷰 수정하기 나눴어요
사실 등록하기 리펙인,,ㅋㅋ

Screenshot 📸

Screen_recording_20250430_011317.mp4

Uncompleted Tasks 😅

서버 붙이기

To Reviewers 📢

사실상 등록하기 리펙을 완료했습니다.
어쩔수 없이 saveState를 NavOpt에서 뺐지만 오히려 좋다랄까요?
서버 통신은 등록하기 재탕이라서...

- `RegisterType` enum 클래스를 추가하여 등록과 수정 시나리오 구분. - 리뷰 수정 시 '다음' 버튼 텍스트를 '리뷰 수정'으로 변경. - `RegisterStepOneScreen`, `RegisterStepTwoScreen`에서 `RegisterType`을 사용하여 UI 및 로직 분기 처리. - `SpoonySearchTextField`에서 클리너 아이콘 표시 여부 제거 및 텍스트가 있을 때만 표시하도록 수정. - `SearchResultItem`에 클리너 아이콘 표시 여부 변수 추가. - 마이페이지에서 리뷰 수정 버튼 추가 및 `navigateToEditReview` 네비게이션 함수 추가. - `Register` 네비게이션 route에 `RegisterType` 및 `postId` 추가. - `RegisterViewModel`에 `registerType`, `postId` 추가 및 리뷰 수정 로직 초안 추가. - 메인 탭 바에서 등록 탭을 누를 때 `RegisterType`을 `CREATE`로 설정해서 전달하도록 변경.

- `MainNavigator`에서 `navigate` 메서드에 `navOptions`를 전달하도록 변경하고, 각 탭별로 다른 `navOptions`를 사용하도록 수정합니다. - `MainTab.REGISTER`에 대한 `navOptions`는 `NAVIGATION_ROOT`로 팝업하도록 설정됩니다. - 하단바 노출 여부를 결정하는 `shouldShowBottomBar` 메서드에서 `currentDestination`이 null이 아닐 때만 로직을 실행하도록 개선합니다. - `RegisterViewModel`에서 `loadState` 함수를 추가하여 `RegisterType`이 `EDIT`일 때 기존 데이터를 불러오도록 구현합니다. - `RegisterScreen`에서 `LaunchedEffect`를 사용하여 `viewModel.loadState()`를 호출하도록 추가합니다.

`SpoonyIconButtonTextField`의 `imeAction`을 `Done`에서 `Next`로 변경합니다.

Hyobeen-Park

수고하셨습니다~~!! 네비게이터가 많이 바뀌었네요ㅎㅎ 조금 고민되는 부분들이 있었는데 리뷰로 달아두었으니 확인 부탁드려요😊

Hyobeen-Park · 2025-04-30T15:03:42Z

+@Composable
+private fun RegisterScreen(
+    paddingValues: PaddingValues,
+    modifier: Modifier = Modifier,


P1: 잡았다!!

ㅋㅋㅋㅋ먼가했어요

Hyobeen-Park · 2025-04-30T15:04:51Z

    onPlaceClear: () -> Unit,
    onDismissSearchResults: () -> Unit,
-    modifier: Modifier = Modifier
+    modifier: Modifier = Modifier,


P1: 잡았다!!

Hyobeen-Park · 2025-04-30T15:10:17Z

    val state by viewModel.state.collectAsStateWithLifecycle()
-    val focusManager = LocalFocusManager.current
+    val registerType by viewModel.registerType.collectAsStateWithLifecycle()
    var isDialogVisible by remember { mutableStateOf(false) }


P2: 이거는 screen 내부에서 관리해주는거 어때요~~??

Hyobeen-Park · 2025-04-30T15:12:41Z

+    isNextButtonEnabled: Boolean,
+    isDialogVisible: Boolean,
+    onDetailReviewChange: (String) -> Unit,
+    onPhotosSelected: (List<SelectedPhoto>) -> Unit,


P5: 이거는 immutableList로 안해도 되려나요?!??

상관은 없는데 좋은게 좋은거니 반영할게요~

Hyobeen-Park · 2025-04-30T15:18:20Z

+    private val _registerType = MutableStateFlow(RegisterType.CREATE)
+    val registerType: StateFlow<RegisterType>
+        get() = _registerType.asStateFlow()


P2: registerType은 중간에 바뀌지 않고 처음 진입했을 때부터 정해져있는 거라서 mutableStateFlow로 구현하지 말고 value로 해도 될 것 같은데 어떻게 생각하시나요??

반영하겟습니다

Hyobeen-Park · 2025-04-30T15:20:05Z

+
+            Button(
+                modifier = Modifier,
+                onClick = { navigateToEditReview(0, RegisterType.EDIT) }
+            ) {
+                Text(text = "Edit Review")
+            }


P1: 나중에 꼬옥 지워야 해..!!

Hyobeen-Park · 2025-04-30T15:25:49Z

+    private val mainTabNavOptions = navOptions {
+        navController.currentDestination?.route?.let {
+            popUpTo(it) {
+                inclusive = true
+                saveState = true
+            }
+        }
+        launchSingleTop = true
+        restoreState = true
+    }


P3: saveState를 제외한 나머지 옵션이 동일해서 saveState 여부를 매개변수로 받아오면 더 깔끔해지지 않을까 싶어요!

맞아요 그것도 고민했는데, 기획이 휙휙바뀌는 느낌이 강해서 확장성있게 좀 열어놨습니다. 언제 또 기획이 바껴서 옵션이 달라질지 모르니..

Hyobeen-Park · 2025-04-30T15:27:23Z

+    }
+
+    private fun MainTab.getNavOptions(): NavOptions = when (this) {
+        MainTab.REGISTER -> navOptions {


P3: 위에서 말한대로 하면 mainTabNavOptions(false)이렇게 정리할 수 있어서요!!

저는 현재 작성되어있는 것 처럼 작성하는게 더 좋을거 같습니당!
현재 방법이 작성해야 할 코드는 더 많지만, 추후 옵션들이 하나씩 변경된다면 매번 매개변수를 만들 수는 없을테니까요 :)

흠 사실 메인탭에만 해당되는 navOption이라 매개변수 받을 일이 많이 없을 것 같아서 드린 말씀이었는데요! 지금 기획이 계속 바뀌고 있어서 오빠 말대로 그냥 두는게 나을 것 같네요ㅎㅎ

Hyobeen-Park · 2025-04-30T15:30:19Z

+        restoreState = true
+    }
+
+    private fun MainTab.getNavOptions(): NavOptions = when (this) {


P2: 흠 이 부분이 여기에 있어야 하는지 MainTab 안에 있어야 하는지 고민했는데요 MainTab 내부에 구현하는 것도 좋아보여서 슬쩍 말씀드려봅니당

흠....MainTab 클래스가 navOption에 대한 의존성을 가지는게 저는 어색한거같아요 어떻게 생각하시는지요? ㅎㅎ

오호라 개큰인정 드립니다ㅋㅋ

MainTab 내부에 구현하면 MainTab에 관련된 내용이 한 곳에 모여있고 유지보수나 확장성 측면에서 더 편할 수 있다고 생각했는데요

민재오빠 말대로 외부에 구현하는 것이 책임이 더 명확히 분리되어 있는 것 같아요! navigator에 두죠?? 다시 생각해보니 그게 더 나은듯ㅋㅋㅋㅋ

- `RegisterViewModel`에서 `registerType`에 대한 `MutableStateFlow`를 제거하고 단순 변수로 변경. - `RegisterStepOneScreen`, `RegisterStepTwoScreen`에서 `registerType`을 `collectAsStateWithLifecycle`로 수집하지 않고 변수로 직접 참조하도록 수정. - `RegisterViewModel.loadState()`에서 `_registerType.value`로 설정하는 부분을 `registerType`으로 변경. - `registerType`의 값으로 비교하는 부분 수정.

chattymin

LGTM! 고생하셨습니다.
이번 코리에서는 코드에서 컨텍스트가 미치는 관리의 난이도와 클래스의 역할에 대해서 한번 생각해보면 좋을 것 같아요. 물론 현재 코드도 문제 없으니 Approve는 남기겠습니다 :)

chattymin · 2025-05-01T06:57:48Z

+    }
+
+    private fun MainTab.getNavOptions(): NavOptions = when (this) {
+        MainTab.REGISTER -> navOptions {


저는 현재 작성되어있는 것 처럼 작성하는게 더 좋을거 같습니당!
현재 방법이 작성해야 할 코드는 더 많지만, 추후 옵션들이 하나씩 변경된다면 매번 매개변수를 만들 수는 없을테니까요 :)

chattymin · 2025-05-01T06:59:37Z

+    RegisterScreen(
+        paddingValues = paddingValues,
+        modifier = modifier,
+        state = state,
+        isTooltipVisible = isTooltipVisible,
+        navController = navController,
+        navigateToExplore = navigateToExplore,
+        onUpdateProgress = viewModel::updateStep,
+        onResetRegisterState = viewModel::resetState,
+        hideRegisterSnackBar = viewModel::hideRegisterSnackBar,
+        viewModel = viewModel
+    )


p5
진짜 진짜 사소하긴 한데 저는 선언 순서와 가능한 맞추는 편입니다.
그래야 나중에 값 수정할때 편하더라구여

chattymin · 2025-05-01T07:01:37Z

 @Composable
-fun RegisterStepTwoScreen(
-    onStepTwoComplete: () -> Unit,
+fun RegisterStepTwoRoute(


p3
이름이 마음에 안들어용...
one과 two 사이에 단계가 추가된다면???

chattymin · 2025-05-01T07:07:45Z

            TopLinearProgressBar(
                currentStep = state.currentStep,
-                totalSteps = 3f,
+                totalSteps = 2f,


p3
경로가 늘어난다면 이 부분도 수정이 들어가야할 것 같네요.
하나의 값이 변경되었을때 그것과 연관되어 같이 변경이 되어야 한다면 추후 관리의 리소스가 커요. Register 단계에서 사용되는 navigation class를 만들고, 내부에서 경로 관리와 화면 이동 등의 역할을 모두 담당하게 하는건 어떨까요? 그 후 해당 네비게이션에서 이동 경로들을 가지고 있고, 그 값을 여기에 넣어주는 방향성도 좋을 것 같아요.

물론 지금 방법이 문제있는 것은 아니지만, "경로가 추가되었을 때" -> "totalSteps를 늘려야 한다." 라는 컨텍스트가 있다는게 추후 문제를 발생시킬 수 있다고 생각해요 :)

Roel4990

다른 분들이 리뷰 잘 달아주셨네요.. 복잡한 Nav 깔끔하게 구현하셨네요. 잘 배워갑니다. 수고하셨습니다!

Hyobeen-Park

어푸푸🚀🚀

…dit-ui # Conflicts: # app/src/main/java/com/spoony/spoony/presentation/main/MainNavigator.kt

- 등록 화면의 내비게이션 그래프를 수정하여 RegisterStartRoute와 RegisterEndRoute로 변경했습니다. - NavHost의 시작 지점을 RegisterRoute.Start로 변경했습니다. - RegisterRoute.Start에서 RegisterRoute.End로 이동하고, RegisterRoute.End에서 완료 시 RegisterRoute.Start까지 popBackStack 하도록 수정했습니다. - RegisterRoute.Start 및 RegisterRoute.End 컴포저블 함수의 이름을 변경했습니다. - RegisterStartScreen 및 RegisterEndScreen 컴포저블 함수의 이름을 변경했습니다. - RegisterRoute를 sealed class로 변경했습니다. - MainNavigator에서 Register 탭 선택 시 navOptions를 수정했습니다. - MainScreen에서 registerNavGraph에 navigateUp 람다를 전달하도록 수정했습니다. - RegisterRoute에서 TopAppBar를 추가하고 ProgressBar의 패딩을 조정했습니다.

angryPodo added 5 commits April 28, 2025 13:38

[REF/#254] 기존 Register 리펙토링

3c58bda

[REF/#254] 기존 Register 리펙토링

e45e8d0

[MOD/#254] lint format

66e2cf6

angryPodo added 💙민재💙 💙민재💙 FEAT✨ 새로운 기능 구현 labels Apr 29, 2025

angryPodo added this to the 2차 스프린트 milestone Apr 29, 2025

angryPodo self-assigned this Apr 29, 2025

angryPodo requested a review from a team as a code owner April 29, 2025 16:16

angryPodo requested review from Hyobeen-Park, Roel4990 and chattymin and removed request for a team April 29, 2025 16:16

angryPodo linked an issue Apr 29, 2025 that may be closed by this pull request

[FEAT] 리뷰 수정 만들어요 #254

Closed

2 tasks

[MOD/#254] SpoonyIconButtonTextField ImeAction 변경

b7b8bec

`SpoonyIconButtonTextField`의 `imeAction`을 `Done`에서 `Next`로 변경합니다.

Hyobeen-Park reviewed Apr 30, 2025

View reviewed changes

angryPodo added 2 commits May 1, 2025 14:31

[MOD/#254] 리뷰 1차 반영

a3ac4f4

chattymin approved these changes May 1, 2025

View reviewed changes

Roel4990 approved these changes May 2, 2025

View reviewed changes

Hyobeen-Park approved these changes May 3, 2025

View reviewed changes

angryPodo added 2 commits May 6, 2025 19:16

Merge remote-tracking branch 'origin/develop' into feat/#254-review-e…

6144796

…dit-ui # Conflicts: # app/src/main/java/com/spoony/spoony/presentation/main/MainNavigator.kt

angryPodo merged commit 5a5500e into develop May 6, 2025
1 check passed

Conversation

angryPodo commented Apr 29, 2025

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

Uh oh!

Hyobeen-Park left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angryPodo May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chattymin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Roel4990 left a comment

Choose a reason for hiding this comment

Uh oh!

Hyobeen-Park left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

angryPodo May 1, 2025 •

edited

Loading

chattymin left a comment •

edited

Loading