4
\$\begingroup\$

It's a user alert script. It worked in every browser I tested. But the function itself looks very messy/repetitive. Is there any way to make this cleaner?

function message(text) {
    var alert = document.createElement('div');
    var alertText = document.createTextNode(text);
    var alertCloseButton = document.createElement('i');
    var buttonText = document.createTextNode('close');
    var shadow = document.createElement('div');

    alert.className = 'alert-text';
    alertCloseButton.className = 'material-icons';
    alertCloseButton.id = 'close-alert';
    alertCloseButton.setAttribute('onclick', 'function() { var a = this.parentNode; var s = a.parentNode; s.parentNode.removeChild(s)}');
    alertCloseButton.onclick = function() { var a = this.parentNode; var s = a.parentNode; s.parentNode.removeChild(s) };
    alertCloseButton.appendChild(buttonText);
    alert.appendChild(alertCloseButton);
    alert.appendChild(alertText);
    shadow.className = 'shadow';
    shadow.appendChild(alert);

    document.body.appendChild(shadow);
}
var button = document.querySelector('button');
button.addEventListener('click', function(){
    message('Button clicked');
});

Here's the code working: https://jsfiddle.net/3aouyfz7/

\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

Explanation:

It's true that using objects can become messy after a while. That's why Template Literals and innerHTML are so powerful in this situation.

I would highly advise to avoid using parentNode when attempting to remove HTML, you may unintentionally remove something in the future if you decide to make your alert HTML more complex.

To solve this, create a container element that is accessible by your onclose method.

Also, I would also advise to not overwrite existing objects. Why not name alert to something else?

Don't add onclick events directly in the HTML, it's messy.

Solution:

function message(text) {

  const container = document.createElement("div");

  const onClose = ()=>container.remove();

  container.innerHTML = `
   <div class="shadow">
      <div class="alert-text">
        <i id="close-alert" class="material-icons">close</i>
        ${text}
      </div>
   </div>
  `;

  document.body.appendChild(container);

  document
    .querySelector('i#close-alert')
    .addEventListener("click", onClose);
}

Working Example:

function message(text) {

  const container = document.createElement("div");

  const onClose = ()=>container.remove();

  container.innerHTML = `
   <div class="shadow">
      <div class="alert-text">
        <i id="close-alert" class="material-icons">close</i>
        ${text}
      </div>
   </div>
  `;
  
  document.body.appendChild(container);
  
  document
    .querySelector('i#close-alert')
    .addEventListener("click", onClose);
}

const button = document.querySelector('button');
button.addEventListener('click', function() {
  message('Button clicked');
});
* {
  box-sizing: border-box;
}

.shadow {
  width: 100%;
  height: 100%;
  background-color: rgba(0, 0, 0, .5);
  position: fixed;
  top: 0;
  left: 0;
  display: flex;
  align-items: center;
  justify-content: center;
  z-index: 98;
}

.alert-text {
  border: 1px solid #aaa;
  padding: 2.5em;
  min-height: 10%;
  width: auto;
  max-width: 80%;
  background: #fff;
  z-index: 99;
  position: relative;
  text-align: center;
}

.alert-text i {
  position: absolute;
  top: 0;
  right: 0;
  color: #777;
  cursor: pointer;
  padding: .5rem;
}

.alert-text i:hover {
  color: #444
}
<!DOCTYPE html>
<html>

<head>
  <title></title>

  <link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/icon?family=Material+Icons">

</head>

<body>

  <h3>Page content</h3>
  <button>alert</button>


</body>

</html>

\$\endgroup\$
1
  • 1
    \$\begingroup\$ thanks for the clean up, i've learned a lot from your answer. \$\endgroup\$
    – Anderson Montelo
    Commented Mar 6, 2019 at 19:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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