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

[MOD] 상세페이지 / 보관함으로 뒤로가기 및 코스 삭제 시 애니메이션 통일 #277

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

unam98
Copy link
Contributor

@unam98 unam98 commented Nov 9, 2023

📌 개요

closed #253

✨ 작업 내용

  • 보관함으로 뒤로가기 및 코스 삭제 시 애니메이션 통일

@unam98 unam98 added 우남 🐼 우남 담당 MOD ☁ 코드 및 내부 파일 수정 labels Nov 9, 2023
@unam98 unam98 requested a review from leeeha November 9, 2023 01:58
@unam98 unam98 self-assigned this Nov 9, 2023
override fun handleOnBackPressed() {
finish()
overridePendingTransition(R.anim.slide_in_left, R.anim.slide_out_right)
}
Copy link
Member

Choose a reason for hiding this comment

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

반복되는 코드를 함수화 하는 게 어떨까요??

finish()
overridePendingTransition(R.anim.slide_in_left, R.anim.slide_out_right)

Copy link
Contributor Author

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.

다른 부분에서도 저 코드가 반복되는 경우가 많더라구요!! 확장함수로 만드는 거 좋은 거 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

확장함수로 만들었고 반영했습니다!

@@ -101,6 +106,8 @@ class MyDrawDetailActivity :

fun addObserver() {
observeGetResult()

onBackPressedDispatcher.addCallback(this, backPressedCallback)
Copy link
Member

Choose a reason for hiding this comment

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

코드 스타일의 차이일 수 있지만, 아래처럼 관련있는 코드를 함수로 묶어두면 나중에 볼 때 편하더라구요!

private fun registerBackPressedCallback() { // 이 함수를 addObserver에서 호출 
  val callback = object : OnBackPressedCallback(true) {
    override fun handleOnBackPressed() {
      naviagteToPreviousScreen() 
    }
  }
  onBackPressedDispatcher.addCallback(this, callback)
}

private fun naviagteToPreviousScreen() {
  finish()
  overridePendingTransition(R.anim.slide_in_left, R.anim.slide_out_right)
}

Copy link
Contributor Author

@unam98 unam98 Nov 10, 2023

Choose a reason for hiding this comment

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

finishViewAnimLeftRight()라는 네이밍으로 함수화해서 커밋 올렸는데 혹시 위의 코드에서 알려주신 navigateToPreviousScreen()이라는 함수가 다른 파일에서도 쓰이고 있는 것인가요? 그렇다면 이름을 navigateToPreviousScreen()으로 맞추겠습니다

Copy link
Member

Choose a reason for hiding this comment

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

저는 다른 코드에서 navigateToPreviousScreen 이라는 함수명을 사용하긴 했습니다!
현재 액티비티에서 뒤로가기 버튼을 누르면 이전 액티비티로 돌아간다는 걸 표현하고 싶어서 그렇게 작성했던 걸로 기억합니다!

Copy link
Member

Choose a reason for hiding this comment

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

addObserver 함수에서 onBackPressedDispatcher에 콜백 등록하는 코드도
registerBackPressedCallback 이라는 함수를 따로 만들어서 그 안에서 작성하는 게 어떨까 싶습니다!

backPressedCallback 이라는 변수를 굳이 전역 변수로 만들 필요가 없을 거 같아서요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다!

- 반복 사용되는 logic을 함수화함
@@ -101,6 +106,8 @@ class MyDrawDetailActivity :

fun addObserver() {
observeGetResult()

onBackPressedDispatcher.addCallback(this, backPressedCallback)
Copy link
Member

Choose a reason for hiding this comment

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

addObserver 함수에서 onBackPressedDispatcher에 콜백 등록하는 코드도
registerBackPressedCallback 이라는 함수를 따로 만들어서 그 안에서 작성하는 게 어떨까 싶습니다!

backPressedCallback 이라는 변수를 굳이 전역 변수로 만들 필요가 없을 거 같아서요!

@unam98 unam98 merged commit 929d9dd into develop Nov 14, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MOD ☁ 코드 및 내부 파일 수정 우남 🐼 우남 담당
Projects
Development

Successfully merging this pull request may close these issues.

[MOD] 보관함 / 코스 상세페이지 진입/뒤로가기 애니메이션 통일
2 participants