0

Code is also available at fiddle. This is a minimal reproducible sample from my project.

<!DOCTYPE html>
<html lang="en">
<head>
</head>
<body>
  <button class="alert">Do stuff</button>
</body>

<template id="modal_template">
    <div class="modal_background">
        <div class="modal_content">
            <h2 class="modal_header"></h2>
            <p class="modal_message"></p>
            <button class="modal_button modal_accept_button"></button>
        </div>
    </div>
</template>
</html>
async function display_modal(title, message, button_label = "Accept") {
  // resolve promise when accept is clicked
  return new Promise((resolve) => {
    
    // create a modal from template
    const temp = document.querySelector("#modal_template");
    let clone = temp.content.cloneNode(true);
    clone.querySelector(".modal_header").innerText = title;
    clone.querySelector(".modal_message").innerText = message;

    // create an accept button
    const button = clone.querySelector(".modal_accept_button");
    button.innerText = button_label;
    button.addEventListener("click", (e) => {
      // On click, delete modal and resolve the promise
      const modal_background = e.srcElement.parentNode.parentNode;
      modal_background.parentNode.removeChild(modal_background);
      resolve();
    });

    document.body.appendChild(clone);
  });
}

window.onload = () => {
  const alert = document.querySelector(".alert");
  alert.addEventListener("click", () => {
    display_modal("Alert", "Important message", "Accept").then(console.log("Accept button clicked"));
  });
}

Intended behavior

  1. User clicks on button "Do stuff"
  2. Modal shows up and gives some buttons for user to click on.
  3. Button on the modal is clicked.
  4. console.log("Accept button clicked") is being run and the modal is deleted.

Actual behavior

  1. User clicks on button "Do stuff". console.log("Accept button clicked") has been run.
  2. Modal shows up and gives some buttons for user to click on.
  3. Button on the modal is clicked.
  4. the modal is deleted.

current code:

window.onload = () => {
  const alert = document.querySelector(".alert");
  alert.addEventListener("click", () => {
    display_modal("Alert", "Important message", "Accept").then(console.log("Accept button clicked"));
  });
}

current behavior feels like:

window.onload = () => {
  const alert = document.querySelector(".alert");
  alert.addEventListener("click", () => {
    display_modal("Alert", "Important message", "Accept");
    console.log("Accept button clicked");
  });
}

Why does this happen?

Pavel Skipenes
  • 342
  • 5
  • 14
  • 3
    `.then` accepts a function. `console.log("Accept button clicked")` isn’t a function. – Sebastian Simon Nov 28 '21 at 21:18
  • @PavelSkipenes - but why the Promise Object? There is nothing asynchronous about that code. – Randy Casburn Nov 28 '21 at 21:22
  • 2
    Please review what a [The Explicit Construction Anti-Pattern](http://bluebirdjs.com/docs/anti-patterns.html#the-explicit-construction-anti-pattern) is all about. Specifically, using it as a _**glorified event emitters or callback utility**_. – Randy Casburn Nov 28 '21 at 21:27
  • @RandyCasburn The modal will display multiple buttons. In this example I just displayed one to make the example shorter. When user clicks on one of the buttons the idea is to perform some other actions. The modal is a part of a big procedural function inside a try block. Will take a look at it. Thanks the resource. – Pavel Skipenes Nov 28 '21 at 21:27
  • Sure, you are still using the antipattern. And are any of those other functions async? – Randy Casburn Nov 28 '21 at 21:28
  • @RandyCasburn yes. It has to do connections to servers. – Pavel Skipenes Nov 28 '21 at 21:30
  • I'm just trying to inform you. That particular antipattern makes your code unecessarily complicated and hard to interpret - wastes memory and processing time and in this case a no-op anyway. Simply wasted code. All the best. – Randy Casburn Nov 28 '21 at 21:33
  • Not totally related with your question however when using promises for your event listeners you best make sure that the event listener gets removed afterwards since once it resolves it will always remain resolved and you won't be able to reuse it in further events unless you replace it with a new one. You may check [this answer](https://stackoverflow.com/a/45858738/4543207) to see how to get rid of the event listener promise function automatically after the event fires. – Redu Nov 28 '21 at 21:41
  • @Redu Thanks for letting me know. Are event listeners still alive even though I'm removing them? script.js:17 – Pavel Skipenes Nov 28 '21 at 21:47
  • 1
    [If a DOM Element is removed, are its listeners also removed from memory?](https://stackoverflow.com/questions/12528049/if-a-dom-element-is-removed-are-its-listeners-also-removed-from-memory) is a place to check then :) – Redu Nov 28 '21 at 21:57

2 Answers2

2

You incorrectly pass console.log to then. Then expects a function, you pass whatever console.log returns. But to obtain possible argument to then, console.log is executed immediately.

An easiest fix should be to have

.then( () => console.log( ...
Randy Casburn
  • 13,840
  • 1
  • 16
  • 31
Wiktor Zychla
  • 47,367
  • 6
  • 74
  • 106
  • 2
    Then this can be closed as a duplicate of [What is the difference between a function call and function reference?](/q/15886272/4642212). – Sebastian Simon Nov 28 '21 at 21:26
  • @SebastianSimon: I hope it was not you to downvote a correct answer just because you've found a duplicate. Regards. – Wiktor Zychla Nov 28 '21 at 21:30
  • I didn’t. Just wanted to link that post and suggest closing instead of answering. – Sebastian Simon Nov 28 '21 at 21:32
  • @SebastianSimon: next time please just vote on closing under the question rather than leave a comment under an answer. Answers cannot be closed, questions can. – Wiktor Zychla Nov 28 '21 at 21:37
  • I didn’t feel confident enough to cast a close vote here yet because I wanted to make sure that this really is the only problem in OP’s code. _You_ felt confident enough to post an answer… – Sebastian Simon Nov 28 '21 at 21:47
  • @SebastianSimon: no problem. Remember that casting a close vote doesn't automatically close a question as it has to be supported by other users. Anyway, I've found something that seems to be even closer duplicate and casted a close vote. – Wiktor Zychla Nov 28 '21 at 22:12
  • I have a JavaScript gold badge… – Sebastian Simon Nov 28 '21 at 22:45
0

For this to work you need to await the response from display_modal.

async function display_modal(title, message, button_label = "Accept") {
  // resolve promise when accept is clicked
  return new Promise((resolve) => {

    // create a modal from template
    const temp = document.querySelector("#modal_template");
    let clone = temp.content.cloneNode(true);
    clone.querySelector(".modal_header").innerText = title;
    clone.querySelector(".modal_message").innerText = message;

    // create an accept button
    const button = clone.querySelector(".modal_accept_button");
    button.innerText = button_label;
    button.addEventListener("click", (e) => {
      // On click, delete modal and resolve the promise
      const modal_background = e.srcElement.parentNode.parentNode;
      modal_background.parentNode.removeChild(modal_background);
      resolve();
    });

    document.body.appendChild(clone);
  });
}

window.onload = () => {
  const alert = document.querySelector(".alert");
  alert.addEventListener("click", async() => {
    await display_modal("Alert", "Important message", "Accept");
    console.log("Accept button clicked");
  });
}
* {
  background-color: #333;
}

.modal_background {
  position: fixed;
  z-index: 2;
  left: 0;
  top: 0;
  width: 100%;
  height: 100%;
  overflow: auto;
  background-color: rgba(0, 0, 0, 0.4);
}

.modal_content {
  background-color: #222;
  max-height: calc(100vh - 100px);
  padding: 20px;
  margin: 20px auto;
  width: 80%;
}
<!DOCTYPE html>
<html lang="en">

<head>
</head>

<body>
  <button class="alert">Do stuff</button>
</body>

<template id="modal_template">
    <div class="modal_background">
        <div class="modal_content">
            <h2 class="modal_header"></h2>
            <p class="modal_message"></p>
            <button class="modal_button modal_accept_button"></button>
        </div>
    </div>
</template>

</html>
GenericUser
  • 3,003
  • 1
  • 11
  • 17