-
Notifications
You must be signed in to change notification settings - Fork 5
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
Feat: add Osano MGMT #179
Feat: add Osano MGMT #179
Conversation
Visit the preview URL for this PR (updated for commit 619f4b0): https://estuary-marketing--pr179-feat-osano-mgmt-kf1jyikt.web.app (expires Sun, 18 Feb 2024 18:32:54 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
Great - this follows the approach called out in the Osano docs. Depending on needs we might want to move this script up in the future. It should load before any other ones. But I checked two other popular brand sites I know use Osano and they are loading exactly as we are so I think we are fine. |
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.
I'm going to hold off on approving this until @dyaffe says go for it, since it also adds Hotjar again. We've added and removed hotjar in the past because it hurts our google pagespeed numbers when we add it, so I don't want to go back and forth again. If you throw up a PR that just adds Osano I'll approve it now 👍
I should add that including Osano will more than likely make an impact on performance. Google will more than likely see it as a blocking scripts that makes other scripts wait on it for loading. However, this is literally the feature that Osano provides so I am not really sure how much can be done about that. Wanting to call this out now so no one is surprised. Also, if Osano is already going to make a potential impact it might be easier to push the addition of HotJat in a stand alone PR to make figuring out performance stuff easier. |
Why are we adding Hotjar? As Joseph mentioned, I'm pretty anxious about adding that in due to page speed issues with Google SEO metrics. |
Removed the hotjar until we figure out a better solution to it |
When are we planning to merge this? |
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.
LGTM
https://cmp.osano.com/16CPXbTOi1sXx4D3/1e6b223c-ed10-4c4b-a442-48fea69f76af/osano.js