Creating efficient function instead of repetitive functions

Question

I have a function which onclick displays the form.

Was wondering if there is any efficient way to code instead of creating 4 different functions for 4 different forms? Below example is for 4 forms but I am working with multiple forms.

<div class="navbar">
  <div class="dropdown">
  <button class="dropbtn" onclick="myFunction1()">Category 1
    <i class="fa fa-caret-down"></i>
  </button>
  </div>

//Same for other 3 categories

<div id="form1" style = "display:none">
<form action="#" method="post" id="demoForm1" class="demoForm1" >
    <fieldset>
        <legend>Use CTRL to select multiple options</legend>

        <p>
            <select name="demoSel[]" id="demoSel" size="4" multiple>
                <option value="ABC">ABC</option>
            </select>
            <input type="submit" value="Submit" />
            <textarea name="display" id="display" placeholder="view select list value(s) onchange" cols="20" rows="4" readonly></textarea>
        </p>

    </fieldset>
</form>
</div>


//Same for other 3 forms


  <script>

function myFunction1() {
document.getElementById("form1").style.display = '';
}

function myFunction2() {
  document.getElementById("form2").style.display = '';
}

function myFunction3() {
  document.getElementById("form3").style.display = '';
}

function myFunction4() {
  document.getElementById("form4").style.display = '';
}
</script>

How are these 4 functions called? and also what is your HTML markup related to these functions and form elements? — palaѕн
– palaѕн, Commented Apr 30, 2020 at 15:20
Hi @palaѕн, these funs are called as below: <button class="dropbtn" onclick="myFunction1()">Category 1 — user7078484
– user7078484, Commented Apr 30, 2020 at 15:25
also, what is your HTML markup related to these buttons and form elements? Please update your main post with the code. — palaѕн
– palaѕн, Commented Apr 30, 2020 at 15:28

palaѕн · Accepted Answer · 2020-04-30 15:37:59Z

It's generally not a good idea to use inline event handlers.

Next, add a data-* attribute to each button and remove the onclick attribute like:

<button class="dropbtn" data-target="form1">...</button>
<button class="dropbtn" data-target="form2">...</button>
<button class="dropbtn" data-target="form3">...</button>
<button class="dropbtn" data-target="form4">...</button>

Then, you can use .addEventListener() on these buttons with class dropbtn and update respective form element display property like:

const btns = document.querySelectorAll(".dropbtn");
btns.forEach(function(btn) {
  btn.addEventListener("click", function(cbox) {
     document.getElementById(this.dataset.target).style.display = '';
  });
});

Demo:

const btns = document.querySelectorAll(".dropbtn");
btns.forEach(function(btn) {
  btn.addEventListener("click", function(cbox) {
    document.getElementById(this.dataset.target).style.display = '';
  });
});

<button class="dropbtn" data-target="form1">Form 1</button>
<button class="dropbtn" data-target="form2">Form 2</button>
<br><br>
<form id="form1" style="display:none">Form 1 Content Here</form>
<form id="form2" style="display:none">Form 2 Content Here</form>

Community · Accepted Answer · 2020-06-20 09:12:55Z

Don't use on-event attributes:

<button onclick='eventHandler()'></button>

Use event listeners or on-event properties:

const btn = document.querySelector('button');

btn.addEventListener('click', eventHandler);

// OR

btn.onclick = eventHandler;

If you have multiple targets to click -- register the click event to a parent tag that all target tags share.

document.querySelector('main').onclick = toggleForm;

Instead of using .style on each <form> toggle classes

// CSS
.off { display: none }

// JavaScript
forms[idx].classList.toggle('off');

Demo

Note: Details are commented in demo

/*
- Reference the parent tag (<main>)
- Register <main> to the click event
- Event handler function toggleForm() is called on click
*/
document.querySelector('main').onclick = toggleForm;

// Event handler always passes Event Object (event)
function toggleForm(event) {
  // Collect all <form>s into a HTML Collection
  const forms = document.forms;
  // Collect all <button> into a NodeList
  const buttons = document.querySelectorAll('button');
  // Reference the tag the user clicked (<button>)
  const clicked = event.target;

  // if a <button> was clicked...
  if (clicked.matches('button')) {
    // ...toggle the <button>'s .on and .off classes
    clicked.classList.toggle('off');
    clicked.classList.toggle('on');
    /*
    - Convert buttons NodeList into a rel Array
    - Iterate through the buttons array and return
      the index of the clicked <button>
    */
    let idx = [...buttons].flatMap((button, index) => clicked === button ? [index] : []);
    /*
    - Toggle the .off class on the <form> located at the 
      index that was obtained from the previous statement
    */
    forms[idx].classList.toggle('off');
  }
}

button {
  display: inline-block;
  width: 11ch
}

button.off::before {
  content: 'Show '
}

button.on::before {
  content: 'Hide '
}

form.off {
  display: none
}

<main>
  <button class='off' type='button'>A</button>
  <button class='off' type='button'>B</button>
  <button class='off' type='button'>C</button>
  <button class='off' type='button'>D</button>
  <hr>
  <form id='A' class='off'>
    <fieldset>
      <legend>Form A</legend>
    </fieldset>
  </form>
  <form id='B' class='off'>
    <fieldset>
      <legend>Form B</legend>
    </fieldset>
  </form>
  <form id='C' class='off'>
    <fieldset>
      <legend>Form C</legend>
    </fieldset>
  </form>
  <form id='D' class='off'>
    <fieldset>
      <legend>Form D</legend>
    </fieldset>
  </form>
</main>

Collectives™ on Stack Overflow

Creating efficient function instead of repetitive functions

2 Answers 2

Comments

Demo

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Demo

Comments

Your Answer

Sign up or log in

Post as a guest

Related