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

feat: 좋아요, 싫어요 버튼을 연속 클릭할 수 없도록 변경 #547

Merged
merged 17 commits into from
Nov 8, 2023

Conversation

briandr97
Copy link
Collaborator

📌 관련 이슈

🛠️ 작업 내용

  • 커스텀뷰 내부 로직 수정
  • 정합성 문제를 해결 방안 변경으로 인해 뷰모델 로직 수정

🎯 리뷰 포인트

디바운스를 적용하려 했지만, 코루틴 플로우에 대한 학습이 필요하여 우선 싱글 클릭으로 처리하였습니다.
차후 플로우를(cold) 학습하게 되면 변경할 예정입니다.

⏳ 작업 시간

추정 시간: 2h
실제 시간: 4h
이유: 디바운스를 알아보다가 시간이 오래걸림

@briandr97 briandr97 self-assigned this Nov 7, 2023
Copy link
Collaborator

@hyunji1203 hyunji1203 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 singleClick(state: PreferenceState) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 singleClick 로직은 훗날 다른곳에서도 사용될 수 있을 것 같아 나중에 공통 로직으로 빼봐도 좋을 것 같아요!
물론 나중에 flow로 변경된다고 하면 이 일회성인 로직이니 굳이 분리할 필요는 없을 것 같긴하네요ㅋㅋ

여기서 한가지 궁금한 점이 있는데, 메서드의 인자로 들어가는 state는 onClick으로 넘겨져서 어떤 일을 하는 건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

preferenceClickListener?.onClick(state)로 전달됩니다.
preferenceClickListenerPreferenceView에 정의된 인터페이스이며 외부에서 이를 구현하여 setPreferenceClickListener함수를 통해 초기화합니다.

@hyunji1203 hyunji1203 merged commit 1140a9b into dev_android Nov 8, 2023
3 checks passed
@hyunji1203 hyunji1203 deleted the feat/#546-preference_singleClick branch November 8, 2023 01:36
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.

2 participants