Skip to content
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: dead click detection #1463

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

feat: dead click detection #1463

wants to merge 28 commits into from

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Oct 12, 2024

We autocapture valid webpage interactions

But some people will want to autocapture invalid interactions

for example in some places in posthog these "i" icons are clickable in others they aren't
Screenshot 2024-10-13 at 23 31 32

deadclick autocapture lets you detect clicks that are not followed by scroll or DOM mutation, so you can see if folk are clicking things thinking something will happen and nothing happens

## Needs

  • posthog PR for taxonomy
  • posthog PR for remote enabling
  • client config for thresholds
  • client config for autocapture allow/deny lists
  • e2e tests

Copy link

vercel bot commented Oct 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Oct 21, 2024 8:31am

Copy link

github-actions bot commented Oct 12, 2024

Size Change: +40.3 kB (+1.42%)

Total Size: 2.87 MB

Filename Size Change
dist/all-external-dependencies.js 180 kB +7.88 kB (+4.59%) 🔍
dist/array.full.es5.js 250 kB +1.59 kB (+0.64%)
dist/array.full.js 331 kB +5.86 kB (+1.8%)
dist/array.full.no-external.js 330 kB +5.86 kB (+1.81%)
dist/array.js 157 kB +1.48 kB (+0.95%)
dist/array.no-external.js 156 kB +1.48 kB (+0.96%)
dist/main.js 158 kB +1.48 kB (+0.95%)
dist/module.full.js 330 kB +5.86 kB (+1.8%)
dist/module.full.no-external.js 329 kB +5.86 kB (+1.81%)
dist/module.js 157 kB +1.48 kB (+0.95%)
dist/module.no-external.js 156 kB +1.48 kB (+0.96%)
ℹ️ View Unchanged
Filename Size
dist/dead-clicks-autocapture.js 9.67 kB
dist/exception-autocapture.js 8.75 kB
dist/external-scripts-loader.js 2.19 kB
dist/recorder-v2.js 92.6 kB
dist/recorder.js 92.7 kB
dist/surveys-preview.js 56.7 kB
dist/surveys.js 62.1 kB
dist/tracing-headers.js 1.33 kB
dist/web-vitals.js 10.3 kB

compressed-size-action

import { uuidv7 } from '../../uuidv7'
import { DeadClicksAutocapture } from '../../extensions/dead-clicks-autocapture'

describe('DeadClicksAutocapture', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this outer wrapper delegates entirely to the lazy loaded one so we can test it interacts with a mock as desired and ignore the behaviour

@@ -37,6 +37,202 @@ function limitText(length: number, text: string): string {
return text
}

export function getAugmentPropertiesFromElement(elem: Element): Properties {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bunch of functions move out of the class so we can use them directly because they (mostly) don't rely on state of the class

it should help with bundle size a tiny bit since the names are now minifiable

@pauldambra pauldambra marked this pull request as ready for review October 13, 2024 22:33
@pauldambra pauldambra requested a review from a team October 14, 2024 08:41
@robbie-c
Copy link
Contributor

Does selecting text trigger this? It's not a dom mutation or a scroll, but it's not dead either

@pauldambra
Copy link
Member Author

@robbie-c you get 2 hedgehog points for spotting that! fix incoming


export function getPropertiesFromElement(
elem: Element,
maskAllAttributes: boolean,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed this parameter which has always been wrong

@robbie-c
Copy link
Contributor

Related to text selection, I guess any interaction with form elements (e.g. clicking a select input) wouldn't change the dom but isn't dead either

@pauldambra
Copy link
Member Author

Related to text selection, I guess any interaction with form elements (e.g. clicking a select input) wouldn't change the dom but isn't dead either

fair point... made any autocapturable element not eligible for dead click detection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants