# Activate submit button on form change and detach event listener

We have pages that contain 1-3 forms. Each form does have a submit button which is styled as inactive* as long as the user didn't change the form.

Now I need to:

• remove the class from the button as soon as the form changes
• remove the event listener from the form

I came up with this:

for (const form of Array.from(document.getElementsByClassName('form'))) {
const formHandlerChange = event => {
const submit = form.getElementsByClassName('disabled')[0];
submit.classList.remove('disabled');
form.removeEventListener('change', formHandlerChange);
};

}


Is this a good solution? Is it a bad thing to create formHandlerChange for each form? Would it be better to get the form each time formHandlerChange is entered manually and have this function defined only once outside the loop?

* It's also disabled but I removed it for simplicity of this review.

• Is it really necessary to remove the event handler after it triggers? In your case it would seem that you would just remove the disabled class more than once. What I prefer to do, instead of removing the event, is to check some condition. For example: if (submit.classlist.contains('disabled') { ... – Marc Rohloff Aug 29 '18 at 15:20
• Well, I think that getting the DOM element over and over again only to check whether a class is set already seems like a lot of overhead? However, I could add an extra class to the form like enabled and test this for event.target, but this only makes the code more complex than needed, right? What do you think? @MarcRohloff – lampshade Aug 31 '18 at 9:31
• I think that you're not going to notice a drop in performance for this kind of usage and you can always use Máté's suggestion of using currentTarget to avoid the lookup. I really don't think either direction would be wrong and you should do what you think is best. My rule of thumb is mostly just to be consistent across your code-base. – Marc Rohloff Aug 31 '18 at 19:04
• I also just noticed your asterisk comment. If you are disabling the button then why use a class at all? You can target it in css with the :disabled and :enabled pseudo-classes. developer.mozilla.org/en-US/docs/Web/CSS/:disabled – Marc Rohloff Aug 31 '18 at 19:06

If you use event.currentTarget instead of form, you can declare the event handler function once and reuse it:

function formHandlerChange(event) {
const form = event.currentTarget;
const submit = form.querySelector('.disabled');
submit.classList.remove('disabled');
form.removeEventListener('change', formHandlerChange);
}

for (const form of document.querySelectorAll('form')) {

(Note: I also used querySelector() and querySelectorAll() as they are more flexible than the various getElementBy methods)
• Thanks for querySelector - didn't have this in mind. A note: For the code ro tun in Edge unfortunately you need Array.from(), see https://stackoverflow.com/questions/47287344/edge-15-throws-error-when-using-for-of-on-a-nodelist. – lampshade Aug 31 '18 at 9:28