analytics icon indicating copy to clipboard operation
analytics copied to clipboard

added form submit and link click tracking

Open RobertJoonas opened this issue 2 years ago • 4 comments

Changes

Added a tagged-events script extension with support for form submit and link click tracking.

Changelog

  • [x] Entry has been added to changelog

Documentation

  • [ ] Docs have been updated

RobertJoonas avatar May 30 '22 20:05 RobertJoonas

BundleMon

Unchanged files (7)
Status Path Size Limits
:white_check_mark: static/css/app.css
515.1KB -
:white_check_mark: static/js/dashboard.js
287.08KB -
:white_check_mark: static/js/app.js
12.13KB -
:white_check_mark: static/js/embed.host.js
5.58KB -
:white_check_mark: static/js/embed.content.js
5.06KB -
:white_check_mark: tracker/js/plausible.js
748B -
:white_check_mark: static/js/applyTheme.js
314B -

No change in files bundle size

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar May 30 '22 20:05 bundlemon[bot]

Good point about using the callbacks. I've now switched to using those. Although a delay of 5 seconds in case of a failure sounds a bit extreme. I currently put a 1s timeout. I think that's more than enough?

The latest commit here also fixes https://github.com/plausible/analytics/issues/1941. I've tested it with Fancybox myself and it seems to work all nicely. The way it works, is using the Event.defaultPrevented property to see if some other logic has intercepted. And if it has, it will not be the concern of our event handler so we'll stop worrying about continuing the default behavior.

Currently, from this point on we would have three extensions that do a very similar thing - listen for events and send a custom event to Plausible. These are outbound-links, file-downloads and the new tagged-events. I'm a little concerned about using more than one of these together. Let's say we're using plausible.outbound-links.file-downloads.js and an outbound file download link on our site is clicked. Then, only the first one of these requests is guaranteed to run to completion, because the second event handler will see that the default action is already prevented (as mentioned above).

I've been brainstorming heavily with this, but haven't figured out how to solve this. Any ideas on how to proceed?

RobertJoonas avatar Jun 02 '22 13:06 RobertJoonas

Although a delay of 5 seconds in case of a failure sounds a bit extreme. I currently put a 1s timeout. I think that's more than enough?

I think 1 second is too short. The delay is there to allow time for the POST /api/event request to finish. It's quite common for requests to that endpoint to take longer than 1 second. But it is very rare for them to take more than 5 seconds. In most cases, the request will finish in <500ms and the user will not notice a delay. So I'm not really concerned about the 5 seconds, it should be rare but it will give more accurate stats.

At least on our own /register page we use an up to 5 second delay and it seems to work well.

The latest commit here also fixes https://github.com/plausible/analytics/issues/1941. The way it works, is using the Event.defaultPrevented property to see if some other logic has intercepted.

Nice! I think this makes sense in general and makes our script a better citizen. If some external script prevented the default then it's reasonable to expect them to continue the action.

Then, only the first one of these requests is guaranteed to run to completion, because the second event handler will see that the default action is already prevented (as mentioned above).

Yeah, this is a concern. But since we control all script extensions, we can implement some coordination between them. So there could be some central data structure + function in our script, that all of these 3 extensions use, that can deal with the concept of 2 extensions BOTH wanting to send their respective custom events. This central function can just fire off requests and each time one finished, check if there are any other in-progress requests. IDK if this makes sense to you, but that's how I would start solving this.

ukutaht avatar Jun 03 '22 07:06 ukutaht

I sketched the idea I had below. Basically it replaces the linkOpened variable with inFlightRequests which is able to track how many requests are still waiting for a response and will only follow the link once all requests have finished.

🚧 Untested pseudocode. May not work at all as advertised 🚧

var inFlightRequests = {}

function eventCallback(eventName, continueFn) {
  return function() {
    Map.delete(inFlightRequests, eventName)

    if (Map.keys(inFlightRequests).length == 0) {
        continueFn()
    }  
  }
}

function sendEventAndThen(eventName, eventProps, continueFn) {
  inFlightRequests[eventName] = true

  plausible(eventName, {props: eventProps, callback: eventCallback(eventName, continueFn)})
  setTimeout(eventCallback(eventName, continueFn), 5000)
}

...

var followLink = function() { ... }
sendEventAndThen('Outbound Link: Click', {url: 'url.com'}, followLink)
sendEventAndThen('File download', {url: 'url.com'}, followLink)

// should wait for both to finish and only then follow the link

ukutaht avatar Jun 03 '22 08:06 ukutaht

@RobertJoonas I'm closing this PR as stale since it has so many conflicts and depends on stuff that is changing anyways.

The branch will remain so you can reference the code here. But I'll wait for another PR once the refactoring is finished.

ukutaht avatar Sep 08 '22 12:09 ukutaht