CODEWITHSUNDEEP
javascriptevents
petek
petek

Reputation: 1088

Javascript issue with moving list items up and down

Recently started learning javascript, and am at the moment trying to build a simple app, that is an ul, with list items, where each li has two buttons, to move that li up or down.

But, I'm running into problem. Last item in list can never be moved up

    var ul = document.querySelector('ul');
    
    ul.addEventListener('click', (e) => {
    	let clicked = e.target;
    	let li		= clicked.parentNode;
    	let next    = li.nextElementSibling.nextElementSibling;    
     	let prev    = li.previousElementSibling;
        
        if (clicked.className === 'down') {
        	ul.removeChild(li);
            ul.insertBefore(li, next);
        } else if (clicked.className === 'up') {
        	ul.removeChild(li);
        	ul.insertBefore(li, prev);
        }
    });
    <ul>
        <li>item1 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item2 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item3 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item4 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item5 <button class="down">Move down</button><button class="up">Move up</button></li>
    </ul>

Here is the jsfiddle: https://jsfiddle.net/hu13x8w0/3/

Can someone explain to me, why it wont work in those 2 cases? What did I do wrong?

Upvotes: 1

Views: 417

Answers (1)

nnnnnn
nnnnnn

Reputation: 150040

If you check the browser's console (F12 to open Dev Tools in most browsers) you'll see an error like this:

Uncaught TypeError: Cannot read property 'nextElementSibling' of null

That's because on this line:

li.nextElementSibling.nextElementSibling;

...the last element's .nextSibling is null (it doesn't have a next sibling). You need to test for null rather than just chaining an extra .nextElementSibling on the end:

    var ul = document.querySelector('ul');
    
    ul.addEventListener('click', (e) => {
    	let clicked = e.target;
    	let li		= clicked.parentNode;
    	let next    = li.nextElementSibling;               // <-- Code changed
        if (next != null) next = next.nextElementSibling;  // <-- Code inserted
     	let prev    = li.previousElementSibling;
        
        if (clicked.className === 'down') {
        	ul.insertBefore(li, next);
        } else if (clicked.className === 'up') {
        	ul.insertBefore(li, prev);
        }
    });
    <ul>
        <li>item1 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item2 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item3 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item4 <button class="down">Move down</button><button class="up">Move up</button></li>
        <li>item5 <button class="down">Move down</button><button class="up">Move up</button></li>
    </ul>

Note that you don't need the calls to .removeChild(), because .insertBefore() will move the element (not copy it).

Upvotes: 2

Related Questions