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

throttle/debounce user callback after preventDefault/stopPropagation is executed #3479

Closed
wants to merge 2 commits into from
Closed

Conversation

hudon
Copy link
Contributor

@hudon hudon commented Mar 23, 2023

This fixes the issue outlined here: #3478

There is a tradeoff worth noting here and perhaps a breaking change. This fix implies that if a user types dragover.prevent.throttle, they do intend the dragover events to all preventDefault(), even the ones that end up not firing the callback because they're getting throttled. If some users however are depending on the current behavior and expecting the entire wrapped callback to be throttled (stopProp/preventDef included), this patch will break their use case. It would be inconsistent behavior, though, to have the event sometimes propagate and sometimes not.

If the approach is right and you want tests, please lmk

@joshhanley
Copy link
Collaborator

@hudon thanks for the PR! Yes, please add tests 🙂

Also, can you close and resubmit the PR using a feature branch? It is good practice to submit PRs from a different branch than your main branch in your fork, as maintainers can't typically modify/ contribute to the PR when submitted from the main branch. Thanks!

@hudon
Copy link
Contributor Author

hudon commented Mar 23, 2023

done and tested: #3481

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.

None yet

2 participants