-
Notifications
You must be signed in to change notification settings - Fork 49
feat(analytics-browser): add dead + rage clicks to analytics-browser #1162
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
base: frustration-analytics
Are you sure you want to change the base?
feat(analytics-browser): add dead + rage clicks to analytics-browser #1162
Conversation
…m:amplitude/Amplitude-TypeScript into AMP-125613/add-dead-clicks-to-element-interactions
… AMP-125613/add-dead-clicks-to-element-interactions
… AMP-125613/add-dead-clicks-to-element-interactions
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.
Bug: Debug Statement Left In Code
A debug console.log('!!!options', options);
statement was accidentally left in the code and should be removed to prevent console output pollution.
packages/plugin-autocapture-browser/src/frustration-plugin.ts#L36-L37
Amplitude-TypeScript/packages/plugin-autocapture-browser/src/frustration-plugin.ts
Lines 36 to 37 in efb2109
): BrowserEnrichmentPlugin => { | |
console.log('!!!options', options); |
Bug: Test Name Mismatch
The test name for isFrustrationInteractionsEnabled
with an undefined
parameter ("should return true with undefined parameter") contradicts its assertion (toBe(false)
). The test name should be updated to "should return false with undefined parameter" to accurately reflect the expected behavior.
packages/analytics-browser/test/default-tracking.test.ts#L21-L24
Amplitude-TypeScript/packages/analytics-browser/test/default-tracking.test.ts
Lines 21 to 24 in efb2109
test('should return true with undefined parameter', () => { | |
expect(isFrustrationInteractionsEnabled(undefined)).toBe(false); | |
}); |
Comment bugbot run
to trigger another review on this PR
Was this report helpful? Give feedback by reacting with 👍 or 👎
📝 Documentation updates detected! New suggestion: Document frustrationInteractions autocapture option for Browser SDK |
✅ No documentation updates required. |
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! See nit.
@@ -160,6 +161,11 @@ export interface AutocaptureOptions { | |||
* @defaultValue `false` | |||
*/ | |||
elementInteractions?: boolean | ElementInteractionsOptions; | |||
/** | |||
* Enables/disables frustration interactions tracking. | |||
* @defaultValue `false` |
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.
based on the code it looks like the default value is true?
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.
It should be "false" by default. Just like elementInteractions. Is it not?
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 implemented "isFrustrationInteractionsEnabled" the same as "isElementInteractionsEnabled" so it should be the same
8aa11ab
to
27cbf45
Compare
06b3905
to
87970f9
Compare
…e-TypeScript into AMP-125613/config-prerelease
Summary
Add public API for capturing dead clicks and rage clicks via a new configuration called "autocapture.frustrationInteractions".
Checklist