Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript JavaScript and the DOM (Retiring) Traversing the DOM Getting the First and Last Child

My solution to the challenge... Perhaps someone could review?

https://w.trhou.se/0h55bl5bxv

I managed to make everything work as It's supposed to, showing the necessary buttons in every possible scenario (1 element, 2 elements, 3+), adjusting the affected buttons when adding or removing, etc, and I think I've successfully done that, but I'm not sure if my code is very "elegant".

You'll notice that I've also added a few little checks here and there, obviously not every possible scenario, but still...

Anyway, any suggestions are very welcome on how I could improve this or any application going forward, such as perhaps refactoring something further if possible, code commenting style (I'm not sure if my comments are the best for ease of understanding), etc. It seems I've spent quite a few hours on this considering it's such a small app... Was I adding unnecessary complexity in any way?

Thanks everyone!

2 Answers

I think your comments are very easy to understand! I also think the way you checked for previousElementSiblings was very clever. If you wanted the code to be more 'elegant' I would recommend writing a function to test if an li element is the first and/or last element and adjust the buttons accordingly. Then you can call the function whenever the list changes. You can see my function here:

const toggleLiButtons = (ul) => {
  for(let i=0; i<ul.children.length; i++){
    let li = ul.children[i];
    let up = li.querySelector('button.up');
    let down = li.querySelector('button.down');
    if(li.isSameNode(listUL.firstElementChild)) {
      up.style.display = 'none';
    } else {
      up.style.display = 'block'; 
    }
    if(li.isSameNode(listUL.lastElementChild)) {
      down.style.display = 'none';
    }  else {
     down.style.display ='block';
    }
  }
}

I just declare it right after the attachListItemButtons function and then I just call it like

toggleLiButtons(listUl)

when I create the initial list and inside the event handlers for adding an item, removing an item, or moving an item up or down. That way you don't have to write similar checks and button removals for each different possibility throughout the code.

Jacek Skrzypek
Jacek Skrzypek
5,040 Points

Emmie Cole I really like your solution, though when I try to implement it, first item's buttons are differently alligned than the rest - they 'stick' to the text content of the first li (left). Any ideas why?