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

[SP1] 앰플리튜드 이벤트 트래킹 - 메인페이지, 활동후기, 프로젝트 #202

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

SeojinSeojin
Copy link
Member

Summary

앰플리튜드 이벤트 트래킹을 붙입니다.

Comment

여러분 Part 등은 다 영어로 가도록 하셨나여 ..???

그리고 click 이벤트를 한땀한땀 붙이다가 생각한 것은, 엘리먼트의 프로퍼티로 넣어둬도 괜찮을 것 같다는 생각이 들어용

AS-IS

<div onClick={()=>track("click_review_detail")} />

TO-BE

<div amplitude-click-track="review_detail" />

어딘가에서

window.addEventListener('click', (e)=> {
	// 클릭 당한 엘리먼트에게 amplitude-click-track 프로퍼티가 있는지 확인하고,
	// 있으면 track(`click_${그것}`); 쏴주기!
});

이벤트 위임 방식인데요, 클릭리스너 하나라 부하가 심할 것 같지도 않고 앞으로 개발하기도 편할 것 같아서 의견을 내 봅니다!!
(물론 커스텀 파라미터 전해줘야 할 때는 컴포넌트에서 직접 쏴주거나, 다른 방법을 찾아보아야 합니당...)
@solar3070 @f0rever0 님 의견을 듣고 싶습니다!!!!

Copy link
Member

@solar3070 solar3070 left a comment

Choose a reason for hiding this comment

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

엘리먼트의 프로퍼티로 넣어둬도 괜찮을 것 같다는 생각이 들어용

오 생각지도 못한 굳 아이디어입니다!! 👍👍👍
다만 몇 가지 질문이 있습니다.

  1. 이벤트 리스너는 어디에 작성하실 생각인가요?
    1-a. 훅? 훅이라면 훅은 어디에서 불러올지?
  2. 프로퍼티가 있는지 확인하고자 할 때 e.target.프로퍼티명과 같이 체크하게 되나요? 이벤트의 타입을 MouseEvent로 지정하면 커스텀 프로퍼티 속성이 없다고 뜨는데 타입은 어떤식으로 지정하실 생각인가요?
  3. 엘리먼트의 프로퍼티는 string 타입만 가능하니 이벤트 프로퍼티가 있는 경우 추가적인 코드 작성이 필요한데 이렇게 되면 원래의 목적인 개발의 편리함을 벗어나게 되는 것이 아닌가 하는 생각이 듭니다. 뭔가 생각하신 방안이 있으실까요?

Comment on lines 10 to 13
const onTabClick = (tab: ExtraPart) => {
track('click_review_part', { part: tab });
setSelectedTab(tab);
};
Copy link
Member

Choose a reason for hiding this comment

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

여러분 Part 등은 다 영어로 가도록 하셨나여 ..???

저는 한글로 보냈습니다! tab.label로 보냈어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 tab.value로 영어로 보냈는데 통일이 필요할 것 같네요!

@SeojinSeojin
Copy link
Member Author

  1. 이벤트 리스너는 어디에 작성하실 생각인가요?
    1-a. 훅? 훅이라면 훅은 어디에서 불러올지?

이벤트 리스너는 훅 형태로 만들고 _app.tsx 에서 불러오게 될 것 같습니다!!

  1. 프로퍼티가 있는지 확인하고자 할 때 e.target.프로퍼티명과 같이 체크하게 되나요? 이벤트의 타입을 MouseEvent로 지정하면 커스텀 프로퍼티 속성이 없다고 뜨는데 타입은 어떤식으로 지정하실 생각인가요?

data-* 로 프로퍼티 이름을 쓰면 element.dataset 로 접근할 수 있어요!!
참고자료 : https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

타입스크립트에서 저것을 써보지 않았지만, 불가능할 것 같지는 않습니다 .!.!

  1. 엘리먼트의 프로퍼티는 string 타입만 가능하니 이벤트 프로퍼티가 있는 경우 추가적인 코드 작성이 필요한데 이렇게 되면 원래의 목적인 개발의 편리함을 벗어나게 되는 것이 아닌가 하는 생각이 듭니다. 뭔가 생각하신 방안이 있으실까요?

보통 프로퍼티는 그 컴포넌트에서 큰 힘 들이지 않고 접근 가능한 방법이니 아래 방법들이 가능할 것 같습니다!

위의 data-* 프로퍼티로 이벤트 프로퍼티 또한 집어넣고, 그것도 보내도록 코드를 짠다
(예시: data-amplitude-prop-tabs="안드로이드")

막 정규표현식 돌리고 먼가 해야겠지만 .. 프로퍼티 개수가 막 100개 넘어가지는 않으니 괜찮을 것 같다고 생각이 들어요 ..!!

@solar3070
Copy link
Member

위의 data-* 프로퍼티로 이벤트 프로퍼티 또한 집어넣고, 그것도 보내도록 코드를 짠다 (예시: data-amplitude-prop-tabs="안드로이드")

아하 그러면 이벤트 프로퍼티 개수에 따라서 엘리먼트 프로퍼티 또한 늘어나게 되는 건가요?
e.g. title, author, id.. 와 같은 이벤트 프로퍼티 존재

<li 
  data-amplitude-click-track="..."
  data-amplitude-prop-title="..."
  data-amplitude-prop-author="..."
  data-amplitude-prop-id="..."
...

@SeojinSeojin
Copy link
Member Author

그렇습니다 ..!! 좋은 포인트이네요.! 현재는 특성이 0~1개 정도지만 4개 이상만 되어도 코드가 길어지겠군요

track 함수 호출할 때도 속성이 많으면 코드가 길어져서 요 방법의 단점만은 아닐 것 같기도 합니다!!

객체 하나로 묶어서 보낼 방법을 생각해보아도 좋겠네요 ..!!! 최대한 깔끔하게 보낼 수 있도록!!

Copy link
Contributor

@f0rever0 f0rever0 left a comment

Choose a reason for hiding this comment

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

서진이가 얘기해준 훅으로 관리를 해보자는 아이디어는 좋은 것 같아요!
이 방법에 대해서는 고민을 해보고 메인페이지 개편 작업때 리팩토링으로 훅을 붙여보는 건 어떨까요?
(어떤 프로퍼티를 보내는지도 개편이후에 많이 달라질 것 같아서요. 이후에 수정에도 공수가 꽤 들지 않을까 생각이 들어서 제안합니다!)

@pull-request-size pull-request-size bot added size/S and removed size/M labels Sep 28, 2023
@SeojinSeojin SeojinSeojin merged commit 7073746 into develop Sep 28, 2023
1 check passed
@SeojinSeojin SeojinSeojin deleted the feat/#201_amplitude-event-tracking branch September 28, 2023 13:33
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.

앰플리튜드 이벤트 트레킹 - 메인, 프로젝트, 활동 후기
3 participants