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/#192] 여행 대시 보드 뷰 / 코드 리팩토링 #197

Merged
merged 11 commits into from
Feb 8, 2024

Conversation

leeeyubin
Copy link
Member

@leeeyubin leeeyubin commented Feb 5, 2024

⛳️ Work Description

  • Intent 로직 수정
  • navigateToScreen 확장함수 적용
  • adapter에 val inflater by lazy { LayoutInflater.from(parent.context) } 적용
  • 리스트 어댑터 Click Listener 로직 수정
  • 기타 수정들

📢 To Reviewers

  • 여러분 오랜만입니다 ㅎㅅㅎ
    잘하고 있는지 확인차,, 여행 대시 보드 뷰 먼저 올리고 다른 거 한 번에 할게욤

  • navigateToScreen 확장함수 적용할 때 그냥 화면 전환만 하는 거면 null, false를 넣는 게 맞겠죠..?

  • 그리구 사실 신경 쓰였던 부분이 원래 스크롤뷰를 막았었는데 아이폰은 안 막혀 있어서 통일해주는 게 좋을 것 같아서 안 막는 걸로 바꿨습니다

Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

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

너무 잘하자나~~~!!!!
이게 갓기?!?!?!?

private fun navigateToSettingScreen() {
Intent(this, SettingActivity::class.java).apply {
startActivity(this)
navigateToScreen<SettingActivity>(null, false)
Copy link
Member

Choose a reason for hiding this comment

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

아래와 같이 기본값이 있기 때문에 빈괄호로 하셔도 됩니당~
navigateToScreen<SettingActivity>()

inline fun <reified T : Activity> Activity.navigateToScreen(
    flags: List<Int>? = null,
    isFinish: Boolean = true,
) {
    Intent(this, T::class.java).apply {
        flags?.map {
            addFlags(it)
        }
        startActivity(this)
    }
    if (isFinish) {
        finish()
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

엇 근데 그냥 빈괄호로 사용하면 isFinish가 true로 들어가서 백버튼 눌렀을 때 바로 앱이 종료되는 것 같습니다,, 그래서 null, false순으로 넣었던 건데 혹시 다른 방법이 있을까요.....?

Copy link
Member

@chattymin chattymin Feb 8, 2024

Choose a reason for hiding this comment

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

아 제가 실수로 잘못 봤네요!
이렇게 되면 아래처럼 하나만 매개변수 써주시면 됩니당•• 😅
isFinish = false

Copy link
Member Author

Choose a reason for hiding this comment

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

오 이렇게 써줄 수도 있군요!! 수정했습니당

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): OngoingViewHolder {
val inflater by lazy { LayoutInflater.from(parent.context) }
Copy link
Member

Choose a reason for hiding this comment

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

값이 할당된 후 바로 밑에서 사용되는데 by lazy를 사용한 이유는 뭘까요??

Copy link
Member

Choose a reason for hiding this comment

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

동민이도 이거 알아오는거 숙제 ~

Copy link
Member Author

Choose a reason for hiding this comment

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

오호,,,생각해보도록 하겠습니닷

Comment on lines 23 to 31
when {
item.day <= 0 -> {
tvDashboardDeadline.text =
itemView.context.getString(R.string.dashboard_tv_traveling)
}

else -> {
tvDashboardDeadline.text =
itemView.context.getString(R.string.dashboard_tv_deadline, item.day)
Copy link
Member

Choose a reason for hiding this comment

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

확장성을 고려해서 바꾼걸까요?
가독성도 좋고 너무 좋네요!! 👍

Copy link
Member

Choose a reason for hiding this comment

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

이 부분이 tvDashboardDeadline.text 설정에만 관련된 부분이라면 tvDashboardDeadline.text = when {} 형태로 가도 더 확실하게 표시해줄 수 있을듯 ! 물론 이부분이 추후에 확장된 가능성이 있다면 이것도 조하용

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했습니다~!

Copy link
Member

@Marchbreeze Marchbreeze left a comment

Choose a reason for hiding this comment

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

아웅 너무 야무지네요 ㅎㅎ

@@ -8,21 +8,18 @@ import com.going.presentation.databinding.ItemDashBoardCompletedBinding
import com.going.ui.extension.ItemDiffCallback

class CompletedAdapter(
private val listener: OnDashBoardSelectedListener
private val itemDetailClick: (DashBoardTripModel) -> Unit
Copy link
Member

Choose a reason for hiding this comment

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

람다로 수정해주는거 너무 좋아요 ~~~~

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): CompletedViewHolder {
val inflater by lazy { LayoutInflater.from(parent.context) }
Copy link
Member

Choose a reason for hiding this comment

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

이거로 쓰는게 왜 좋게~요

Comment on lines +41 to +44
TodoActivity.createIntent(
requireContext(),
dashBoardTripModel.tripId
).apply { startActivity(this) }
Copy link
Member

Choose a reason for hiding this comment

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

좋아용! 사용법 잘 알고있네요 ~~~ 안에 내용이 좀 길어진다 싶으면 따로 함수화해주어도 가독성이 더 좋아집니다 !

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): OngoingViewHolder {
val inflater by lazy { LayoutInflater.from(parent.context) }
Copy link
Member

Choose a reason for hiding this comment

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

동민이도 이거 알아오는거 숙제 ~

Comment on lines 23 to 31
when {
item.day <= 0 -> {
tvDashboardDeadline.text =
itemView.context.getString(R.string.dashboard_tv_traveling)
}

else -> {
tvDashboardDeadline.text =
itemView.context.getString(R.string.dashboard_tv_deadline, item.day)
Copy link
Member

Choose a reason for hiding this comment

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

이 부분이 tvDashboardDeadline.text 설정에만 관련된 부분이라면 tvDashboardDeadline.text = when {} 형태로 가도 더 확실하게 표시해줄 수 있을듯 ! 물론 이부분이 추후에 확장된 가능성이 있다면 이것도 조하용

Copy link
Contributor

@crownjoe crownjoe left a comment

Choose a reason for hiding this comment

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

이게 갓기...미쵸따~

Comment on lines +41 to +44
TodoActivity.createIntent(
this,
tripId
).apply { startActivity(this) }
Copy link
Contributor

Choose a reason for hiding this comment

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

좋네욥 코드가 깔끔해진게 보여요 굿굿

@leeeyubin leeeyubin merged commit 521b6b6 into develop Feb 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 여행 대시 보드 뷰 / 코드 리팩토링
4 participants