-
Notifications
You must be signed in to change notification settings - Fork 7
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엘리먼트의 프로퍼티로 넣어둬도 괜찮을 것 같다는 생각이 들어용
오 생각지도 못한 굳 아이디어입니다!! 👍👍👍
다만 몇 가지 질문이 있습니다.
- 이벤트 리스너는 어디에 작성하실 생각인가요?
1-a. 훅? 훅이라면 훅은 어디에서 불러올지? - 프로퍼티가 있는지 확인하고자 할 때
e.target.프로퍼티명
과 같이 체크하게 되나요? 이벤트의 타입을MouseEvent
로 지정하면 커스텀 프로퍼티 속성이 없다고 뜨는데 타입은 어떤식으로 지정하실 생각인가요? - 엘리먼트의 프로퍼티는
string
타입만 가능하니 이벤트 프로퍼티가 있는 경우 추가적인 코드 작성이 필요한데 이렇게 되면 원래의 목적인 개발의 편리함을 벗어나게 되는 것이 아닌가 하는 생각이 듭니다. 뭔가 생각하신 방안이 있으실까요?
const onTabClick = (tab: ExtraPart) => { | ||
track('click_review_part', { part: tab }); | ||
setSelectedTab(tab); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여러분 Part 등은 다 영어로 가도록 하셨나여 ..???
저는 한글로 보냈습니다! tab.label
로 보냈어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 tab.value
로 영어로 보냈는데 통일이 필요할 것 같네요!
이벤트 리스너는 훅 형태로 만들고 _app.tsx 에서 불러오게 될 것 같습니다!!
data-* 로 프로퍼티 이름을 쓰면 element.dataset 로 접근할 수 있어요!! 타입스크립트에서 저것을 써보지 않았지만, 불가능할 것 같지는 않습니다 .!.!
보통 프로퍼티는 그 컴포넌트에서 큰 힘 들이지 않고 접근 가능한 방법이니 아래 방법들이 가능할 것 같습니다! 위의 data-* 프로퍼티로 이벤트 프로퍼티 또한 집어넣고, 그것도 보내도록 코드를 짠다 막 정규표현식 돌리고 먼가 해야겠지만 .. 프로퍼티 개수가 막 100개 넘어가지는 않으니 괜찮을 것 같다고 생각이 들어요 ..!! |
아하 그러면 이벤트 프로퍼티 개수에 따라서 엘리먼트 프로퍼티 또한 늘어나게 되는 건가요? <li
data-amplitude-click-track="..."
data-amplitude-prop-title="..."
data-amplitude-prop-author="..."
data-amplitude-prop-id="..."
... |
그렇습니다 ..!! 좋은 포인트이네요.! 현재는 특성이 0~1개 정도지만 4개 이상만 되어도 코드가 길어지겠군요 track 함수 호출할 때도 속성이 많으면 코드가 길어져서 요 방법의 단점만은 아닐 것 같기도 합니다!! 객체 하나로 묶어서 보낼 방법을 생각해보아도 좋겠네요 ..!!! 최대한 깔끔하게 보낼 수 있도록!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서진이가 얘기해준 훅으로 관리를 해보자는 아이디어는 좋은 것 같아요!
이 방법에 대해서는 고민을 해보고 메인페이지 개편 작업때 리팩토링으로 훅을 붙여보는 건 어떨까요?
(어떤 프로퍼티를 보내는지도 개편이후에 많이 달라질 것 같아서요. 이후에 수정에도 공수가 꽤 들지 않을까 생각이 들어서 제안합니다!)
Summary
앰플리튜드 이벤트 트래킹을 붙입니다.
Comment
여러분 Part 등은 다 영어로 가도록 하셨나여 ..???
그리고 click 이벤트를 한땀한땀 붙이다가 생각한 것은, 엘리먼트의 프로퍼티로 넣어둬도 괜찮을 것 같다는 생각이 들어용
AS-IS
TO-BE
어딘가에서
이벤트 위임 방식인데요, 클릭리스너 하나라 부하가 심할 것 같지도 않고 앞으로 개발하기도 편할 것 같아서 의견을 내 봅니다!!
(물론 커스텀 파라미터 전해줘야 할 때는 컴포넌트에서 직접 쏴주거나, 다른 방법을 찾아보아야 합니당...)
@solar3070 @f0rever0 님 의견을 듣고 싶습니다!!!!