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

[Feature] GA4 클릭/스크롤 이벤트 추가 #245

Merged
merged 31 commits into from
May 1, 2024
Merged

Conversation

ThirFir
Copy link
Contributor

@ThirFir ThirFir commented Apr 29, 2024

개요


상세 작업 내용

  • 이벤트 Logger 싱글톤 객체 추가 - click, scroll 로깅 함수
  • 명세에 따라 필요한 곳에서 로깅

TabLayout + ViewPager2 이슈

  • TabLayout의 탭 선택으로 클릭 이벤트, ViewPager2의 스크롤에 의한 스크롤 이벤트를 분리해서 로깅해야 했음
  • TabLayout의 addOnTabSelectedListener 사용 불가(탭 선택, 스크롤 이벤트를 모두 수신)
  • ViewPager2의 스크롤 콜백리스너를 일반적인 방법으로 사용 불가(탭 선택으로 인한 스크롤도 수신)

해결 방안

  1. TabLayout의 각 Child에 clickListener 설정 -> 스크롤에는 반응하지 않음
  2. ViewPager2의 스크롤 콜백리스너에 탭 선택에 의한 프로그램의 스크롤인지 유저의 스크롤인지 판단하는 로직 추가

이벤트 명세

/**
* @param action: 이벤트 발생 도메인(BUSINESS, CAMPUS, USER)
* @param category: 이벤트 종류(click, scroll, ...)
* @param label: 이벤트 소분류
* @param value: 이벤트 값
* @sample logEvent("BUSINESS", "click", "main_store_categories", "전체보기")
*/
private fun logEvent(action: String, category: String, label: String, value: String) {
    Firebase.analytics.logEvent(action) {
        param("event_category", category)
        param("event_label", label)
        param("value", value)
    }
}

@ThirFir ThirFir self-assigned this Apr 29, 2024
@ThirFir ThirFir requested a review from a team as a code owner April 29, 2024 02:32
Copy link
Contributor

@nodobi nodobi left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 !!

Copy link
Collaborator

@Jokwanhee Jokwanhee left a comment

Choose a reason for hiding this comment

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

로깅하는 함수를 추가할 때, 굉장히 기네요.. 뭔가 더 줄일 방법이 없을 지 고민해봐도 괜찮을 것 같아요! 고생하셨습니다!

@@ -41,14 +43,40 @@ class BusMainFragment : Fragment(R.layout.bus_main_fragment) {
initViewModel()
}

private var isUserSelection = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Configuration changed가 발생했을때 해당 flag값이 날라갈수도 있습니다. AAC ViewModel을 활용해보는거는 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ViewModel 활용해서 flag 체크하도록 변경하였습니다!!
88a12f9

busDepartureSpinner.setOnItemSelectedListener { _, _, position, _ ->
viewModel.setDeparture(position.busNodeSelection)
if(isUserSelection) {
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 Interection도 ViewModel에 추가하면 테스트 케이스로 추가 가능합니다.

@ThirFir
Copy link
Contributor Author

ThirFir commented Apr 30, 2024

로깅하는 함수를 추가할 때, 굉장히 기네요.. 뭔가 더 줄일 방법이 없을 지 고민해봐도 괜찮을 것 같아요! 고생하셨습니다!

아무래도 일일이 로깅할 문자열을 지정해서 넘겨줘야 하다보니 길어질 수 밖에 없었던 것 같습니다...
상속이나 확장함수를 이용하여 하는 방법도 시도해보았는데, 막상 시도해보니 로깅을 발생시키는 부분에서의 활용 방법이 상이해서 공통의 인터페이스를 만드는 게 어려워 현재로서는 싱글톤 객체를 활용하는 것이 최선이라고 생각하여 진행하였습니다 ㅠ_ㅠ

Copy link

@https-namhoon-kim https-namhoon-kim left a comment

Choose a reason for hiding this comment

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

리뷰했습니다.

@@ -0,0 +1,48 @@
package `in`.koreatech.koin.core.constant

object AnalyticsConstant {

Choose a reason for hiding this comment

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

object in object 구조를 차용하는 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이벤트 로깅에 사용되는 상수임을 명확히 하기 위해 관련 상수들을 AnalyticsConstant라는 object 내에 배치하였고, 그 안에서도 각 상수들이 어떤 parameter에 쓰이는지 명확히 하기 위해 object로 다시 한 번 분류하였습니다.

Copy link
Contributor

@nodobi nodobi left a comment

Choose a reason for hiding this comment

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

고생하셨어요! 👍

@ThirFir ThirFir merged commit a9116c0 into develop May 1, 2024
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.

6 participants