0

This is a beginner question. I just cannot figure out the answer as simple as it may be.

I have two files: One with a JavaScript function and the other with the HTML code. The JavaScript should build the elements of a menu.

This is what I have:

JavaScript (mynaivesdk.js):

async function buildMenu(element){

    const webElements = [{"name": "Home", "href": "index.html"}, {"name": "Works", "href": "works.html"}, {"name": "Contact", "href": "contact.html"}]
    var menu = document.getElementById(element);
    
    webElements.forEach((webelement) => {
        const row = '<li><a href="' + webelement.href + '">' + webelement.name + '</a></li>';
        menu.innerHTML += row;
    });
}

HTML:

<html>
<head>
    <script type="module" src="mynaivesdk.js"></script>
    <title>My Menu</title>
</head>
<body>
    <h1>This is my menu</h1>    
    <ul id="mymenu" onload="buildMenu('mymenu')">        
    </ul>    
   
</body>
</html>

The result is that nothing is displayed. There is no error. I am concluding my HTML is not invoking the JavaScript function at all.

This is what I would expect to have:

<html>
<head>
    <script type="module" src="mynaivesdk.js"></script>
    <title>My Menu</title>
</head>
<body>
    <h1>This is my menu</h1>    
    <ul id="mymenu">
        <li><a href=ïndex.html">Home</a></li>
        <li><a href=works.html">Works</a></li>
       <li><a href=contact.html">Contact</a></li>      
    </ul>    
   
</body>
</html>

I know this should be extremely simple. I just cannot figure out the correct way of doing this.

Thank you!

cssyphus
  • 37,875
  • 18
  • 96
  • 111
Wilmar
  • 558
  • 1
  • 5
  • 16

3 Answers3

2

Call your function when the JS file loads and it will build your menu for you.

You'll need to add the defer attribute to your <script> tag. This simple attribute replaces the need to do the DOMContentLoaded thing.

Here you go:

function buildMenu(menu) {

  const webElements = [{
    "name": "Home",
    "href": "index.html"
  }, {
    "name": "Works",
    "href": "works.html"
  }, {
    "name": "Contact",
    "href": "contact.html"
  }]

  webElements.forEach((webelement) => {
    const row = '<li><a href="' + webelement.href + '">' + webelement.name + '</a></li>';
    menu.innerHTML += row;
  });
}

buildMenu(document.getElementById('mymenu'));
<html>

<head>
  <script defer type="module" src="mynaivesdk.js"></script>
  <title>My Menu</title>
</head>

<body>
  <h1>This is my menu</h1>
  <ul id="mymenu">
  </ul>

</body>

</html>
Randy Casburn
  • 13,840
  • 1
  • 16
  • 31
  • Hi Randy. Thanks for the answer. That works well, you are correct, however, I will have more functions to generate different HTML components and I want to separate each of them into a function so it is readable and clean. What would be the best approach then> Thanks! – Wilmar Nov 09 '21 at 19:53
  • Let me rewrite this - all you have to do is call the function in your JS. – Randy Casburn Nov 09 '21 at 19:58
  • Hi Randy - that's a solid answer, although different from what you originally proposed. I would comment, though, that using the `defer` tag, while correct, is not frequently seen - whereas `DOMContentLoaded` (or in jQuery-ese, `$(function(){ ... });` ) is extremely common. For a newbie, that should be noted. However, your answer is worth upvoting, so +1. – cssyphus Nov 09 '21 at 20:08
  • @cssyphus - While I appreciate your comment, the language improvements that are made over. time should be recognized for their value. If we continue to hold on to old, outmoded ways of accomplishing simple goals, we lose out on the benefits of the language improvements. The `defer` attribute is one of those simple improvements that should be recognized for its value and embraced - while casting off the old, out dated "because its always been done that way" ideas. But, these are just opinions - right? – Randy Casburn Nov 09 '21 at 20:15
  • 1
    Actually, your reasoning is along the lines of my comment that `const/let` are more correct than *(the older, outdated)* `var`. In other words, you're bang-on correct. Perhaps this back/forth will be helpful to the OP as our discussion has uncovered the concept of the javascript firing before necessary elements are present in the DOM - which is something the OP might not have fully realized without this conversation. – cssyphus Nov 09 '21 at 20:21
  • Thanks very much @RandyCasburn, very good explanation. Now my only issue is to chose which answer to select as "the answer" :). Thank you again! – Wilmar Nov 10 '21 at 13:10
  • @Wilmar - yes, that is always an issue - 9 different ways to solve every problem. I would choose the most direct and simplest, but then I would be choosing my own answer - LOL. Good luck and have fun with this stuff. – Randy Casburn Nov 10 '21 at 13:34
1

There are a couple of things to know:

  1. As epascarello pointed out in a comment, the UL element doesn't have a load event, so the buildMenu() function will never fire.

  2. So, you need to call the buildMenu function, based on some event (a button press, hovering over a specific element, or - in this case - the document load event.

  3. The document load event (DOMContentLoaded) is important because it is fired as soon as the elements are all present in the DOM. Before that, if you ran the buildMenu function, the #mymenu element might not be there yet, and nothing will happen.

  4. This method is easily expanded, and you can add more function calls, etc, as you build-out your app. It is also worth noting that this is the standard way of structuring the loading/calling of JavaScript functions.

Here's what it looks like:

document.addEventListener('DOMContentLoaded', () => {
   const myUl = document.getElementById('mymenu');
   buildMenu(myUl);
});

function buildMenu(myEl){
    const webElements = [{"name": "Home", "href": "index.html"}, {"name": "Works", "href": "works.html"}, {"name": "Contact", "href": "contact.html"}]
    //var menu = document.getElementById('mymenu'); //not needed
    
    webElements.forEach((webelement) => {
        const row = '<li><a href="' + webelement.href + '">' + webelement.name + '</a></li>';
        myEl.innerHTML += row;
    });
}
<html>
<head>
    <script type="module" src="mynaivesdk.js"></script>
    <title>My Menu</title>
</head>
<body>
    <h1>This is my menu</h1>    
    <ul id="mymenu" onload="buildMenu('mymenu')">        
    </ul>    
   
</body>
</html>
cssyphus
  • 37,875
  • 18
  • 96
  • 111
  • DOMContentLoaded is not necessary and adds unneeded complexity for no benefit. – Randy Casburn Nov 09 '21 at 20:04
  • Just because *this example* works without DOMContentLoaded, doesn't mean it's not worth knowing/including. Using const/let is not necessary - but it's more correct than var. If a newbie thinks that `DOMContentLoaded` is unnecessary, at some point they will be caught out when their js runs before the DOM content has all been loaded. Besides, isn't that exactly what `defer` is doing on your script tag? So they are two methods to do the same thing - why would you say that method A is unnecessary just because your answer uses method B? – cssyphus Nov 09 '21 at 20:13
  • Please see my response to your other comment for the answer to why A over B. – Randy Casburn Nov 09 '21 at 20:16
  • Thanks very much @cssyphus, I really appreciate the detailed explanation. As I said to Randy now my problem is to chose which answer to mark as "best". Both of you have good arguments. Thanks again! – Wilmar Nov 10 '21 at 13:11
  • Please choose Randy's - his answer is more current. But remember `DOMContentLoaded` also. – cssyphus Nov 10 '21 at 15:01
0
  1. Element doesn't have onload event.
  2. You don't have to add the async keyword.

Please read about JavaScript Asynchronous

  1. You can either put your javaScript code before </body> inside a <script> tag or just make a new js file.
<body>
.....

<!-- Here your change -->
<script>buildMenu("mymenu");</script>
</body>
  1. Please use template literals, it's better than string concatenation.

JavaScript Template literals

const row = `<li><a href="${element.href}">${element.name}</a></li>`;

The final code will be something like this.

In your script.js file:

function buildMenu(menuID) {
  const webElements = [
    { name: "Home", href: "index.html" },
    { name: "Works", href: "works.html" },
    { name: "Contact", href: "contact.html" }
  ];
  var menu = document.getElementById(menuID);

  webElements.forEach((element) => {
    const row = `<li><a href="${element.href}">${element.name}</a></li>`;
    menu.innerHTML += row;
  });
}

In your index.html file

<html>
<head>
    <title>My Menu</title>
</head>
<body>
    <h1>This is my menu</h1>    
    <ul id="mymenu">        
    </ul>    
   
  <script src="script.js"></script>
  <script>
    buildMenu("mymenu")
  </script>
</body>
</html>

Live Example

Nawaf Khalifah
  • 594
  • 3
  • 11
  • Hi Nawaf - welcome to StackOverflow - you're doing very well in a short time. One quick note, in case you don't already know: you can press `Ctrl + M` in the Answer editor to get a built-in CodePen/jsFiddle environment that we call "StackSnippets". That will keep your complete answer, including code, on this system *(CodePen is unlikely to ever disappear, but other sandbox environments have disappeared in the past, so we created this tool to keep code examples local.)* StackSnippets is not always the best sandbox *(for example, with React answers)* - but it's nice to know about. – cssyphus Nov 09 '21 at 20:32
  • 1
    Thank you @Nawaf for your answer. I appreciate you taking the time. I will certainly follow your advice on using template literals. – Wilmar Nov 10 '21 at 15:09
  • @Wilmar No problem ❤ – Nawaf Khalifah Nov 10 '21 at 15:20