jondavidjohn / payform

:credit_card: A library for building credit card forms, validating inputs, and formatting numbers.
https://jondavidjohn.github.io/payform/
Other
427 stars 81 forks source link

Adds support for detaching events from input fields #62

Closed sergiocruz closed 5 years ago

sergiocruz commented 5 years ago

Detaching formatting helpers

This PR references issue #61 where I asked if there was a proper way to detach events from elements once we used the formatting helpers.

I asked that question because I am using payform in a Single Page Application environment where it's extra important to be able to detach event listeners from input elements before removing them from the DOM to avoid memory leaks.

API Changes

I created 4 new methods to make this possible (on both main plugin & jQuery implementation):

Approach

In order to property detach events from input elements, we have to keep a reference of the original function handler that was used to attach the event. To achieve this, I created a new variable that is called eventList. The eventList variable contains an object that holds all of our four input types (cvcInput, expiryInput, cardNumberInput, numericInput).

These input types hold arrays of objects containing the event names and their normalized handlers. I had to normalize the handlers here to ensure we kept the same reference of the handler function to be able to then use that same reference to remove the event listeners from elements.

On the methods formatting methods now, instead of making multiple calls to our _on() method, we run a for loop on the respective event types, therefore attaching the same events as before with their respective handlers, in the same order as before.

To achieve the actual detachment of events, I created a new internal method called _off(). I followed the jQuery fashion a la .on() and .off() methods for this.

Questions?

Please don't hesitate to add any questions, comments on concerns to this PR as I will be happy to address them.

Also feel free to add suggestions to different approaches I should have used instead as I will be happy to try them out as well.

Thanks you all :)

ddayguerrero commented 5 years ago

I did one last check and ran local tests, everything looks good to me!

ddayguerrero commented 5 years ago

@jondavidjohn We've made all the necessary changes as per the contribution guidelines. Would you mind doing a last check before I publish to npm? 🙂

sergiocruz commented 5 years ago

Hey @ddayguerrero or @jondavidjohn I see a 1.4.0 release here on GitHub but not on npm. Is that on purpose? I'd like to implement this functionality into my app.

Thank you!

jondavidjohn commented 5 years ago

@ddayguerrero Yeah, you're good to go, no need to get my approval 👍

ddayguerrero commented 5 years ago

@jondavidjohn perfect, thanks! 🎉 @sergiocruz expect an update by the end of the day :)

sergiocruz commented 5 years ago

Thank you @ddayguerrero, I'll be looking forward to seeing it!