Log click events on search homepage and repogroup page #12405
Conversation
…ked on repogroup page
web/src/repogroups/RepogroupPage.tsx
Outdated
| @@ -182,11 +182,13 @@ export const RepogroupPage: React.FunctionComponent<RepogroupPageProps> = (props | |||
| ) | |||
| } | |||
|
|
|||
| const RepoLinkClicked = (): void => eventLogger.log('RepogroupPageRepoLinkClicked') | |||
There was a problem hiding this comment.
I wonder if we want to pass in any data with the event? Such as repo name in this case? I'm not sure if we are able to do that but it seems that slicing these events based on associated data can be useful
There was a problem hiding this comment.
If we can pass along data about the specific click that'd be ideal!
There was a problem hiding this comment.
@ebrodymoore pass through the actual language/example that was clicked?
There was a problem hiding this comment.
+1, would be great if we could pass through the link clicked on:
RepogroupPageRepoLinkClicked -> Repo name or search url
ExampleSearchClicked -> search url
ExampleLanguageSearchClicked -> Language or search url
|
@attfarhan the naming convention is good! |
Codecov Report
@@ Coverage Diff @@
## master #12405 +/- ##
==========================================
- Coverage 50.62% 50.55% -0.07%
==========================================
Files 1415 1414 -1
Lines 79235 79133 -102
Branches 6539 6539
==========================================
- Hits 40111 40004 -107
- Misses 35666 35674 +8
+ Partials 3458 3455 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -61,8 +61,9 @@ interface Props | |||
| showCampaigns: boolean | |||
| } | |||
|
|
|||
| const SearchExampleClicked = (): void => eventLogger.log('ExampleSearchClicked') | |||
| const LanguageExampleClicked = (): void => eventLogger.log('ExampleLanguageSearchClicked') | |||
| const SearchExampleClicked = (url: string) => (): void => eventLogger.log('ExampleSearchClicked', { url }) | |||
There was a problem hiding this comment.
It seems the the url itself might not as human readable and stable, maybe give them human readable name?
SearchExampleClicked("js_alert_calls")There was a problem hiding this comment.
I'm going to avoid this for now, since examples can change down the line, and a naming convention for each could be imprecise. We pass URLs for other events so it's not an uncommon thing to see in our codebase. @ebrodymoore any objections here?
There was a problem hiding this comment.
No objections from me. Thanks @attfarhan
| @@ -185,11 +185,14 @@ export const RepogroupPage: React.FunctionComponent<RepogroupPageProps> = (props | |||
| ) | |||
| } | |||
|
|
|||
| const RepoLinkClicked = (repoName: string) => (): void => | |||
There was a problem hiding this comment.
Plain functions that are not React components should be camelCased (same for other functions)
| @@ -64,6 +64,10 @@ interface Props | |||
| globbing: boolean | |||
| } | |||
|
|
|||
| const SearchExampleClicked = (url: string) => (): void => eventLogger.log('ExampleSearchClicked', { url }) | |||
| const LanguageExampleClicked = (language: string) => (): void => | |||
There was a problem hiding this comment.
Instead of currying these functions, you could also just define the function as a closure in the component
Fixes #12356.
Adds 3 event to our logging:
RepogroupPageRepoLinkClickedevent when clicking on repository links on the repogroup pageExampleSearchClickedwhen an example search is clicked on the homepageExampleLanguageSearchClickedwhen an example language is clicked on the homepage@ebrodymoore do these names suffice or should they be follow a certain format?