-
Notifications
You must be signed in to change notification settings - Fork 58
fix(plugin-autocapture-browser): do not trigger rage click when text being highlighted #1471
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: main
Are you sure you want to change the base?
fix(plugin-autocapture-browser): do not trigger rage click when text being highlighted #1471
Conversation
- @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected]
This reverts commit b6afc2a.
This reverts commit db34376.
…cript into AMP-146045-rage-click-fix-mobile-scroll-false-positive
- @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected] - @amplitude/[email protected]
| unsubscribe: () => { | ||
| rageClickSubscription.unsubscribe(); | ||
| /* istanbul ignore next */ | ||
| selectionSubscription?.unsubscribe(); | ||
| }, |
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.
Pending rage‑click timeout isn’t cleared, so it can fire after teardown or a window reset and emit unexpectedly. Suggest always cancel and null any pending rageClick timer during teardown and whenever the click window is reset.
| unsubscribe: () => { | |
| rageClickSubscription.unsubscribe(); | |
| /* istanbul ignore next */ | |
| selectionSubscription?.unsubscribe(); | |
| }, | |
| unsubscribe: () => { | |
| if (pendingRageClick) { | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-argument | |
| clearTimeout(pendingRageClick.timerId); | |
| pendingRageClick = null; | |
| } | |
| rageClickSubscription.unsubscribe(); | |
| /* istanbul ignore next */ | |
| selectionSubscription?.unsubscribe(); | |
| }, |
🚀 Want me to fix this? Reply ex: "fix it for me".
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.
This is fine, this was here before. It just means that if it's unsubscribed and a rage click happens within the short window after unsubscribing, it will fire off one last event.
3696ef2 to
2f9f9a5
Compare
… of github.com:amplitude/Amplitude-TypeScript into AMP-146044-fix-highlightable-text-rage-clicks
… AMP-146045-rage-click-fix-mobile-scroll-false-positive
… of github.com:amplitude/Amplitude-TypeScript into AMP-146044-fix-highlightable-text-rage-clicks
… AMP-146044-fix-highlightable-text-rage-clicks
|
📝 Documentation updates detected! New suggestion: Document rage click text selection behavior |
Summary
Cancel a rage click if there is a text highlight event that happens after a click.
If a user clicks 1 or more times, and then text is highlighted, that suggests that the user's clicks caused the text to be highlighted
Here's a demonstration to show if you highlight text in an input, the first 3 clicks cause the text to be highlighted, which cancels out the rage click. You thus need at least 7 clicks for it to be a rage click (4 clicks after the text is done being highlighted).
Screen.Recording.2026-01-02.at.5.41.11.PM.mov
Checklist
Note
Prevents rage-click events when user text selection occurs by integrating a new observable and resetting detection state.
SelectionObservabletoAllWindowObservablesand wires it infrustration-pluginviadocument.selectionchange(handles inputs/textareas and generic selections)track-rage-clickto subscribe toselectionObservableandresetClickWindow()on selection; returns a composite subscription that unsubscribes both click and selection listenersObservablesEnum) to includeselectionObservablefrustration-plugin.test.tsandtrack-rage-click.test.tsWritten by Cursor Bugbot for commit 9eb5742. This will update automatically on new commits. Configure here.