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

[FIX] 러닝 기록 / 삭제 버튼 활성화 오류 #278

Merged
merged 5 commits into from
Nov 14, 2023

Conversation

unam98
Copy link
Contributor

@unam98 unam98 commented Nov 9, 2023

📌 개요

closed #265

✨ 작업 내용

  • 기존에 버튼이 TextView로 만들어져있던 걸 다 들어내고 Button으로 다시 만들어주었음
  • 버튼 활성화 안 됐던 문제 해결

✨ PR 포인트

같은 기능을 보관함에서 구현한 코드가 있는데 여기는 제가 짠 코드가 아니라 일단 디버깅만 해놓고 추후에 제가 보관함에 짜놓은 스타일로 리팩토링을 할 예정입니다. 따라서 특별히 봐주실 부분은 없습니다!

📸 스크린샷/동영상

KakaoTalk_20231109_112542120.mp4

버그 잡으려고 코드 보고 있는데

버튼이 TextView랑  selector로 만들어져있고 ViewModel에 불필요하게 변수가 많이 생성돼있어서 코드 파악에 상당한 시간이 소요되어 불편했음.

그래서 기존의 코드를 수정 중
@unam98 unam98 added 우남 🐼 우남 담당 FIX 💥 버그 및 오류 해결 labels Nov 9, 2023
@unam98 unam98 requested a review from leeeha November 9, 2023 02:41
@unam98 unam98 self-assigned this Nov 9, 2023
Copy link
Member

@leeeha leeeha left a comment

Choose a reason for hiding this comment

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

텍스트뷰 대신 버튼을 사용하면 해결되는 이슈했던 건가요?? 😮

viewModel.selectedItemsCount.observe(this) { count ->
updateDeleteButton(count)
viewModel.itemsToDeleteLiveData.observe(this) { count ->
updateDeleteButton(count.size)
Copy link
Member

Choose a reason for hiding this comment

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

itemsToDeleteLiveData가 관찰하는 데이터 타입이 List<Int> 이므로
count라는 매개변수 이름 대신 목록이라는 걸 알아차릴 수 있는 이름으로 바꾸는 게 좋을 거 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다!

@@ -195,15 +195,18 @@ class MyHistoryActivity : BindingActivity<ActivityMyHistoryBinding>(R.layout.act
btnMyPageHistoryEditHistory.text = EDIT_MODE
tvMyPageHistoryTotalCourseCount.text = viewModel.getHistoryCount()
if (::adapter.isInitialized) adapter.clearSelection()
tvMyPageHistoryDelete.isVisible = viewModel.editMode.value!!
btnMyPageHistoryDelete.isVisible = viewModel.editMode.value!!
viewModel.clearItemsToDelete()
}
}

private fun updateDeleteButton(count: Int) {
Copy link
Member

Choose a reason for hiding this comment

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

옵저버 함수에서 updateDeleteButton 함수를 호출할 때 count는 목록을 나타냈는데, 여기서는 Int형이어서 약간 헷갈리는 거 같습니다!

List<Int> 타입의 매개변수를 다른 이름으로 바꿔주는 게 좋을 거 같아요!

Copy link
Contributor Author

@unam98 unam98 Nov 9, 2023

Choose a reason for hiding this comment

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

이쪽은 제가 작성한 코드가 아니어서 일단 버그만 먼저 잡았고 이후 제 스타일대로 코드 다시 짜려고 합니다! 우선 피드백 주신 내용 반영하도록 하겠습니다!

@@ -195,15 +195,18 @@ class MyHistoryActivity : BindingActivity<ActivityMyHistoryBinding>(R.layout.act
btnMyPageHistoryEditHistory.text = EDIT_MODE
tvMyPageHistoryTotalCourseCount.text = viewModel.getHistoryCount()
if (::adapter.isInitialized) adapter.clearSelection()
tvMyPageHistoryDelete.isVisible = viewModel.editMode.value!!
btnMyPageHistoryDelete.isVisible = viewModel.editMode.value!!
Copy link
Member

Choose a reason for hiding this comment

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

non-null 연산자를 사용하지 않는 방향으로 바꾸는 게 좋을 거 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다!

@unam98
Copy link
Contributor Author

unam98 commented Nov 9, 2023

텍스트뷰 대신 버튼을 사용하면 해결되는 이슈했던 건가요?? 😮

아뇨 그건 아닐 텐데 여기처럼 리사이클러뷰 item 다중 삭제 기능이 보관함에도 구현돼있습니다. 그쪽 코드가 제가 짠 코드인데 지금 여기 짜여져 있는 코드는 복잡하게 좀 꼬아진 느낌이라 디버깅보다 코드 읽는 것에 시간이 더 걸릴 것 같아서 싹 날리고 다시 짜려고 했습니다. 그래서 정확한 원인 파악을 안 했습니다.

@unam98 unam98 merged commit db27254 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
FIX 💥 버그 및 오류 해결 우남 🐼 우남 담당
Projects
Development

Successfully merging this pull request may close these issues.

[FIX] 러닝 기록 / 삭제하기 버튼 활성화 오류
2 participants