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.

Sign up for GitHub

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

Jump to bottom

Javascript data-confirm fires multiple times #284

Open
tomciopp opened this issue Jan 28, 2020 · 4 comments
Open

Javascript data-confirm fires multiple times #284

tomciopp opened this issue Jan 28, 2020 · 4 comments

Comments

@tomciopp
Copy link

If you're using something like turbolinks, phoenix_html may get loaded multiple times. This leads to a situation where users will have to click the confirm dialog over and over to get the dialog to disappear.

Looking at the code and the docs for addEventListener, I believe there should be a simple fix to this problem.

If multiple identical EventListeners are registered on the same EventTarget with the same parameters, the duplicate instances are discarded. They do not cause the EventListener to be called twice, and they do not need to be removed manually with the removeEventListener() method.

Note, however that when using an anonymous function as the handler, such listeners will NOT be identical, because anonymous functions are not identical even if defined using the SAME unchanging source-code simply called repeatedly, even if in a loop.

It looks like the root of the problem is the fact that we are using anonymous functions when adding the event listener. If we can ensure that the functions are static when the code runs again the additional event listener will be ignored since it is identical. Since this is based off behavior from rails I think it would be good to borrow/steal their implementation since it should handle these problems out of the box.

https://github.com/rails/rails/tree/master/actionview/app/assets/javascripts
@josevalim
Copy link
Member

Makes sense. Can you please send a PR that moves to a named function? Thank you!

@tomciopp
Copy link
Author

@josevalim Sure, I will take a look at implementing a solution tomorrow.

This was referenced Jan 28, 2020
Closed
Open
@josevalim
Copy link
Member

Hi @tomciopp, so I asked around to know if the global is necessary and I was told that Turbolinks do not make it a requirement as long as you load scripts on the head, as advised by Turbolinks. Are you loading Phoenix HTML and friends on the <head> and the problem still persists?

@tomciopp
Copy link
Author

tomciopp commented Feb 3, 2020

Yes, everything is loaded in the <head> and the problem persists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bugfix(phoenix_html.js): Make IIFE idempotent by injecting a global _… tomciopp/phoenix_html
2 participants
@josevalim @tomciopp