-
Notifications
You must be signed in to change notification settings - Fork 18
refactor: change RAP interface design #211
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
Changes from all commits
8702d43
edb6bc7
cec9c56
36dc600
de8326f
03faef8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,49 +1,49 @@ | ||
| // import { logEvent } from "@every-analytics/react-analytics-provider"; | ||
| import React from 'react'; | ||
| import navigate from 'router/navigate'; | ||
| import {useAnalyticsContext} from '@every-analytics/react-analytics-provider'; | ||
| import {useAnalytics} from '@every-analytics/react-analytics-provider'; | ||
|
|
||
| const NavBar = () => { | ||
| const analytics = useAnalyticsContext(); | ||
| const analytics = useAnalytics(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A. 관례긴 한데 요즘은 생략도 많이 합니다. 우리도 생략해요~ 👍 |
||
|
|
||
| return ( | ||
| <header className="App-header"> | ||
| <NavItem | ||
| href="/" | ||
| onClick={() => { | ||
| analytics.onEvent('Click logo'); | ||
| analytics.trackEvent('Click logo'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아예 으로 짧게가면 어떨까요? ㅎㅎ 굿
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. set에 track이 안 붙은 이유는 아래 스레드에 있어요
|
||
| }} | ||
| > | ||
| Fruit Store | ||
| </NavItem> | ||
| <NavItem | ||
| href="/products?color=red" | ||
| onClick={() => { | ||
| analytics.onEvent('Click products', {color: 'red'}); | ||
| analytics.trackEvent('Click products', {color: 'red'}); | ||
| }} | ||
| > | ||
| Red | ||
| </NavItem> | ||
| <NavItem | ||
| href="/products?color=yellow" | ||
| onClick={() => { | ||
| analytics.onEvent('Click products', {color: 'yellow'}); | ||
| analytics.trackEvent('Click products', {color: 'yellow'}); | ||
| }} | ||
| > | ||
| Yellow | ||
| </NavItem> | ||
| <NavItem | ||
| href="/login" | ||
| onClick={() => { | ||
| analytics.onClick('Click login', {color: 'yellow'}); | ||
| analytics.trackClick('Click login', {color: 'yellow'}); | ||
| }} | ||
| > | ||
| Login | ||
| </NavItem> | ||
| <NavItem | ||
| href="/set-currency" | ||
| onClick={() => { | ||
| analytics.onSet({currency: 'KRW'}); | ||
| analytics.set({currency: 'KRW'}); | ||
| }} | ||
| > | ||
| Currency | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,48 +1,20 @@ | ||
| import * as React from 'react'; | ||
|
|
||
| import AnalyticsProviderContext from '../../contexts/AnalyticsProviderContext'; | ||
| import {UnknownRecord} from '../../types/common'; | ||
| import {Analytics} from '../../core'; | ||
|
|
||
| interface Props { | ||
| onInitialize(): void; | ||
| onPageView?(params?: UnknownRecord): void; | ||
| onEvent?(name: string, params?: UnknownRecord): void; | ||
| onClick?(name: string, params?: UnknownRecord): void; | ||
| onSet?(...args: [string, UnknownRecord] | [UnknownRecord]): void; | ||
| onSetUserId?(userId: string | null): void; | ||
| onSetUserProperty?(params: UnknownRecord): void; | ||
| analytics: Analytics; | ||
| children: React.ReactNode; | ||
| } | ||
|
|
||
| export function AnalyticsProvider({ | ||
| onInitialize, | ||
| onPageView = () => null, | ||
| onEvent = () => null, | ||
| onClick = () => null, | ||
| onSet = () => null, | ||
| onSetUserId = () => null, | ||
| onSetUserProperty = () => null, | ||
| children, | ||
| }: Props) { | ||
| export function AnalyticsProvider({analytics, children}: Props) { | ||
| React.useEffect(() => { | ||
| onInitialize(); | ||
| }, [onInitialize]); | ||
| analytics.initialize(); | ||
| }, [analytics]); | ||
|
|
||
| return React.useMemo( | ||
| () => ( | ||
| <AnalyticsProviderContext.Provider | ||
| value={{ | ||
| onPageView, | ||
| onEvent, | ||
| onClick, | ||
| onSet, | ||
| onSetUserId, | ||
| onSetUserProperty, | ||
| }} | ||
| > | ||
| {children} | ||
| </AnalyticsProviderContext.Provider> | ||
| ), | ||
| [children, onClick, onEvent, onPageView, onSet, onSetUserId, onSetUserProperty], | ||
| () => <AnalyticsProviderContext.Provider value={analytics}>{children}</AnalyticsProviderContext.Provider>, | ||
| [children, analytics], | ||
| ); | ||
| } |
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.
track이 붙은곳과 아닌곳이 혼재되어있어 헷갈릴거같아요
동사(set)로 시작하는곳엔 track을 생략해주신거같은데
'set을 추적한다' 니까 trackSet이 더 맞지 않을까요?
track- 과 on- 중에 어떤게 더 직관적인지 고민고민..
Uh oh!
There was an error while loading. Please reload this page.
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.
track이 붙은 건 이벤트 추적에(기록) 사용되는 것이고, track이 안 붙은 경우는 이벤트 추적에 사용되는 메서드가 아니에요! set은 이벤트 기록이(tracking) 아니라 UserProperty 같은 걸 저장할 때 쓰는 거라고 이해했기 때문에 trackSet이 아니라 set이라고 썼어요
react-analytics-provider/src/utils/googleAnalytics/set.ts
Line 9 in d381609