Feature/jaino/#85 #103

jeongjaino · 2024-01-13T05:54:38Z

1. 📄 관련된 이슈 및 소개

#85 설문탭에서 작성된 설문 열람 기능 구현

2. 🔥변경된 점

기존 관리탭에서 설문 확인 -> 설문탭에서 설문 확인되도록 수정

작성된 설문 리스트 화면 구현,

설문탭 -> 작성된 설문 목록 -> 자세한 설문 내용 확인 으로 구성

survey -> survey-check -> survey-detail

기존 survey-check 네이밍이 -> survey-detail로 변경되었습니다
survey-check 네이밍은 설문 목록 화면으로 구성하였습니다.

3. 📸 스크린샷(선택)

(회원인 경우 )

(운영진)

(설문 목록화면)

(설문 확인 화면)

4. 💡알게된 혹은 궁금한 사항들

추후 구현사항

설문 목록 정렬 기능 (설문에 따라, 시간에 따라 )
현재 이벤트가 더미데이터로 매핑되어 있음. -> 이벤트조회 로직구현되면 그에따라 더미 -> 실 데이터로 변경

…l / survey-detail -> survey-check 화면 전환 구현

tgyuuAn

진호상 약간의 잔잔바리들 코멘트 남겨봤습니다.!!

크게 고쳐야할 점은 없네요 아주 나이쓰입니다!

tgyuuAn · 2024-01-13T06:11:38Z

feature/survey-check/src/main/java/com/wap/wapp/feature/survey/check/SurveyCheckScreen.kt

 ) {
-    val surveyUiState by viewModel.surveyUiState.collectAsStateWithLifecycle()
+    val surveyListUiState = viewModel.surveyListUiState.collectAsStateWithLifecycle().value


진호상 여기 by를 소거하고 다시 value를 붙인 이유가 있을까요?!

이게 파일 이름을 변경하고, 새로운 파일을 이전 파일 이름으로 변경했더니,

새로 추가된 내용인데, 변경사항으로 나오는군요 캬캬 ,,

따로 소거한게 아니라, 새로 구현한 사항입니다 캬캬,,, 하하하하하하

@jeongjaino

아하 그런거라면 value를 소거하고 by만 붙여줘도 될 것 같아요!

tgyuuAn · 2024-01-13T06:13:13Z

feature/survey-check/src/main/java/com/wap/wapp/feature/survey/check/SurveyCheckViewModel.kt

                }
                .onFailure { throwable ->
-                    _surveyCheckUiEvent.emit(SurveyUiEvent.Failure(throwable))
+                    _errorFlow.emit(throwable)
                }


스코프 하나 제거할 수 있을 것 같아요.

getSurveyListUseCase().onSuccess { surveyList -> // <- 요쪽 _surveyListUiState.value = SurveyListUiState.Success(surveyList) }

하면 개행해야하는 너비인가요?!

혹시 어떤 스코프 말씀하신건가요 ,, ?

onSuccess 개행 말씀하신건가요 ??

컨벤션에 메소드 체이닝시, 첫 줄은 붙어있길래 코멘트 달아봤씁니다...!

캬캬 해당 컨벤션에 코드에는 이어져 있긴한데,

사실 큰 의미는 개행이 필요한 경우, 무조건 한줄에 한 체인만 설정하도록 하는게 주 의미라고 생각해요

개인적으로 바로 onSuccess를 붙이는게 가독성도 좋지않고,
onFailure랑 너무 떨어진 느낌이 들어서 개인적으로 무조건 다음줄부터 개행하는걸 좋아해요 ...

@jeongjaino

아하 저 부분에 대해서는 굳이 철저히 지키지 않아도 되는 컨벤션일까요 ?.?

사실 지금도 섞여있긴 해서

저는 저 컨벤션을 알고 쓰고 있고, 어긴 사례는 제가 생각나는 것중에는 없어요 !

해당 컨벤션이 말 그대로 체이닝을 할떄 한 줄로 안되면 무조건 개행하라. 가 주 목적이라

useCase().onSuceess()
.onFailure().onRestore() -> 이렇게 하는게 안되는부분이고

useCase()
.onSuccess()
.onFailure() -> 이건 가능해요

tgyuuAn · 2024-01-13T06:13:50Z

...re/survey-check/src/main/java/com/wap/wapp/feature/survey/check/detail/SurveyDetailScreen.kt

+    LaunchedEffect(true) {
+        viewModel.getSurvey(surveyId)
+    }
+
+    LaunchedEffect(true) {
+        viewModel.errorFlow.collectLatest {
+            snackBarHostState.showSnackbar(it.toSupportingText())
+        }
+    }


둘 다 한 번만 호출이 되는 거라면 분리하신 이유가 궁금해요!

단순히 로직 뷰모델 호출 로직 분리 때문에 나뒀어요 !

키 값이 서로 true라 서로 같은 스코프에 나둬도 차이는 없을 것 같아요 !

tgyuuAn · 2024-01-13T06:14:51Z

feature/survey/src/main/java/com/wap/wapp/feature/survey/SurveyContent.kt

+                Text(
+                    text = "작성된 설문 보러가기",
+                    style = WappTheme.typography.contentRegular,
+                )
+
+                Icon(
+                    painter = painterResource(id = R.drawable.ic_magnifier),
+                    contentDescription = "작성된 설문 보러가기 아이콘",
+                )


이 부분 귀찮더라도 Strings.xml로 빼주시면 좋을 것 같아요!

호우 있는지도 몰랐네요 캬캬 바로 소거하겠습니다 !

tgyuuAn · 2024-01-13T06:15:07Z

feature/survey/src/main/java/com/wap/wapp/feature/survey/SurveyFormItemCard.kt

+
+package com.wap.wapp.feature.survey
+


허걱 여기 첫 행에 개행이 들어가있어요!

오우예~~ 매의눈 티지와이유유

tgyuuAn · 2024-01-13T06:15:47Z

feature/survey/src/main/java/com/wap/wapp/feature/survey/SurveyScreen.kt

+                        UserRole.MEMBER -> {
                            viewModel.getSurveyFormList()
                        }


스코프 제거할 수 있을 것 같아요!

좋습니다잇~~

jeongjaino added 11 commits January 13, 2024 14:44

[FEATURE] #85 : 매니저인 경우, survey-check로 전환하는 버튼 구현

06ff210

[FEATURE] #85 : 매니저인 경우, survey-check로 전환하는 버튼 구현

272b902

[CHORE] #85 : surveyScreen 컴포넌트 분리

5860a97

[FEATURE] #85 : 작성된 설문 리스트 불러오는 로직 구현

6d6b391

[FEATURE] #85 : 설문 확인 화면 구현

aab285c

[UI] #85 : 작성된 설문 목록 아이템 구현

b4fd607

[CHORE] #85 : Survey-check -> survey-check/detail 로 path 수정

4ad4f4a

[FEATURE] #85 : Survey -> survey-check / survey-check -> survey-detai…

c9834e9

…l / survey-detail -> survey-check 화면 전환 구현

[FEATURE] #85 : 설문 확인 모듈 네비게이션 구현

aeb5698

[FEATURE] #85 : 설문 확인 모듈 네비게이션 구현

c4dd5ce

[CHORE] #85 : Image/String Resource 추가

41e811a

jeongjaino added 🚀진호🚀 ESTP 정진호 23세 🌱기능🌱 새로운 기능 두두둥장! 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다. labels Jan 13, 2024

jeongjaino self-assigned this Jan 13, 2024

tgyuuAn approved these changes Jan 13, 2024

View reviewed changes

tgyuuAn added ✏️수정 요청✏️ 코드 리뷰후 코드 수정 요청 and removed 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다. labels Jan 13, 2024

jeongjaino added 3 commits January 13, 2024 16:15

[CHORE] #85 : Raw -> String Resource 수정

53639d2

[STYLE] #85 : 빈 개행 수정

ea18423

[STYLE] #85 : 불필요 스코프 소거

da4b8ee

tgyuuAn added 🌟머지 해주세요🌟 코드 리뷰가 완료된 뒤 PR을 올린사람이 Merge를 하면 되는 단계입니다. and removed ✏️수정 요청✏️ 코드 리뷰후 코드 수정 요청 labels Jan 13, 2024

jeongjaino merged commit 7024694 into develop Jan 13, 2024

jeongjaino linked an issue Jan 13, 2024 that may be closed by this pull request

[FEATURE]: (운영진) 설문 탭에서 작성된 설문 열람 구현 #85

Closed

3 tasks

tgyuuAn deleted the feature/jaino/#85 branch February 6, 2024 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/jaino/#85 #103

Feature/jaino/#85 #103

jeongjaino commented Jan 13, 2024 •

edited

Loading

tgyuuAn left a comment

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024 •

edited

Loading

tgyuuAn Jan 13, 2024

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024 •

edited

Loading

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024 •

edited

Loading

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024

tgyuuAn Jan 13, 2024

jeongjaino Jan 13, 2024


		package com.wap.wapp.feature.survey

Feature/jaino/#85 #103

Feature/jaino/#85 #103

Conversation

jeongjaino commented Jan 13, 2024 • edited Loading

1. 📄 관련된 이슈 및 소개

2. 🔥변경된 점

3. 📸 스크린샷(선택)

4. 💡알게된 혹은 궁금한 사항들

tgyuuAn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeongjaino Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeongjaino Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeongjaino Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeongjaino commented Jan 13, 2024 •

edited

Loading

jeongjaino Jan 13, 2024 •

edited

Loading

jeongjaino Jan 13, 2024 •

edited

Loading

jeongjaino Jan 13, 2024 •

edited

Loading