1
\$\begingroup\$

I've built out a section on a page that contains multiple instances (why I'm using querySelectorAll()) of this Request Brochure form. They are using Campaign Monitor so much of the form code has been removed for this post.

Operation

  • On page load an event handler that calls preventDefault on click events is added to elements with class download to prevent the submit action.
  • A class of .is-open is added to the .download container.
  • This triggers a keyframe animation that transitions the height, then the opacity of the forms
  • The event listener is removed to allow the button to perform it's submit action

I did try using the method of setting the button to disabled initially, then removing this attribute onclick but found this meant the hover states would not work.

I'm looking to see if this is the most efficient way of adding, then removing preventDefault once it is no longer required.

const packageCMCont = document.querySelectorAll('.download');

packageCMCont.forEach(function(item) {

  function handleClick(e) {
    e.preventDefault()
    var btn = item.querySelector('.btn');

    item.classList.add('is-open');

    btn.classList.remove('is-style-outline');
    btn.classList.add('is-style-default');

    item.removeEventListener('click', handleClick);
  }

  item.addEventListener('click', handleClick);

})
.download {
  width: 100%;
  max-width: 600px;
  margin: 0 auto;
 }
.download.is-open .inputs {
    display: flex;
    height: 100px;
    animation: showForms 3s forwards;
}
.inputs {
  opacity: 0;
  display: none;
  flex-direction: column;
  height: 0;
  overflow: hidden;
}
input {
  height: 50px;
}
button {
  height: 50px;
  width: 100%;
  background-color: green;
  color: white; 
}
button:hover {
  background-color: white;
  color: green;
}

@keyframes showForms {
  0% {
    height: 0;
  }
  50% {
    height: 100px;
    opacity: 0;
  }
  100% {
    height: 100px;
    opacity: 1;
  }
}
<div class="download">
  <form>
    <div class="inputs">
      <input aria-label="Name" id="fieldName" maxlength="200" name="cm-name" placeholder="Name">
      <input autocomplete="Email" aria-label="Email" id="fieldEmail" maxlength="200" required="" type="email" placeholder="Email">
    </div>
    <div class="button-cont">
      <button class="btn" type="submit">Request Brochure</button>
    </div>
  </form>
</div>

\$\endgroup\$
1
\$\begingroup\$

Event delegation can be used to improve efficiency. Instead of adding a click handler to every single element with the class download, an event handler could be added to the entire document or a sub-element that contains all elements with class download.

document.addEventListener('click', e => {
  let node = e.target;
  do {
    if (node.classList.contains('download') && !node.classList.contains('is-open')) {
      e.preventDefault();
      const btn = node.querySelector('.btn');

      btn.classList.remove('is-style-outline');
      btn.classList.add('is-style-default');

      node.classList.add('is-open');
      break;
    } 
    node = node.parentNode;
  } while (node && node.classList !== undefined);
});

In the example above, the click handler inspects the target element to see if it or a parent node has the class download and if such an element doesn't have class is-open before modifying class names for the elements.

I noticed that I wasn't able to click on the div element with class download without clicking the button, despite the container element having space on the sides of the button, and in the original code the click event is bubbled up from the button to the div. Instead of needing to use the do while loop to go up the DOM chain, the click handler could check to see if the target element has class name btn and in that case if it has an ancestor up three levels that matches the specified class names then modify the class names.

document.addEventListener('click', e => {
  const node = e.target;
  if (node.classList.contains('btn')) {
    if (node.parentNode && node.parentNode.parentNode && node.parentNode.parentNode.parentNode) {
      const ancestorDiv = node.parentNode.parentNode.parentNode;
      if (ancestorDiv.classList && ancestorDiv.classList.contains('download') && !ancestorDiv.classList.contains('is-open')) {
        e.preventDefault();

        node.classList.remove('is-style-outline');
        node.classList.add('is-style-default');

        ancestorDiv.classList.add('is-open');
      }
    }
  }
});

Because the code above contains multiple 'if' statements that lead to multiple indentation levels, the logic could be switched to return early instead:

document.addEventListener('click', e => {
  const node = e.target;
  if (!node.classList.contains('btn')) {
    return;
  }
  if (!(node.parentNode && node.parentNode.parentNode && node.parentNode.parentNode.parentNode)) {
    return;
  }
  const ancestorDiv = node.parentNode.parentNode.parentNode;
  if (ancestorDiv.classList && ancestorDiv.classList.contains('download') && !ancestorDiv.classList.contains('is-open')) {
    e.preventDefault();

    node.classList.remove('is-style-outline');
    node.classList.add('is-style-default');

    ancestorDiv.classList.add('is-open');
  }
});

I presume the classes is-style-outline and is-style-default are styled by a library (e.g. wordpress) but the styles could be incorporated based on whether the container div (i.e. with class download) contains class is-open, in the same way the inputs are displayed depending on those class names.


Indentation in this code is fairly uniform, though one CSS ruleset (i.e. .download.is-open .inputs) contains rules indented by four spaces instead of two

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.