JavaScript dynamically element duplicate each time function is called

Question

I have a function called validateForm() which checks for me form input. If the input is not as it should be (each field filled) alert message should appear and should be removed after 3 seconds and it does, but each time I call this function this element is created in DOM. How can I improve that code? What I want to get is div with alert message as below, but not created in the DOM each time the function is called, but once.

static validateForm() {
    const title = document.querySelector('#title').value;
    const author = document.querySelector('#author').value;
    const isbn = document.querySelector('#isbn').value;

    if (title === '' || author === '' || isbn === '') {
        const div = document.createElement('div');
        div.className = 'alert alert-dismissible alert-danger';
        const message = document.createTextNode('Please fill all fields before adding.');
        div.appendChild(message);

        const bookForm = document.querySelector('#book-form');
        bookForm.parentNode.insertBefore(div, bookForm);


        setTimeout(() => {
            div.classList.add('d-none');
        }, 3000);

        return false;
    }

    return true;
}

since you use applenChild, the related action is removeChild — apple apple
– apple apple, Commented Dec 25, 2018 at 3:07

McBern · Accepted Answer · 2018-12-25 04:06:35Z

1

Try this:

//construct div globally
var div = document.createElement('div');
div.className = 'alert alert-dismissible alert-danger';
div.style.display = "none";
const bookForm = document.querySelector('#book-form');
bookForm.parentNode.insertBefore(div, bookForm);


static validateForm() {
    const title = document.querySelector('#title').value;
    const author = document.querySelector('#author').value;
    const isbn = document.querySelector('#isbn').value;

    if (title === '' || author === '' || isbn === '') {
        const message = document.createTextNode('Please fill all fields before adding.');
        div.html = message;
        div.style.display = "block";

        setTimeout(() => {
            div.classList.add('d-none');
        }, 3000);

        return false;
    }

    return true;
}

answered Dec 25, 2018 at 4:06

McBern

5491 gold badge4 silver badges8 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Minh Tri Le · Accepted Answer · 2018-12-25 02:11:43Z

1

You can create div for alert in your HTML and give it class hidden to hide it.

Then when the input is not as it should be, you can change the inner HTML for that div element by your alert message and add class show for it to display it.

When the show time is over let's remove class show from that div.

.hidden {
  display: none;
} 

.show {
  display: block;
}

answered Dec 25, 2018 at 2:11

Minh Tri Le

3603 silver badges12 bronze badges

Comments

apple apple · Accepted Answer · 2018-12-25 03:14:15Z

1

the inversion of appendChild is removeChild, in case you do not want to toggle the style

//you can reuse the div
const div = document.createElement('div');
{
  div.className = 'alert alert-dismissible alert-danger';
  const message = document.createTextNode('Please fill all fields before adding.');
  div.appendChild(message);
}
function foo() {
  const bookForm = document.querySelector('#root');
  bookForm.parentNode.insertBefore(div, bookForm);

  setTimeout(()=>bookForm.parentNode.removeChild(div), 3000);
}

<div id="root"></div>

<button onclick="foo()">show node</button>

answered Dec 25, 2018 at 3:14

apple apple

10.7k2 gold badges20 silver badges41 bronze badges

3 Comments

Useless Code Over a year ago

One thing to be aware of if you go this route, storing it in a global var and removing the node from the DOM (as opposed to just hiding it; or creating the div in foo so it is local) is that if foo is clicked more than once in a 3 second window, it will create 2 timers, when the second timer fires it will raise an error because it tries to removeChild but the node was already removed by the first timer. No one would probably notice so long as they aren't viewing your site with the dev console. So, it probably isn't a big deal, but still something to be aware of.

Useless Code Over a year ago

You could use Node.contains inside of the setTimeout callback to see if the div is currently in the DOM before trying to remove it, which would prevent any errors: setTimeout(()=> { let parent = bookForm.parentNode; if (parent.contains(div)) { parent.removeChild(div); } }, 3000);

apple apple Over a year ago

@UselessCode If it is clicked more than once, it may be preferred to extend the visible time (which is a little more complex). I'd keep my answer as-is, but thanks for the great suggestion.

jobinrjohnson · Accepted Answer · 2018-12-25 02:09:01Z

0

The simplest approach is to create the div using html with the class d-none and then add the class d-block whenever you want to display the div. or use JQuery slideDown() function. Then set a timeout and remove the d-block class and add d-none class or use JQuery slideUp() function

answered Dec 25, 2018 at 2:09

jobinrjohnson

5585 silver badges13 bronze badges

Collectives™ on Stack Overflow

JavaScript dynamically element duplicate each time function is called

4 Answers 4

Comments

Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related