-
Notifications
You must be signed in to change notification settings - Fork 6
[Feat] #230 - Firebase Google Analytics 작업을 진행 하였습니다. #234
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
The head ref may contain hidden characters: "#230---GA-\uCD94\uAC00"
Conversation
cocoaPods로 라이브러리 이슈가 있어서 임시로 달아놨습니다.
보관함, 마이페이지, 방문자 모드에서 발생할 수 있는 이벤트를 추가하였습니다.
513sojin
left a comment
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.
수고하셨습니다 ~ 특히나 Event 하나하나 변수로 만드는게 쉽지 않았을텐디 ..... 피드백 하나 남겨두긴 했는데 고민해봐도 좋을 것 같아요 !
| extension SignInSocialLoginVC { | ||
| private func analyze() { | ||
| GAManager.shared.logEvent(eventType: .screen(screenName: Event.View.viewSocialLogin)) | ||
| } | ||
|
|
||
| private func kakaoLoginSuccessAnalyze() { | ||
| GAManager.shared.logEvent(eventType: .button(buttonName: Event.Button.clickKaKaoLogin)) | ||
| } | ||
|
|
||
| private func appleLoginSuccessAnalyze() { | ||
| GAManager.shared.logEvent(eventType: .button(buttonName: Event.Button.clickAppleLogin)) | ||
| } | ||
| } |
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.
제가 파악한 바로는 eventType에 해당하는 부분만 달라지는 것 같은데
GAManager.shared.logEvent(eventType:� )
UI 뷰컨 익스텐션으로
func analyze(_ eventType : EventType) {
GAManager.shared.logEvent(eventType: eventType)
}
analyze(.screen(screenName: Event.View.viewSocialLogin))
analyze(.button(buttonName: Event.Button.clickKaKaoLogin))
이런식으로 만들어서 사용하면 GA가 필요한 부분마다 새로 함수를 만들지 않아도 될 것 같은데 어떠신가요?!
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.
같은 VC에서 사용하는 것 중에 중복되는 부분이 있으니, 재사용해서 하는 게 낫다고 이해했는데 맞을까요!?
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.
네 마자여 !! 어차피 버튼에 단다고 해도 버튼 이벤트 처리를 뷰컨에서 할테니 UIViewController extension에 저기에 있는 analyze함수를 넣는게 어떻겠냐는 의견이었습니다 .
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.
확인 완료 ~~
|
|
||
| import Foundation | ||
|
|
||
| struct Event { |
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.
GAEvent 같이 조금 구체적인 이름은 어떨까요 ?!
저도 이벤트 주고 받을때 그냥 eventType 이런 식으로 많이 썼는데 다음에 코드 짤때는 좀 더 이름을 구체적으로 지어보려구요 ..^__^
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.
해당 부분 반영 하겠습니다 ~
| final class GAManager { | ||
| static let shared = GAManager() | ||
|
|
||
| private init() {} | ||
|
|
||
| enum EventType { |
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.
싱글톤으로 작성한 부분도, 타입 나눈 것도 그렇고 확실히 ga 함수 다는게 쉬워질 것 같아요 !! 고민 많이 하셨을텐데 너무 좋다고 봅니다 👍🏻
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.
감사합니다 ~
🌱 작업한 내용
🌱 PR Point
GAManager와GAEvent구조체로 작업을 하려는데 코드가 괜찮은지 ,,GA 다는 함수도 아직 전체적으로 넣진 않았어요 로그인 부분이나 초기 부분에 테스트로 심어놨습니다
왜냐하면 싱글톤으로 만든 클래스가 괜찮은지 보기 위해서
당연하지만
pod install해줘야 합니다 ~~GAManager는
싱글톤전역으로 사용 하고자 합니다.📸 스크린샷
📮 관련 이슈