Story/google analytics #48
Conversation
…itle and also triggers a page change event
…ere is a form to submit
…ent for future user research
…into view when it is embedded in another site
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
| ga && pageTitleAsScreen(ga, t(screen.title)); | ||
|
|
||
| //triggers a new event called page_change that shares your current page and next page | ||
| ga && pageChange(ga, t(screen.title), t(SCREENS[nextScreen].title)); |
There was a problem hiding this comment.
Do we want the translated version or the English version of the title? Should we perhaps ask design about this?
There was a problem hiding this comment.
Probably just English because the only people looking at this data will be the agency webmasters
There was a problem hiding this comment.
The changeScreen function gets called whenever a button is clicked, which doesn't always correspond to whether or not there's a new screen (i.e. if there's an error in one of the input fields, changeScreen will still get called even if the module doesn't actually move on to the next screen). We should probably move this to right before the setScreen gets called (I believe I refactored this changeScreen function quite a bit, so the placement would be something we'd probably have to figure out after merging that).
There was a problem hiding this comment.
Alternatively, I think we could also just add this to the useEffect where screen is a dependency? not 100% sure on this though
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
| @@ -0,0 +1,21 @@ | |||
| export function trackFutureResearch(ga) { | |||
There was a problem hiding this comment.
Instead of passing in ga to all of these methods, could we refactor this into a hook and just have useGA4React there since they all need ga anyways?
feedback-module/src/index.js
Outdated
| ); | ||
| Div.attributes.gaid && (gaid = Div.attributes.gaid.value); | ||
|
|
||
| const ga4react = new GA4React(gaid); |
There was a problem hiding this comment.
Do we need to create a new GA4React object every time the app is re-rendered (i.e. could we put this line outside of the renderApp function)? Not too familiar with how GA4React works, so I'm just wondering.
There was a problem hiding this comment.
Right now we are getting the gaid attribute within the render app which is why I put this here. I guess we could make the assumption that the user won't change their gaid so we wouldn't need to observe for it?
There was a problem hiding this comment.
Hmmm that's a good point— I guess we can keep it how it is just in case?
|
|
||
| const headerRef = useRef(null); | ||
| const firstCheckRef = useRef(null); | ||
|
|
||
| const { t, i18n } = useTranslation(); | ||
| const en = i18n.getFixedT("en"); | ||
|
|
||
| const ga = useGA4React(); | ||
|
|
||
| const ref = useRef(); |
There was a problem hiding this comment.
Should probably use a more descriptive name for this ref since there are other refs in this file
…ame of ref to be more descriptive
| import { useEffect, useState } from "react"; | ||
|
|
||
| export default function moduleOnScreen(ref) { | ||
| const [isIntersecting, setIntersecting] = useState(false); |
There was a problem hiding this comment.
Could we move const [userViewed, setUserViewed] = useState(false); here, move checkVisible() to this file as well and change the if statement to if (!userViewed && isIntersecting), and then call checkVisible() after line 7? I think we could also combine this hook with the googleAnalytics() hook, so that we have easy access to the userViewedModule() method.
There was a problem hiding this comment.
The only problem with this is wouldn't it reset userViewed to false every time. We only want to send this event once
There was a problem hiding this comment.
I don't think so— we never call setUserViewed(false) and since we're not actually changing/reloading the page or anything like that, moduleOnScreen(ref) only gets called once so the state should stay the same on all the screens; I don't think it's functionally any different than having the useState(false) for userViewed in Module.js.
There was a problem hiding this comment.
I'll test it out and see what happens
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
| @@ -130,6 +114,8 @@ function Module({ pagetitle, endpoint, dir }) { | |||
| focusFirstError(); | |||
| } else { | |||
| updateOtherField(); | |||
| pageTitleAsScreen; | |||
There was a problem hiding this comment.
Do we not need to provide any arguments here?
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
| @@ -100,6 +114,8 @@ function Module({ pagetitle, endpoint, dir }) { | |||
| focusFirstError(); | |||
| } else { | |||
| updateOtherField(); | |||
| pageTitleAsScreen(en(screen.title)); | |||
| pageChange(en(screen.title), en(SCREENS[nextScreen].title)); | |||
There was a problem hiding this comment.
Should be en(screen.title, { page: pagetitle }) (same for other one) bc of the string interpolation in our translation config.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-48.eastus2.azurestaticapps.net |
No description provided.