Solution for the first-last feature

I know this isn't really in the spirit of the challenge, as it doesn't use Javascript, but a quicker way to do this would be to add these two CSS rules.

ul li:first-of-type .up {
    visibility:hidden;
}

ul li:last-of-type .down {
    visibility:hidden;
}

Hey All,

You want your solution to add as little code and be as readable as possible. For this reason, I most like James Wood's CSS solution, as well as Chris Wright's very compact solution.

Keeping it in the spirit of javaScript puzzles (even though I would use CSS in the real world), I offer my solution. It only adds some 8 to 10 essential lines of code if you don't include linebreaks for end brackets or extra variables to keep things readable. However, with variables and formatting it is more like 12 to 14 lines.

My way of thinking about this problem is just, "what is the simplest way I can do this?" Finding the answer to that question requires me to think in very basic non-coding terms about what is happening and what I need to change. My brainstorm went like this:

• We need to refresh each li's buttons as it shifts position.
• Refreshing means deleting and adding again.
• Whenever we add the buttons, certain buttons are NOT added depending on the li's position.
• We already have a function that adds the buttons.
• Therefore, all we need to do is test when adding buttons and refresh the li's when moving.

This can be done without much code!

If your solution was much longer than 20 lines and you want to try and find a slimmer solution on your own, stop reading now and try again with these things in mind: focus on only the li's that are shifting, add a couple (very small) helper functions, and make small adjustments to the functions and handlers you already made with Guil's instruction. You can do it in less than 20 lines of code for sure.

Here's my solution:

First, you want to update the attachLiButtons function that we made with Guil so that each li is tested and is given the appropriate buttons to begin with. I add a simple test for the up and down buttons, before they are appended.

function attachLiButtons (li) {
  let up = document.createElement('button');
  let down = document.createElement('button');
  let remove = document.createElement('button');
  up.className = "up";
  up.textContent = "Up";
  down.className = "down";
  down.textContent = "Down";
  remove.className = "remove";
  remove.textContent = "Remove";
  if (li.previousElementSibling) li.appendChild(up); //add test here
  if (li.nextElementSibling) li.appendChild(down); //and here
  li.appendChild(remove);
}

Next, I created a function to delete all buttons -- a necessary step in refreshing each li.

function removeLiButtons (li) {
  while (li.firstElementChild) li.removeChild(li.firstElementChild);
}

Then, I created a function to refresh the li buttons, which simply calls removeLiButtons and attachLiButtons back to back. Remember that the testing is now built in to attachLiButtons. This is just to keep things readable and DRY later on.

function refreshLiButtons (li) {
  removeLiButtons(li);
  attachLiButtons(li);
}

Finally, in the button click handler, I simply call refreshLiButtons on whatever li elements are being shifted. Remember that we are always shifting two at a time. (Two elements are essential swapping places, and so two need to be refreshed).

listUl.addEventListener('click', (e) => {
  if (e.target.tagName === 'BUTTON') {
    let button = e.target;
    let li = button.parentNode;
    let prevLi = li.previousElementSibling;   //new variable just for readability later
    let nextLi = li.nextElementSibling;          //ditto ^
    let ul = li.parentNode;
    if (button.className === 'remove') {
      ul.removeChild(li);
    }
    if (button.className === 'up') {
      if (prevLi) {
        ul.insertBefore(li, prevLi);
        refreshLiButtons(prevLi);    //in this case, prevLi is being moved, so we need to refresh it.
      }
    }
    if (button.className === 'down') {
      if (nextLi) {
        ul.insertBefore(nextLi, li);
        refreshLiButtons(nextLi);    //nextLi has been moved, so refresh it
      }
    }
    refreshLiButtons(li);    //no matter what, the original li has moved, so refresh it. No need to do this in each if statement. Keep it DRY.
  }
});

Doh! I found a bug. When adding a new item, it only has a remove button. This is because in Guil's function for adding a new item, the buttons are added before the item is appended, therefore the item has no previous or next siblings and fails both tests in attachLiButtons. So, the call to attachLiButtons needs to be moved to after the append, and the previous sibling then needs to be refreshed as well.

addItemButton.addEventListener('click', () => {
  let li = document.createElement('li');
  li.textContent = addItemInput.value;
  listUl.appendChild(li);
  attachLiButtons(li);     //moved this call to happen after the new item is appended to the DOM
  refreshLiButtons(li.previousElementSibling);     //the previous item is no longer last, so it needs to be refreshed.
  addItemInput.value = '';
});

That's all!

Hey guys, I wrote a single function you have to call out 3 times: after for () with attachButtons, in the end of listUl.addEventListener and in the end of addItemButton.addEventListener.

This code allows to remove the first Up and the last Down buttons when moving and deleting items and also adding new ones.

function check() {
  let upButtons = listUl.getElementsByClassName('up');
  let downButtons = listUl.getElementsByClassName('down');
  let buttonsNumber = upButtons.length;
  for (let i = 0; i < buttonsNumber; i += 1) {
    upButtons[i].style.visibility = 'visible';
    downButtons[i].style.visibility = 'visible';
  }
  upButtons[0].style.visibility ='hidden';
  downButtons[buttonsNumber-1].style.visibility = 'hidden';
}

To cut the story short I duplicated your last conditions checking whether there are buttons existing and if not they will be created. I used these creating conditions in the first part of the programme to create missing buttons IF

1. firstListItem is missing the down button and vice versa

but since when you move firstListItem down it will try to create Up button (since as firstListItem it didnt have it). But there is NO Down button so I had to change insertBefore(upButton) before Remove Button (since at this point there is no Down button to be used as a referral point. Is it clear? No..? I almost hanged myself solving this.... I am positive this can be refactored by at least two more functions but as I said I am glad to be done with it :D Now the only missing thing is when there is only one list item there should be no buttons. That should be easy to do though...

function checkForChange(){
  let firstListItem = listUl.firstElementChild;
  let lastListItem = listUl.lastElementChild;
  for (let i = 0; i < lis.length; i++) {
    if (firstListItem == lis[i]) { //ak sa zo vsetkych listitemov jeden rovna firstListItem
      let upButton = firstListItem.querySelector("button.up");
      if (upButton !== null) { //ak tam Up Button je!
        firstListItem.removeChild(upButton);
      } if (lis[i].querySelector("button.down") === null) {
        let down = document.createElement("button");
        down.className = "down";
        down.textContent = "Down";
        let removeButton = lis[i].querySelector("button.remove");
        lis[i].insertBefore(down, removeButton);
      }
    } else if (lastListItem == lis[i] ) {
      let downButton = lastListItem.querySelector("button.down");
      if (downButton !== null) { //ak tam Down Button je!
        lastListItem.removeChild(downButton);
      }if (lis[i].querySelector("button.up") === null) {
        let up = document.createElement("button");
        up.className = "up";
        up.textContent = "Up";
      let removeButton = lis[i].querySelector("button.remove");
        lis[i].insertBefore(up, removeButton);
      }
    } else {
      if (lis[i].querySelector("button.up") === null) {
        let up = document.createElement("button");
        up.className = "up";
        up.textContent = "Up";
        let downButton = lis[i].querySelector("button.down");
        lis[i].insertBefore(up, downButton);
      }

      if (lis[i].querySelector("button.down") === null) {
        let down = document.createElement("button");
        down.className = "down";
        down.textContent = "Down";
        let removeButton = lis[i].querySelector("button.remove");
        lis[i].insertBefore(down, removeButton);
      }
    }
  }
}

What do you mean by upButton !== null? I really don't understand this code? Could someone please explain?

I created my own function for looping through the list and finding each up and down button each time, then setting the opacity to 0 to block it from being viewed.

This seemed more elegant than removing the button as it made the buttons jump around the page for me.

const disableButtons = (li) => {
  for(let i = 0; i < li.length; i++){
    let upBtn = li[i].children[0];
    let downBtn = li[i].children[1];
    upBtn.style.opacity = 1;
    downBtn.style.opacity = 1;
    if(i == 0){
      upBtn.style.opacity = 0;
    }
    if(i == li.length - 1){
      downBtn.style.opacity = 0;
    }
  }
}

I then fired this function passing in the lis const created within the tutorial.

disableButtons(lis);

This was necessary to be checked on load of the page and when clicking any button so it was added into the following function at the bottom after all the code had run.

listUL.addEventListener

addItemButton.addEventListener

I thought hiding the buttons looked a little nicer than moving them around.

//Defining constants
const toggleList = document.getElementById('toggleList');
const listDiv = document.querySelector('.list');
const descriptionInput = document.querySelector('input.description');
const descrisptionP = document.querySelector('p.description');
const changeList = document.querySelector('button.description');
const addItemInput = document.querySelector('input.addItemInput');
const addItemButton = document.querySelector('button.addItemButton');
const ul = document.querySelector('.listItems');
const listItems = document.getElementsByTagName('li');
const list = ul.children;


//Defining functions
function attachButtons (li) {
  let up = document.createElement('button');
  up.className = 'up';
  up.textContent = "Up";
  li.appendChild(up);
    if (li == ul.firstElementChild) {
      up.style.opacity = '0.2';
    }

  let down = document.createElement('button');
  down.className = 'down';
  down.textContent = "Down";
  li.appendChild(down);
    if (li == ul.lastElementChild) {
      down.style.opacity = '0.2';
    }

  let remove = document.createElement('button');
  remove.className = 'delete';
  remove.textContent = "Remove";
  li.appendChild(remove);
}

function deleteButtons(li) {
  while (li.firstElementChild) {
    li.removeChild(li.firstElementChild);
  }
}

function checkButtons(li) {
  deleteButtons(li);
  attachButtons(li);
}

//Loop will attach buttons when the page loads
/********************************************/
for (let i = 0; i < list.length; i++) {
  attachButtons(list[i]);
}

//Event handlers
/***************/
listDiv.addEventListener('click', function (event) {
  if (event.target.className === 'up' || 'down' || 'delete'){
    let li = event.target.parentNode;
    let prevLi = li.previousElementSibling;
    let nextLi = li.nextElementSibling;

    if (event.target.className === 'delete'){
      let ulist = li.parentNode;
      if (prevLi) {
        ul.removeChild(li);
      }else if (prevLi = 'null') {
        nextLi.firstElementChild.style.opacity = '0.2';
        ul.removeChild(li);
      }
      if (nextLi) {
        ul.removeChild(li);
      }else if (nextLi = 'null') {
          prevLi.firstElementChild.nextElementSibling.style.opacity = '0.2';
          ul.removeChild(li);
        }
    }
    if (event.target.className === 'up') {
      if(prevLi){
      ul.insertBefore(li, prevLi);
      checkButtons(prevLi);
      checkButtons(li);
      }
    }
    if (event.target.className === 'down') {
      if (nextLi){
      ul.insertBefore(nextLi, li);
      checkButtons(nextLi);
      checkButtons(li);
      }
    }
  }
});

toggleList.addEventListener('click', function() {
  if (listDiv.style.display == "none") {
     listDiv.style.display = "block";
     toggleList.textContent = "Hide List";
  } else {
      listDiv.style.display = 'none';
      toggleList.textContent = "Show List";
  }
});

changeList.addEventListener('click', function() {
  descrisptionP.innerHTML = descriptionInput.value + ' :';
  descriptionInput.value = "";
});

addItemButton.addEventListener('click', function() {
  let li = document.createElement('li');
  li.textContent = addItemInput.value;
  ul.appendChild(li);
  attachButtons(li);
  checkButtons(li.previousElementSibling);
  addItemInput.value = "";
});

Hi, I managed to achieve the challenge with just two lines of code. hopefully, it is the right solution I added the code to the attachListItemButtons function

function attachListItemButtons(li) {

    let up = document.createElement('button');
    if (li.previousElementSibling) {  // Added code to check for previous li
        up.className = "up";
        up.textContent = "up";
        li.appendChild(up);
    }

    let down = document.createElement('button');
    if (li.nextElementSibling) {    // Added code to check for next li
        down.className = "down";
        down.textContent = "down";
        li.appendChild(down);
    }

    let remove = document.createElement('button');
    remove.className = "remove";
    remove.textContent = "remove";
    li.appendChild(remove);

}

so i wraped the code we wrote to create the button in if statment to check if it has previous sibiling or next sibling. Let me know what do you think guys

This seems to work pretty smoothly, wondering what folks think. (Some of my variable names are different from the class. I like to try and work ahead and see if I can figure things out before Guil explains how he did it.)

The first thing I did was add a 'while' condition to the top of attachListItemButtons() that will remove any buttons from the LI elements to prevent duplicating any buttons as it runs through the function. Then I set some variables to store which items are the first and last children of the UL element. Then at the end of the function, I set two IF statements that clear off the unnecessary buttons from the first and last LI elements.

function attachListItemButtons(li) {
  while(li.firstElementChild) {
    li.removeChild(li.firstElementChild);
  }
  let firstLi = ulElement.firstElementChild;
  let lastLi = ulElement.lastElementChild;
  let upButton = document.createElement('button');
  upButton.textContent = 'Move Up';
  upButton.className = 'up';
  let downButton = document.createElement('button');
  downButton.textContent = 'Move Down';
  downButton.className = 'down';
  let removeButton = document.createElement('button');
  removeButton.textContent = 'Remove Item';
  removeButton.className = 'remove';
  li.appendChild(upButton);
  li.appendChild(downButton);
  li.appendChild(removeButton);
  if(li === firstLi) {
    li.removeChild(li.firstElementChild);
  }
  if(li === lastLi) {
    li.removeChild(li.lastElementChild.previousElementSibling);
  }
}

After that it was just a matter of making sure to feed only LI elements that were affected by button presses back through the attachListItemButtons function. To do that I called the function on the li and sibling variables from within the ".up" and ".down" handlers, and that was pretty concise. But that left the remove button buggy if you removed the last item or the first item. So I feel like it started getting a little sloppy.

ulElement.addEventListener('click', (event) => {
 if(event.target.tagName == 'BUTTON'){
  if(event.target.className == 'remove') {
    let li = event.target.parentNode;
    let ul = li.parentNode;
    let prevLi = li.previousElementSibling;
    let nextLi = li.nextElementSibling;
    ul.removeChild(li);
    if(prevLi) {
      attachListItemButtons(prevLi);
    }
    if(nextLi) {
      attachListItemButtons(nextLi);
    } 
  }
  if(event.target.className == 'up') {
    let li = event.target.parentNode;
    let ul = li.parentNode;
    let sibling = li.previousElementSibling;
    if(sibling) {
      ul.insertBefore(li, sibling);
      attachListItemButtons(li);
      attachListItemButtons(sibling);
    }
  }
  if(event.target.className == 'down') {
    let li = event.target.parentNode;
    let ul = li.parentNode;
    let sibling =li.nextElementSibling;
    if(sibling) {
      ul.insertBefore(sibling, li);
      attachListItemButtons(li);
      attachListItemButtons(sibling);
    }
  }
 }
});

Then passing the current lastChild and the new list item through whenever adding a new item to the list.

addItemButton.addEventListener('click', () => {

 if(addItemText.value == ""){
  return;
  } else {
  let prevLastLi = ulElement.lastElementChild;         //stores the current last item on the list                  
  let li = document.createElement('li');
  li.textContent = addItemText.value;
  listOfStuff.appendChild(li);
  attachListItemButtons(li);                                
  attachListItemButtons(prevLastLi);        //runs previous last item through to add missing buttons
  addItemText.value = '';
  }
});

I love the CSS solution earlier up on this post. Seems like the 'real' way to do this so far. But in the spirit of using javascript and firstElementChild and lastElementChild, this seems to be sort of straight forward and not very processing intensive.

I feel like stripping all the buttons off, then reapplying them, and then removing excess buttons if its the first or last item seems like a wrong way to go about it. I just don't have the skills yet to know another way. Then, even if it is the way it should be done, I feel like my code calling attachListItemButtons so many times in the code isn't the DRY way.