Stuck on get all the li elements ink. the li that created dynamically

Hi everyone!

I am a newbie. Please help me with this exercise.
I need to create a shoppinglist and it will be able to add more goods to the list by a button (which i have done).
I also have to change the color of all the li element when it’s clicked on but my code in JS just work for the li that created by HTML, not the li element that I created by JS. I can not figure out what is the reason. Can anyone give me some tips and trick?
Obs! It is not allowed to use jQuery and not either any change on the HTML page.

This is my HTML:

<h2>Handlingslista</h2>
	<div>
		<input type="text" id="myInput" placeholder="Ange vara..." />
		<button onclick="nyttElement()">Lägg till</button>
	</div>

	<ul id="myUL">
		<li>Kakor</li>
		<li>Bullar</li>
		<li style="text-decoration: line-through;">Nutella</li>
	</ul>

and this is my code for JS:

    var handlingsLista = document.querySelector('#myUL');
    var listInput = document.querySelector('#myInput');

    function nyttElement() {
        // Add element to handlingsLista
        var newElement = document.createElement('li');
        newElement.className = 'nyProdukt';
        var innehåll = document.createTextNode(listInput.value);
        newElement.appendChild(innehåll);
        handlingsLista.appendChild(newElement);
    }

    //Problem here! it changes only the li elements that created by HTML, not by JS, WHY?
    var listItems = document.getElementsByTagName('li');
    for (var i = 0; i < 1000; i++) {
        var iterms = listItems[i];
        iterms.onclick = function () {
            this.style.color = 'red';
            this.style.fontSize = '18px';
            org_html = this.innerHTML;
            new_html = "<strike id='slidesInner'>" + org_html + "</strike>";
            this.innerHTML = new_html;
        }
    }

44

As you can see, the last goods that are new added don’t change anything. I’m stuck!

Many thanks for helps!
Tho

Hi Tho, the code below your “//Problem here!” comment only runs once, shortly after the page loads. So you’re successfully registering the onclick handler for the <li> elements that exist when the page is created, but you never register the onclick handler for the <li> elements that you create dynamically using JS.

The most straightforward way (IMHO) to modify your code to work the way you want is to change the anonymous function you have in the for loop into a named function outside of the loop, which will then allow you to call that function whenever it’s needed. It would look something like this:

var listItems = document.getElementsByTagName('li');
for (var i = 0; i < 1000; i++) {
    var iterms = listItems[i];
    iterms.onclick = redderAndBigger;
}

function redderAndBigger() {
    this.style.color = 'red';
    this.style.fontSize = '18px';
    org_html = this.innerHTML;
    new_html = "<strike id='slidesInner'>" + org_html + "</strike>";
    this.innerHTML = new_html;
}

Now you can call the redderAndBigger() function each time you add a new <li> element by adding a line to your nyttElement() function, which will register the onclick event on your newly created <li>.

function nyttElement() {
    // Add element to handlingsLista
    var newElement = document.createElement("li");
    newElement.onclick = redderAndBigger;
    newElement.className = "nyProdukt";
    var innehåll = document.createTextNode(listInput.value);
    newElement.appendChild(innehåll);
    handlingsLista.appendChild(newElement);
}
1 Like

OMG! That’s it! I get it works now. Can’t say how much grateful I am. Thank you so much for helping me! You safe my entire week! :heart_eyes:

1 Like

Hi again!

I got a new problem. I’m working on it all day long and still do not know why it comes up with this erro message : “Uncaught TypeError: Cannot set property ‘onclick’ of undefined”. Do you think that my way to solve this exercise is not correct?

Thanks again for helping me!

That error implies you are attempting to set a property to something that does not exist yet.

So when I look at your example and the suggestion made to you:

for (var i = 0; i < 1000; i++) {
        var iterms = listItems[i];

Will become a problem if you do not have 1000 lis.

I was bored., so I made this primarily so everyone could learn from it.
https://jsfiddle.net/wu1t6jmr/3/

const initApp = () => {
  // if this div does not exist exit script
  if (!getElement('#my_project')){
  	 return;
  }
  
  // allow the LIs created by HTML to become clickable
  const $LIs = Array.from(getElements('li.item'));
  $LIs.forEach(($li) => $li.addEventListener('click', handleClickListItem));
  
  // makes the add item button do something
  getElement('a#add_item')
  	.addEventListener('click', handleClickAddItem);
};

// crosses & uncrosses LIs clicked
const handleClickListItem = (evt) => {
    const $itemSelected = evt.target.closest('li');
    $itemSelected.classList.toggle('crossed_off');
}

// creates new item & adds the ability to click on LI
const handleClickAddItem = (evt) => {
	// prevents anchor tag from navigating to #
	evt.preventDefault();
  
    const $newItem = getElement('input#new_item');
    const newItem = $newItem.value;
  
    $newItem.addEventListener('click', handleClickListItem);
  
    // not completely necessary,. but this function allows you to add an item to any list
    addItemToMyList(newItem, getElement('.my_list'));
    clearOut($newItem);
};

const addItemToMyList = (newItem, $list) => {
    // prevents empty items from being added to list
    if (newItem === ''){
  	  return;
    }
	const $newLI = createElement('li');
    $newLI.classList.add('item');
	$newLI.innerText = newItem;
	$list.append($newLI);
};

// not completely necessary,. but it cuts down on amount of text
const clearOut = ($element) => $element.value = '';
const createElement = (element) => document.createElement(element);
const getElement = (element) => document.querySelector(element);
const getElements = (element) => document.querySelectorAll(element);


// start app
initApp();
1 Like

The code above is certainly longer,. but it allows reusability if you had other lists on page.

Thanks Kera! I tested your code. But the problem is when I add new item it doesn’t change the color of the item. :frowning:

@ThoTr User @kerafyrm02 has given you more than enough help to get you started. See if you can use what you have been given and use part of your original code related to the color changes to achieve what you want. If you get stuck, post your code and ask questions. We are not here to just give you full solutions. We are here to guide you to solutions.

2 Likes

Thanks! I’m trying my best :smiley:

There are almost always a lot of ways to accomplish something in code and, although your solution is not ideal in some ways, it is correct if it meets all of the requirements of the exercise. :smiley_cat:

If your code needs to run without any errors, then you’ll definitely need to modify it to eliminate the ‘Uncaught TypeError’ of course. @kerafyrm02 correctly points out that this error means that you are trying to set the onclick property of an item that doesn’t exist (i.e. the item is ‘undefined’). This error occurs even with your original code and, since you are only setting the onclick property in one line of your original JS code, that line is where you’d want to start looking for the issue:

iterms.onclick = redderAndBigger;

iterms is a variable you want to contain each item in the listItems array, one at a time. As kerafyrm02 notes, you don’t have 1000 items in that array so, once you’ve gone through all of the items in listItems, iterms is going to be an empty variable. And when you try to set the onclick property on an empty variable, you’ll trigger an error saying that we can’t set that property on something that’s undefined.

To fix this issue, we’d want to run through your for loop exactly one time for each <li> element that exists when this code runs, instead of always looping 1000 times.

for (var i = 0; i < 1000; i++) {

Initializing your counter to 0 is fine, and your iterator (i++) is fine, but the conditional is problematic. Instead of looping until i equals 1000, we’d like to run until i is equal to the total number of items in the listItems array. And as it turns out, JS has a way to help us. Check out this page: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/length and see if that helps at all.

2 Likes

Just to clarify, this code wasn’t suggested to him, it was in his original post.

Lo and behold., there was an error in my example.

$newItem.addEventListener('click', handleClickListItem); is incorrect.

it should have been:
$newLI.addEventListener('click', handleClickListItem);

I was adding the eventListener to the input and not the LI.

Here is a new link:
https://jsfiddle.net/dLbu6y0g/

1 Like

OMG! Now it’s working without any problem (unbelievable :star_struck:). And I’ve learn different ways to do it. :+1: Coding is so difficult! :joy: I couldn’t resolve it without your helps! Thank you guys!

1 Like