iLoveElysia/pjax
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 9 Wiki Activity

there is something wrong when pjax running with google analytics. #56

New Issue
Closed
opened 2016-01-12 05:13:15 -05:00 by coolhihi · 12 comments
coolhihi commented 2016-01-12 05:13:15 -05:00 (Migrated from github.com)
Copy link
Copy source

thank you for this great pjax.

but I may found a bug when it run with google analytics.

options.analytics() didn't have any param. so ga("send", "pageview", {"page": options.url, "title": options.title}) got an error Cannot read property 'url' of undefined.

I fixed it by replaced with ga("send", "pageview", {"page": location.pathname, "title": document.title}).

thank you for this great pjax. but I may found a bug when it run with google analytics. `options.analytics()` didn't have any param. so `ga("send", "pageview", {"page": options.url, "title": options.title})` got an error `Cannot read property 'url' of undefined`. I fixed it by replaced with `ga("send", "pageview", {"page": location.pathname, "title": document.title})`.
darylteo commented 2016-01-14 17:56:03 -05:00 (Migrated from github.com)
Copy link
Copy source

@coolhihi hi can you tell us which version of pjax you are using?

@coolhihi hi can you tell us which version of pjax you are using?
coolhihi commented 2016-01-14 22:03:38 -05:00 (Migrated from github.com)
Copy link
Copy source

@darylteo v0.1.4
I get it by npm install pjax

@darylteo v0.1.4 I get it by `npm install pjax`
darylteo commented 2016-01-14 22:04:43 -05:00 (Migrated from github.com)
Copy link
Copy source

@coolhihi we are currently working on a new release 0.2.2. Perhaps when we do release it, you can test your stuff to see if this issue is still present?

@coolhihi we are currently working on a new release 0.2.2. Perhaps when we do release it, you can test your stuff to see if this issue is still present?
coolhihi commented 2016-01-14 22:17:52 -05:00 (Migrated from github.com)
Copy link
Copy source

@darylteo Thank you. I can only got 0.2.1 by npm. Is the master HEAD 0.2.2? Or which way I can get 0.2.2?

@darylteo Thank you. I can only got 0.2.1 by npm. Is the master HEAD 0.2.2? Or which way I can get 0.2.2?
darylteo commented 2016-01-14 22:34:46 -05:00 (Migrated from github.com)
Copy link
Copy source

@coolhihi You will need master HEAD. 0.2.1 is broken and will not work.

@coolhihi You will need master HEAD. 0.2.1 is broken and will not work.
coolhihi commented 2016-01-14 22:40:14 -05:00 (Migrated from github.com)
Copy link
Copy source

@darylteo I found that google analytics was removed in new version. But that's okay, I will add it myself. thanks

@darylteo I found that google analytics was removed in new version. But that's okay, I will add it myself. thanks
darylteo commented 2016-01-14 22:40:31 -05:00 (Migrated from github.com)
Copy link
Copy source

Please PR =)

~~Please PR =)~~
darylteo commented 2016-01-14 22:46:36 -05:00 (Migrated from github.com)
Copy link
Copy source

@coolhihi actually I suspect that the reason why it was removed was because you can easily do this by just adding a listener to document (pjax:complete) then firing the appropriate GA event. This would be the most appropriate thing to do as we cannot assume everyone is using Google Analytics.

@coolhihi actually I suspect that the reason why it was removed was because you can easily do this by just adding a listener to document (pjax:complete) then firing the appropriate GA event. This would be the most appropriate thing to do as we cannot assume everyone is using Google Analytics.
MoOx commented 2016-01-17 03:11:24 -05:00 (Migrated from github.com)
Copy link
Copy source

We should at least add doc for this, so I am reopening.

We should at least add doc for this, so I am reopening.
coolhihi commented 2016-01-17 06:43:58 -05:00 (Migrated from github.com)
Copy link
Copy source

@MoOx @darylteo the HEAD version also have this bug. I had sent pull request. But I agree with darylteo the google analytics code is not necessary. Google may change the api in the future, and users can easily do this in pjax:complete or pjax:success. So I suggest to remove the code about google analytics.

@MoOx @darylteo the HEAD version also have this bug. I had sent pull request. But I agree with darylteo the google analytics code is not necessary. Google may change the api in the future, and users can easily do this in `pjax:complete` or `pjax:success`. So I suggest to remove the code about google analytics.
kulakowka commented 2016-01-28 13:41:31 -05:00 (Migrated from github.com)
Copy link
Copy source

I have this bug too. I solve this problem with custom analytics function like this:

new Pjax({
  analytics: function () {
    window.ga('send', 'pageview', {page: document.location.pathname, title: document.title})
  }
})
I have this bug too. I solve this problem with custom analytics function like this: ``` javascript new Pjax({ analytics: function () { window.ga('send', 'pageview', {page: document.location.pathname, title: document.title}) } }) ```
MoOx commented 2016-03-12 01:25:27 -05:00 (Migrated from github.com)
Copy link
Copy source

Closed by #59

Closed by #59
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
bug
dependencies
Pull requests that update a dependency file
 duplicate
enhancement
invalid
question
wontfix
No labels bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
iLoveElysia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#56