[FEAT/#212] 여행 정보 수정 뷰 / UI 구현 #215
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
…into feat/#212-edit-trip-info
chattymin
approved these changes
Feb 28, 2024
Comment on lines
69
to
83
| private fun sendDateInfo() { | ||
| if (isStart) { | ||
| viewModel.startYear.value = binding.dpEditTripDate.year | ||
| viewModel.startMonth.value = binding.dpEditTripDate.month + 1 | ||
| viewModel.startDay.value = binding.dpEditTripDate.dayOfMonth | ||
| viewModel.checkStartDateAvailable() | ||
|
|
||
| } else { | ||
| customEndDate() | ||
| viewModel.endYear.value = binding.dpEditTripDate.year | ||
| viewModel.endMonth.value = binding.dpEditTripDate.month + 1 | ||
| viewModel.endDay.value = binding.dpEditTripDate.dayOfMonth | ||
| viewModel.checkEndDateAvailable() | ||
| } | ||
| } |
There was a problem hiding this comment.
묘하게 viewModel이 자주 나오지 않나요?
with로 묶어도 됩니다!
하지만, viewModel의 값을 acivity나 fragment에서 직접 조작하는 것 보다는 viewModel 내부 함수를 활용한다면 더욱 좋겠죠?
There was a problem hiding this comment.
아하 viewModel도 with로 묶을 수 있군요!
There was a problem hiding this comment.
오호! 좋은 답변 감사합니다! 뷰모델 내부에서 함수 작성하도록 수정했씁니다!
Comment on lines
92
to
100
| calendar.set(Calendar.YEAR, viewModel.startYear.value ?: 0) | ||
| calendar.set(Calendar.MONTH, viewModel.startMonth.value ?: 0) | ||
| calendar.set(Calendar.DAY_OF_MONTH, viewModel.startDay.value ?: 0) | ||
| val startDate = calendar.time | ||
|
|
||
| calendar.set(Calendar.YEAR, viewModel.endYear.value ?: 0) | ||
| calendar.set(Calendar.MONTH, viewModel.endMonth.value ?: 0) | ||
| calendar.set(Calendar.DAY_OF_MONTH, viewModel.endDay.value ?: 0) | ||
| val endDate = calendar.time |
There was a problem hiding this comment.
그리고 두번 반복되는 구조가 같다보니까 함수로도 만들 수 있을 것 같네요!
아래는 제가 생각하는 예시~~~~ 다르게 구현해도 너무 조음!!
val endDate = getCalendarTime(viewModel.endYear.value?:0, ...)
There was a problem hiding this comment.
앗 답변 감사합니다!
코드 다시 보다보니 이제 시작일과 종료일을 계산해서 비교할 필요가 없어져서 이 부분 코드는 지워도 될 것 같아서 이렇게 수정하였습니다!
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="37dp" | ||
| android:text="@string/edit_trip_info_one_line_info_tv_title" | ||
| android:textColor="@color/black_000" |
There was a problem hiding this comment.
?!?!?!?!?!!?!?!??!?!!?!?!?!?!?
우리 앱에 이런 색이 있었나요!!
Comment on lines
55
to
116
| <TextView | ||
| android:id="@+id/tv_edit_trip_info_name_title" | ||
| style="@style/TextAppearance.Doorip.Body3.Bold" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="24dp" | ||
| android:layout_marginTop="40dp" | ||
| android:text="@string/edit_trip_info_tv_title" | ||
| app:layout_constraintStart_toStartOf="@id/tb_edit_trip_info" | ||
| app:layout_constraintTop_toBottomOf="@id/tb_edit_trip_info" /> | ||
|
|
||
| <androidx.appcompat.widget.AppCompatEditText | ||
| android:id="@+id/et_edit_trip_info_name" | ||
| style="@style/edit_text_style" | ||
| android:layout_width="0dp" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginHorizontal="24dp" | ||
| android:layout_marginTop="8dp" | ||
| android:afterTextChanged="@{(text) -> viewModel.checkNameAvailable()}" | ||
| android:background="@drawable/shape_rect_4_gray700_line" | ||
| android:hint="@string/edit_trip_info_et_name_hint" | ||
| android:imeOptions="actionDone" | ||
| android:inputType="text" | ||
| android:maxLines="3" | ||
| android:text="@={viewModel.name}" | ||
| android:textAppearance="@style/TextAppearance.Doorip.Detail1.Regular" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/tv_edit_trip_info_name_title" /> | ||
|
|
||
| <TextView | ||
| style="@style/TextAppearance.Doorip.Detail3.Regular" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="4dp" | ||
| android:layout_marginTop="4dp" | ||
| android:text="@{viewModel.isNameAvailable() == NameState.Blank ? @string/name_blank_error : @string/trip_over_error}" | ||
| android:textColor="@color/red_500" | ||
| android:visibility="@{(viewModel.isNameAvailable() == NameState.Blank) || (viewModel.isNameAvailable() == NameState.OVER) ? View.VISIBLE : View.GONE}" | ||
| app:layout_constraintStart_toStartOf="@id/et_edit_trip_info_name" | ||
| app:layout_constraintTop_toBottomOf="@id/et_edit_trip_info_name" /> | ||
|
|
||
| <TextView | ||
| android:id="@+id/tv_name_counter" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="4dp" | ||
| android:text="@{@string/counter(viewModel.nameLength, viewModel.MAX_TRIP_LEN)}" | ||
| android:textColor="@color/gray_200" | ||
| app:layout_constraintEnd_toEndOf="@id/et_edit_trip_info_name" | ||
| app:layout_constraintTop_toBottomOf="@id/et_edit_trip_info_name" /> | ||
|
|
||
| <TextView | ||
| android:id="@+id/tv_edit_trip_info_on_line_info_title" | ||
| style="@style/TextAppearance.Doorip.Body3.Bold" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="37dp" | ||
| android:text="@string/edit_trip_info_one_line_info_tv_title" | ||
| android:textColor="@color/black_000" | ||
| app:layout_constraintStart_toStartOf="@id/et_edit_trip_info_name" | ||
| app:layout_constraintTop_toBottomOf="@id/et_edit_trip_info_name" /> |
There was a problem hiding this comment.
이 복잡한 코드... 복붙하느라 고생 많으셨습니다!
복붙... 불편하지 않으셨나요? 그런당신을 위해 CustomView가 준비되어 있답니다 ㅎㅎㅎㅎ
수정하십쇼!
Comment on lines
40
to
73
| private fun observeIsNameAvailable() { | ||
| viewModel.isNameAvailable.observe(this) { state -> | ||
| setColors( | ||
| state, | ||
| binding.tvNameCounter, | ||
| ) { background -> | ||
| binding.etEditTripInfoName.background = ResourcesCompat.getDrawable( | ||
| this.resources, | ||
| background, | ||
| theme, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun setColors( | ||
| state: NameState, | ||
| counter: TextView, | ||
| setBackground: (Int) -> Unit, | ||
| ) { | ||
| val (color, background) = when (state) { | ||
| NameState.Empty -> R.color.gray_200 to R.drawable.shape_rect_4_gray200_line | ||
| NameState.Success -> R.color.gray_700 to R.drawable.shape_rect_4_gray700_line | ||
| NameState.Blank -> R.color.red_500 to R.drawable.shape_rect_4_red500_line | ||
| NameState.OVER -> R.color.red_500 to R.drawable.shape_rect_4_red500_line | ||
| } | ||
|
|
||
| setCounterColor(counter, color) | ||
| setBackground(background) | ||
| } | ||
|
|
||
| private fun setCounterColor(counter: TextView, color: Int) { | ||
| counter.setTextColor(getColor(color)) | ||
| } |
There was a problem hiding this comment.
이런 복잡한것들을 customView를 사용하면 안써도 된답니당~
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="@string/edit_trip_info_slash" | ||
| android:textColor="@color/black_000" |
Comment on lines
54
to
67
| private fun customEndDate() { | ||
| val datePicker = binding.dpEditTripDate | ||
| val calendar = Calendar.getInstance() | ||
|
|
||
| val currentEndYear = viewModel.startYear.value ?: 0 | ||
| val currentEndMonth = viewModel.startMonth.value ?: 0 | ||
| val currentEndDay = viewModel.startDay.value ?: 0 | ||
|
|
||
| calendar.set(currentEndYear, currentEndMonth - 1, currentEndDay) | ||
| datePicker.minDate = calendar.timeInMillis | ||
|
|
||
| calendar.set(2100, 0, 1) | ||
| datePicker.maxDate = calendar.timeInMillis | ||
| } |
leeeyubin
approved these changes
Feb 28, 2024
Comment on lines
54
to
67
| private fun customEndDate() { | ||
| val datePicker = binding.dpEditTripDate | ||
| val calendar = Calendar.getInstance() | ||
|
|
||
| val currentEndYear = viewModel.startYear.value ?: 0 | ||
| val currentEndMonth = viewModel.startMonth.value ?: 0 | ||
| val currentEndDay = viewModel.startDay.value ?: 0 | ||
|
|
||
| calendar.set(currentEndYear, currentEndMonth - 1, currentEndDay) | ||
| datePicker.minDate = calendar.timeInMillis | ||
|
|
||
| calendar.set(2100, 0, 1) | ||
| datePicker.maxDate = calendar.timeInMillis | ||
| } |
| android:layout_marginTop="4dp" | ||
| android:text="@{viewModel.isNameAvailable() == NameState.Blank ? @string/name_blank_error : @string/trip_over_error}" | ||
| android:textColor="@color/red_500" | ||
| android:visibility="@{(viewModel.isNameAvailable() == NameState.Blank) || (viewModel.isNameAvailable() == NameState.OVER) ? View.VISIBLE : View.GONE}" |
Comment on lines
69
to
83
| private fun sendDateInfo() { | ||
| if (isStart) { | ||
| viewModel.startYear.value = binding.dpEditTripDate.year | ||
| viewModel.startMonth.value = binding.dpEditTripDate.month + 1 | ||
| viewModel.startDay.value = binding.dpEditTripDate.dayOfMonth | ||
| viewModel.checkStartDateAvailable() | ||
|
|
||
| } else { | ||
| customEndDate() | ||
| viewModel.endYear.value = binding.dpEditTripDate.year | ||
| viewModel.endMonth.value = binding.dpEditTripDate.month + 1 | ||
| viewModel.endDay.value = binding.dpEditTripDate.dayOfMonth | ||
| viewModel.checkEndDateAvailable() | ||
| } | ||
| } |
There was a problem hiding this comment.
아하 viewModel도 with로 묶을 수 있군요!
Comment on lines
23
to
25
| val isNameAvailable = MutableLiveData(NameState.Empty) | ||
| private val isTripAvailable = MutableLiveData(false) | ||
| var isCheckTripAvailable = MutableLiveData(false) |
There was a problem hiding this comment.
isTripAvailable만 private인 거는 이 친구만 다른 곳에서 안 써줘서 그런 건가용...?
There was a problem hiding this comment.
넵 isTripAvailable 은 해당 뷰모델에서만 사용해서 private로 해주었습니다!!
Marchbreeze
approved these changes
Feb 29, 2024
Comment on lines
11
to
12
| class BottomSheetEditDateFragment(val viewModel: EditTripInfoViewModel, val isStart: Boolean) : | ||
| BaseBottomSheet<FragmentBottomSheetEditDateTripBinding>(R.layout.fragment_bottom_sheet_edit_date_trip) { |
There was a problem hiding this comment.
뷰모델을 파라미터로 보내게 된다면 종속성이 생기게 됩니다 !!
다른 프래그먼트에서 하는 것처럼, 바텀시트 내부에서 선언해주세요 ~~ 다른 바텀시트들도 수정해주세요
그리고 네이밍 저는 EditDateBottomSheet처럼 바텀시트가 뒤로 가게 통일하는건 어떨까요~ 프래그먼트까지 쓰면 너무 길어지길래 .. ㅎㅎ
Comment on lines
32
to
34
| val currentStartYear = calendar.get(Calendar.YEAR) | ||
| val currentStartMonth = calendar.get(Calendar.MONTH) | ||
| val currentStartDay = calendar.get(Calendar.DAY_OF_MONTH) |
There was a problem hiding this comment.
요 친구들은 맨 아래에 datePicker.updateDate(currentStartYear, currentStartMonth, currentStartDay) 에서만 사용되는 것 같은데, 불필요한 변수 선언 대신 스코프 함수로 처리해보는건 어떨까요! (get때문에 안돌아갈수도 있긴 함)
Suggested change
| val currentStartYear = calendar.get(Calendar.YEAR) | |
| val currentStartMonth = calendar.get(Calendar.MONTH) | |
| val currentStartDay = calendar.get(Calendar.DAY_OF_MONTH) | |
| val calendar = Calendar.getInstance() | |
| val datePicker = binding.dpEditTripDate.apply { | |
| updateDate( | |
| calendar.get(Calendar.YEAR), | |
| calendar.get(Calendar.MONTH), | |
| calendar.get(Calendar.DAY_OF_MONTH) | |
| ) | |
| } |
이런 느낌으로요!
There was a problem hiding this comment.
좋은 지적 감사합니다!!! 이렇게 스코프로 수정했습니다!
Comment on lines
54
to
67
| private fun customEndDate() { | ||
| val datePicker = binding.dpEditTripDate | ||
| val calendar = Calendar.getInstance() | ||
|
|
||
| val currentEndYear = viewModel.startYear.value ?: 0 | ||
| val currentEndMonth = viewModel.startMonth.value ?: 0 | ||
| val currentEndDay = viewModel.startDay.value ?: 0 | ||
|
|
||
| calendar.set(currentEndYear, currentEndMonth - 1, currentEndDay) | ||
| datePicker.minDate = calendar.timeInMillis | ||
|
|
||
| calendar.set(2100, 0, 1) | ||
| datePicker.maxDate = calendar.timeInMillis | ||
| } |
There was a problem hiding this comment.
이 부분에서 calendar set이 바뀌고 두번 적용되는 것 같은데, 아예 객체를 따로 가져가는건 어떨까요?
Suggested change
| private fun customEndDate() { | |
| val datePicker = binding.dpEditTripDate | |
| val calendar = Calendar.getInstance() | |
| val currentEndYear = viewModel.startYear.value ?: 0 | |
| val currentEndMonth = viewModel.startMonth.value ?: 0 | |
| val currentEndDay = viewModel.startDay.value ?: 0 | |
| calendar.set(currentEndYear, currentEndMonth - 1, currentEndDay) | |
| datePicker.minDate = calendar.timeInMillis | |
| calendar.set(2100, 0, 1) | |
| datePicker.maxDate = calendar.timeInMillis | |
| } | |
| binding.dpEditTripDate.apply{ | |
| minDate = Calendar.getInstance().apply{ | |
| set( | |
| viewModel.startYear.value ?: 0 | |
| viewModel.startMonth.value ?: 0 | |
| viewModel.startDay.value ?: 0 | |
| ) | |
| }.timeInMillis | |
| maxDate = Calendar.getInstance().apply{ | |
| set(2100,0,1) | |
| }.timeInMillis | |
| } |
요런 느낌..?
There was a problem hiding this comment.
넵 객체 두개로 가져가도록 수정하겠습니다!
Comment on lines
18
to
20
| private lateinit var startBottomSheetDialog: BottomSheetEditDateFragment | ||
| private lateinit var endBottomSheetDialog: BottomSheetEditDateFragment | ||
| private var quitDialog: TripQuitDialogFragment? = null |
There was a problem hiding this comment.
다이얼로그, 바텀시트, 어댑터와 같이 액티비티에 종속되지만 생명주기를 감지할 수 없는 요소들은,
미처 종료를 하지 않아주게 되면 이 액티비티가 종료가 되었을 때에도 혼자서 살아있어 메모리 누수를 유발할 수 있습니다!
20번째 줄에서의 quitDialog 설정해둔 것 처럼, 초기값을 null로 설정해둔 후 onDestroy에서 다시 null로 바꿔주면서 생명주기에 끼워맞추는 작업을 해주신다면 더 좋은 코드를 만드실 수 있어요 ~
|
|
||
| <import type="android.view.View" /> | ||
|
|
||
| <import type="com.going.domain.entity.NameState" /> |
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/tv_edit_trip_info_on_line_info_title"> | ||
|
|
||
| <TextView |
There was a problem hiding this comment.
여기 텍스트들에 폰트 적용이 안되어있는 것 같습니다!
chattymin
approved these changes
Mar 3, 2024
Marchbreeze
approved these changes
Mar 3, 2024
Comment on lines
18
to
19
| private lateinit var startBottomSheetDialog: DateContentBottomSheet | ||
| private lateinit var endBottomSheetDialog: DateContentBottomSheet |
There was a problem hiding this comment.
여기 뷰 다이얼로그들도 생명주기 따라 해제되도록 해제해주면 더 좋을 것 같아요 !
There was a problem hiding this comment.
엇 이걸 놓쳤네욥,,ㅠ 수정하겠습니다!
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
1.mp4
📢 To Reviewers