EPMRPP-80423 || add google event for profile #3353
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3353 +/- ##
========================================
Coverage 62.03% 62.03%
========================================
Files 74 74
Lines 798 798
Branches 121 121
========================================
Hits 495 495
Misses 276 276
Partials 27 27 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| const BASIC_EVENT_PARAMETERS_PROFILE = { | ||
| action: 'click', | ||
| category: PROFILE_PAGE, | ||
| element_name: 'language_drop_down', |
There was a problem hiding this comment.
element_name can change very frequently. That is why I suppose to put that field in every event instead of putting it in BASIC_EVENT_PARAMETERS object. Or we can create a function, which takes element_name as a parameter and returns BASIC_EVENT_PARAMETERS object.
| @@ -33,7 +33,7 @@ import { | |||
| } from 'common/constants/supportedLanguages'; | |||
| import { langSelector, setLangAction } from 'controllers/lang'; | |||
| import { InputDropdown } from 'components/inputs/inputDropdown'; | |||
| import { PROFILE_PAGE_EVENTS } from 'components/main/analytics/events'; | |||
| import { PROFILE_EVENT } from 'components/main/analytics/events/ga4Events/profilePageEvent'; | |||
There was a problem hiding this comment.
We have an alias for that path. You can find it in basic webpack config
| @@ -78,6 +78,16 @@ const messages = defineMessages({ | |||
| }, | |||
| }); | |||
|
|
|||
| const langName = { | |||
There was a problem hiding this comment.
Maybe it is better to move that object to Profile page events?
And we already have these values in constants.
There was a problem hiding this comment.
I think it's more logical to leave the object with languages in the localizationBlock.jsx file than in ProfilePageEvents(analytic file).
And I didn't find the constants, could you show me where they are?
We have constants with codes, but not with language names (names in English)
There was a problem hiding this comment.
That is what I mean.
U can create object langName = { [ENGLISH]:'english' }
There was a problem hiding this comment.
you correct! fixed ✅
| zh: 'chinese', | ||
| }; | ||
|
|
||
| const langNameByCode = (code) => langName[code]; |
There was a problem hiding this comment.
Do we really need that function?
We can easily langName[lang].
And maybe LangNameMap will be a better name?
There was a problem hiding this comment.
I think this function in 1 line improves the readability of the code (langNameByCode(lang) more understandable than langName[lang]), and the name langNameByCode makes it clear what the function does, unlike LangNameMap
| action: 'click', | ||
| category: PROFILE_PAGE, | ||
| element_name: 'language_drop_down', | ||
| place: PROFILE_PAGE, |
There was a problem hiding this comment.
And also, discuss that property with WA. Because in the task description we do not have it.
There was a problem hiding this comment.
already WA added this property
EPMRPP-80423
change universal analytics to ga4