JavaScript JavaScript and the DOM Traversing the DOM Getting the First and Last Child

Ben Bushell
Ben Bushell
4,285 Points

Feedback on my solution

Was hoping somebody might be able to give me some feedback on how to improve my solution to the hiding buttons challenge.

Thanks!

const toggleList = document.getElementById('toggleList');
const listDiv = document.querySelector('.list');
const descriptionInput = document.querySelector('input.description');
const descriptionP = document.querySelector('p.description');
const descriptionButton = document.querySelector('button.description');
const listUl = listDiv.querySelector('ul');
const addItemInput = document.querySelector('input.addItemInput');
const addItemButton = document.querySelector('button.addItemButton');
const removeItemButton = document.querySelector('button.removeItemButton');
const lis = listUl.children;

const createButton = (parentE) => {
  let upButton = document.createElement('button');
  upButton.textContent = "Up";
  upButton.className = "up";
  parentE.appendChild(upButton);

  let downButton = document.createElement('button');
  downButton.textContent = "Down";
  downButton.className = "down";
  parentE.appendChild(downButton);

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

const checkButtons = () => {
  let firstListItemUpButton = listUl.firstElementChild.querySelector('button.up');
  firstListItemUpButton.style.visibility = 'hidden';
  let lastListItemUpButton = listUl.lastElementChild.querySelector('button.down');
  lastListItemUpButton.style.visibility = 'hidden';
};

const showButtons = () => {
  for (let i = 0; i < lis.length; i += 1) {
    lisUpButton = lis[i].querySelector('button.up');
    lisUpButton.style.visibility = 'visible';
    lisDownButton = lis[i].querySelector('button.down');
    lisDownButton.style.visibility = 'visible';
  }
};

for (let i = 0; i < lis.length; i += 1) {
  createButton(lis[i]);
};

checkButtons();

listUl.addEventListener('click', (event) => {
  if (event.target.className === "remove") {
    let li = event.target.parentNode;
    let ul = li.parentNode;
    ul.removeChild(li);
  }
    if (event.target.className === "up") {
    showButtons();
    let li = event.target.parentNode;
    let ul = li.parentNode;
    let prevLi = li.previousElementSibling;
    if (prevLi) {
      ul.insertBefore(li, prevLi);    
    }
  }
    if (event.target.className === "down") {
    showButtons();
    let li = event.target.parentNode;
    let ul = li.parentNode;
    let nextLi = li.nextElementSibling;
    if (nextLi) {
      ul.insertBefore(nextLi, li);
    }
  }
  checkButtons();
});

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

descriptionButton.addEventListener('click', () => {
  descriptionP.innerHTML = descriptionInput.value + ':';
  descriptionInput.value = '';
});

addItemButton.addEventListener('click', () => {
  let ul = document.getElementsByTagName('ul')[0];
  let li = document.createElement('li');
  li.textContent = addItemInput.value;
  ul.appendChild(li);
  createButton(li);
  addItemInput.value = '';
});

1 Answer

Zimri Leijen
Zimri Leijen
8,954 Points

That's a lot of repetition, see if you can refactor it to avoid repeating code that does the same or similar things.