search

HarshdipD / eztrackr

v3 of Eztrackr's Chrome extension. Designed to ease your job hunt by adding your jobs in an organized Trello board ✨
https://hsdeogan.com/eztrackr/
GNU General Public License v3.0
24 stars 11 forks source link

Google Analytics Added #21

Closed Bhavesh27 closed 4 years ago

HarshdipD commented 4 years ago

@Bhavesh27 Can you please explain how to make the analytics work on the dev and prod environment in this PR thread? I have my analytic ID added, and I want to replicate and see live data like button click number before I can push this to main. Screenshots would help! Thanks again!

HarshdipD commented 4 years ago

@Bhavesh27 just waiting on one comment before I merge this!

Bhavesh27 commented 4 years ago

@HarshdipD Sorry. I got occupied. Actually In the google sample example we have Multiple Buttons thats why we are passing second argument as event.target.id. So, as to recognize which button is clicked. On our Project, we only have one button, Add To Trello and thats what i have put up as second arguement of the GA call to recognize that button is clicked. PR is Good to Go from my end.

HarshdipD commented 4 years ago

@Bhavesh27 No worries! I got what you mean, that works! Also, in my previous review, I mentioned if you can track the settings page and Authorize button too, as it falls under this same issue. Additionally, I think the code might have merge conflicts now because the main branch has been updated a couple of time, could you please make sure it's fine from your end? Thank you!

Bhavesh27 commented 4 years ago

@Bhavesh27 No worries! I got what you mean, that works! Also, in my previous review, I mentioned if you can track the settings page and Authorize button too, as it falls under this same issue. Additionally, I think the code might have merge conflicts now because the main branch has been updated a couple of time, could you please make sure it's fine from your end? Thank you!

@HarshdipD Sure i will resolve the merge conflicts.

Also On settings html file, Popup.js is included so we are already firing the pageview event for Settings Page. Event for Authorize Button can be added.

My doubt is if we want Settings Page to be Different i can extract the analytics code from Popup.js and create a separate .js file that can be added to both popup.html and settings.html and we can remove popup.js from the settings page.

HarshdipD commented 4 years ago

@Bhavesh27 This works perfectly! Thank you so much for this PR, and I'm sure these analytics will help the tool become better by tracking user activities! I'll merge this PR now. Thanks again! Feel free to take any other issue if you're interested and star this repo!