AdrianVillamayor / Wizard-JS

A lightweight wizard UI component that supports accessibility and HTML5 in JavaScript Vanilla.
https://adrianvillamayor.github.io/Wizard-JS/
MIT License
44 stars 13 forks source link

setNavEvent and setBtnEvent register multiple times event listeners if wizard.init() is called multiple times #3

Closed aleaforny closed 2 years ago

aleaforny commented 2 years ago

Describe the bug If wizard.init() is called multiple times within a lifecycle of a page (for instance, if it's initialized after an AJAX call), the listeners registered by setNavEvent and setBtnEvent will be called as many times as wizard.init() has been called.

In my case, I was using a wizard from a Jconfirm Dialog content retrieved by AJAX (thus, I must initialize the Wizard object when the AJAX content is loaded, and not before as the DOM doesn't exist at the initial loading of the page). If I reopened the Jconfirm Dialog multiple times, it would produce very strange behaviors with the Wizard (not validating the form elements, skipping steps, buttons wouldn't work every time, etc.)

To Reproduce Steps to reproduce the behavior (requires content loaded by AJAX within a Jconfirm Dialog):

  1. Make sure to initialize Wizard (wizard.init()) in jConfirm onContentReady callback
  2. In Wizard.js, add a console.log("click") in the setBtnEvent() function, on the delegate function
  3. On the browser, launch the jConfirm dialog, click on the "Next" button of the Wizard. "click" appears on the console once.
  4. Close the jConfirm dialog and reopen it.
  5. Click again on the "Next" button. Notice that "click" appears two times, now. And it will add by 1 every time you call back the dialog.

Expected behavior The wizard.init() should register only one event listener for the elements, or at least clear the events before registering again, to deal with these async needs.

In the Steps to reproduce, it should only display "click" one time, no matter how many times the jConfirm dialog is opened.

Screenshots N/A

Workaround I'm not a VanillaJS pro, so I dealt my issue with jQuery as I use it in my project.

I changed this in the setNavEvent:

$_.delegate(document, "click", this.wz_nav + " " + this.wz_step, function (event) {

By that :

$(this.wz_nav + " " + this.wz_step).off().click(function(event) {

And I changed this in the setBtnEvent :

$_.delegate(document, "click", this.wz_buttons + " " + this.wz_button, function (event) {

By that :

$(this.wz_buttons + " " + this.wz_button).off().click(function(event) {

But I'm sure there is a better way to fix it, and not sure that workaround solve everything (it solves my problem in my case).

Notice the .off() jQuery call I'm doing to unbind all the previously binded events to these elements.

AdrianVillamayor commented 2 years ago

Hi @aleaforny ,

Thanks for letting me know, I'll get right on it.

Sorry for the lapse in response time.

AdrianVillamayor commented 2 years ago

Hi @aleaforny ,

A listener cleanup has been added in the init() to avoid duplicating them in case they already exist.

On the other hand I have updated the event system, now it is much more flexible and easy to manage.

You have all the changes here and explained in the README.

I hope I solved your problem, anything else let me know.

Thank you very much for your help πŸ™ŒπŸ».

aleaforny commented 2 years ago

Amazing @AdrianVillamayor thanks for your responsiveness. I didn't help by the way, you did all the work, thanks πŸ˜„