[FEAT/#230] 여행 조회, 여행 수정, 여행 나가기 뷰 / 서버통신 구현 #241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chattymin
approved these changes
Mar 9, 2024
Comment on lines
45
to
47
| viewModel.patchQuitTripFromServer() | ||
| val intent = Intent(requireActivity(), DashBoardActivity::class.java) | ||
| startActivity(intent) |
There was a problem hiding this comment.
아차차 맞다! 불필요한 객체 추가 하지 않기.. 명심하겠습니다 🤓
Comment on lines
68
to
74
| fun splitDate(date: String): Triple<Int, Int, Int> { | ||
| val parts = date.split(".") | ||
| val year = parts[0].toInt() | ||
| val month = parts[1].toInt() | ||
| val day = parts[2].toInt() | ||
| return Triple(year, month, day) | ||
| } |
Comment on lines
128
to
129
| val intent = Intent(this, DashBoardActivity::class.java) | ||
| startActivity(intent) |
Comment on lines
32
to
53
| var currentStartYear = MutableLiveData<Int>() | ||
| var currentStartMonth = MutableLiveData<Int>() | ||
| var currentStartDay = MutableLiveData<Int>() | ||
| var currentEndYear = MutableLiveData<Int>() | ||
| var currentEndMonth = MutableLiveData<Int>() | ||
| var currentEndDay = MutableLiveData<Int>() | ||
|
|
||
| val startYear = MutableLiveData<Int>() | ||
| val startMonth = MutableLiveData<Int>() | ||
| val startDay = MutableLiveData<Int>() | ||
| var startDate = MutableLiveData<String>() | ||
|
|
||
| val endYear = MutableLiveData<Int>() | ||
| val endMonth = MutableLiveData<Int>() | ||
| val endDay = MutableLiveData<Int>() | ||
| var endDate = MutableLiveData<String>() | ||
|
|
||
| val isStartDateAvailable = MutableLiveData(false) | ||
| val isEndDateAvailable = MutableLiveData(false) | ||
|
|
||
| val isTitleAvailable = MutableStateFlow(false) | ||
| var isCheckTripAvailable = MutableLiveData(false) |
There was a problem hiding this comment.
액티비티에서 이친구들을 전부 다 옵저브하고있지 않은데 LiveData로 쓴 이유가 있을까욤?
There was a problem hiding this comment.
맞아요 옵저브나 데이터 바인딩에서 활용하지 않는 단순 저장값들은 그냥 라이브데이터 대신 var 또는 val 로 활용하는 것이 좋아보여요 ~
There was a problem hiding this comment.
조언 감사합니둥! 수정하겟숩니다!
Marchbreeze
approved these changes
Mar 9, 2024
Comment on lines
+6
to
+18
| interface EditTripRepository { | ||
|
|
||
| suspend fun getTripInfo( | ||
| tripId: Long | ||
| ): Result<TripInfoModel> | ||
| suspend fun patchEditTripInfo( | ||
| tripId: Long, | ||
| request: EditTripRequestModel | ||
| ): Result<Unit> | ||
| suspend fun patchQuitTrip( | ||
| tripId: Long | ||
| ): Result<Unit> | ||
| } |
There was a problem hiding this comment.
한번에 서버통신 3개 처리해버리는 그녀... 실력 말안됨
Comment on lines
+53
to
+56
| tvEditTripName.text = viewModel?.title | ||
| tvEditTripInfoStartDate.text = viewModel?.startDate | ||
| tvEditTripInfoEndDate.text = viewModel?.endDate | ||
| viewModel?.gettitleLength() |
There was a problem hiding this comment.
요 친구들 ?를 넣지 않으면 실행할 때 널러블이 있을 수 있다는 오류가 떠서 넣어주었습니다!
Comment on lines
+74
to
+76
| setResult(Activity.RESULT_OK) | ||
| finish() | ||
| return@onEach |
| viewModel.currentEndDay.value ?: calendar.get(Calendar.DAY_OF_MONTH) | ||
| ) | ||
| } | ||
| if (viewModel.startYear.value != viewModel.currentStartYear.value || viewModel.startMonth.value != viewModel.currentStartMonth.value || viewModel.startDay.value != viewModel.currentStartDay.value) { |
There was a problem hiding this comment.
이 로직들은 뷰모델 내부에서 진행해줘도 좋을 것 같아요 !
| maxDate = Calendar.getInstance().apply { | ||
| set(2100, 0, 1) | ||
| }.timeInMillis | ||
| val calendar = Calendar.getInstance() |
There was a problem hiding this comment.
영상보니깐 바텀시트 값들이 2024 / 3 / 10 이 아니라 다른 형식이던데, 수정된 이유가 있었을까요?
There was a problem hiding this comment.
바텀시트에 뜨는 날짜 말씀하시는 거라면 영어로 뜨는 이유는 아마두 애뮬이 영어로 되어 있어서 그런 것 같숩니다.. 실기기에서는 원래 바텀시트 데이트피커 나오는 것(2024 3 10) 처럼 나옵니다!!
Comment on lines
55
to
65
| val (startYear, startMonth, startDay) = splitDate(viewModel.currentStartDate) | ||
| viewModel.currentStartYear.value = startYear | ||
| viewModel.currentStartMonth.value = startMonth | ||
| viewModel.currentStartDay.value = startDay | ||
| viewModel.setStartDate(startYear, startMonth, startDay) | ||
|
|
||
| val (endYear, endMonth, endDay) = splitDate(viewModel.currentEndDate) | ||
| viewModel.currentEndYear.value = endYear | ||
| viewModel.currentEndMonth.value = endMonth | ||
| viewModel.currentEndDay.value = endDay | ||
| viewModel.setEndDate(endYear, endMonth, endDay) |
There was a problem hiding this comment.
요 친구들도 뷰모델 내부에서 진행해줄 수 있을 것 같아요 !
Comment on lines
140
to
143
| const val TRIP_ID = "TRIP_ID" | ||
| const val TITLE = "TITLE" | ||
| const val START_DATE = "START_DATE" | ||
| const val END_DATE = "END_DATE" |
There was a problem hiding this comment.
사실 다 같아서 성능에 영향은 없지만, 캡슐화를 제대로 해주려면 private 달아주면 더 인텐트 진행을 숨겨줄 수 있어요! 잘해보이는 꿀팁 ㅋㅋ
Comment on lines
32
to
53
| var currentStartYear = MutableLiveData<Int>() | ||
| var currentStartMonth = MutableLiveData<Int>() | ||
| var currentStartDay = MutableLiveData<Int>() | ||
| var currentEndYear = MutableLiveData<Int>() | ||
| var currentEndMonth = MutableLiveData<Int>() | ||
| var currentEndDay = MutableLiveData<Int>() | ||
|
|
||
| val startYear = MutableLiveData<Int>() | ||
| val startMonth = MutableLiveData<Int>() | ||
| val startDay = MutableLiveData<Int>() | ||
| var startDate = MutableLiveData<String>() | ||
|
|
||
| val endYear = MutableLiveData<Int>() | ||
| val endMonth = MutableLiveData<Int>() | ||
| val endDay = MutableLiveData<Int>() | ||
| var endDate = MutableLiveData<String>() | ||
|
|
||
| val isStartDateAvailable = MutableLiveData(false) | ||
| val isEndDateAvailable = MutableLiveData(false) | ||
|
|
||
| val isTitleAvailable = MutableStateFlow(false) | ||
| var isCheckTripAvailable = MutableLiveData(false) |
There was a problem hiding this comment.
맞아요 옵저브나 데이터 바인딩에서 활용하지 않는 단순 저장값들은 그냥 라이브데이터 대신 var 또는 val 로 활용하는 것이 좋아보여요 ~
| @@ -57,11 +58,11 @@ | |||
| app:canBlankError="true" | |||
| app:hint="@string/edit_trip_info_et_name_hint" | |||
| app:layout_constraintEnd_toEndOf="parent" | |||
| app:layout_constraintHorizontal_bias="0.0" | |||
| android:gravity="center" | ||
| android:paddingHorizontal="12dp" | ||
| android:enabled="@{viewModel.isCheckTripAvailable()}" | ||
| android:outlineProvider="none" |
There was a problem hiding this comment.
버튼 그림자 효과를 제거해주는 친구라고 알고 있습니다! 앱잼 기간 때 버튼 그림자 없애라고 했었어서 요번에도 추가했습니다!
leeeyubin
approved these changes
Mar 10, 2024
Comment on lines
+6
to
+18
| interface EditTripRepository { | ||
|
|
||
| suspend fun getTripInfo( | ||
| tripId: Long | ||
| ): Result<TripInfoModel> | ||
| suspend fun patchEditTripInfo( | ||
| tripId: Long, | ||
| request: EditTripRequestModel | ||
| ): Result<Unit> | ||
| suspend fun patchQuitTrip( | ||
| tripId: Long | ||
| ): Result<Unit> | ||
| } |
Comment on lines
+113
to
+116
| override fun onDestroy() { | ||
| super.onDestroy() | ||
| if (quitDialog?.isAdded == true) quitDialog?.dismiss() | ||
| } |
Comment on lines
68
to
74
| fun splitDate(date: String): Triple<Int, Int, Int> { | ||
| val parts = date.split(".") | ||
| val year = parts[0].toInt() | ||
| val month = parts[1].toInt() | ||
| val day = parts[2].toInt() | ||
| return Triple(year, month, day) | ||
| } |
Marchbreeze
approved these changes
Mar 10, 2024
Comment on lines
+54
to
+55
| viewModel.splitStartDate() | ||
| viewModel.splitEndDate() |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
⛳️ Work Description
📸 Screenshot
pr_seyeon.mp4
📢 To Reviewers