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

Posted by rhupp
rhupp
11,019 Points

Definitely humbled by this one! | JavaScript and the DOM (Final Challenge)

(This is regarding Guil's parting challenge at the end of "JavaScript and the DOM".)

First off, here's a snapshot of my workspace. (The code is in "js/app.js".)

I have a lot of comments attempting to explain my process, but here's the gist:

  1. I succeeded in hiding the up button for listUl.firstElementChild and the down button for listUl.lastElementChild and briefly rejoiced.

  2. I attempted to add a new list item and discovered the 'gotcha' involved in this challenge. (The buttons 'go wonky', for lack of a better phrase.)

  3. I figured the trick was to keep the buttons still while moving the list items, and I spent many hours trying to do this (I didn't want to cheat myself out of a learning experience).

  4. I finally snuck a peek at this thread and realized I should be focusing on the buttons rather than on the list items.

  5. I decided to have each event handler remove all of the buttons, do whatever needed to be done with the individual list item (add, move up/down, or delete), and add all of the buttons back. I'm certain there is a better way, but it works, and I'm reasonably content.

I just have the one question. Under listUl.addEventListener...

  // conditional to remove items
  if (event.target.className === 'remove') {
    let li = event.target.parentNode;
    let ul = li.parentNode;
    ul.removeChild(li);
    removeListButtons(rows);
    removeListButtons(rows);
    addListButtons(rows);
  }

...you'll notice that I call removeListButtons() twice.

That was the only way to get rid of all of the buttons, given my removeListButtons() function (admittedly a bit 'ugly'):

const removeListButtons = ((li) => {
  /* this function takes all of the buttons away, but only if 
     called twice. I have no idea why. (See line 103).*/
  for (var i = 0; i < li.length; i++) {
    for (var j = 0; j < li[i].children.length; j++) {
      let button = li[i].children[j];
      if (button.tagName == 'BUTTON'){
        li[i].removeChild(button);
      }
    } 
  }
})

I'm sure there's a better way, but I didn't know it and couldn't find it in the documentation. Long story short, can anyone set me straight? Why did I have to call this function twice?

Equally important, what did I miss? What is the basic concept that would have made this last challenge a matter of minutes instead of the insane amount of time it ended up taking me?

1 Answer

Steven Parker
Steven Parker
243,134 Points
Steven Parker
Steven Parker
243,134 Points

Since your loop is accessing the elements by indexing into the collection of "children", then each time one is removed the next one will be skipped over. This happens because the remaining element index numbers shift when one is removed.

You can avoid this by using different element access strategies, but also by just reversing the loop order:

    for (var j = li[i].children.length - 1; j >= 0; j--) {

I recall when I did this course, I chose not to actually remove or re-create the buttons but just change their "visibility" attribute. That avoids any indexing issues and also preserves the button locations so the remaining buttons line up by function on the page.

And the basic concept that will make development go much more quickly is called "experience" :wink:

rhupp
rhupp
11,019 Points

Brilliant! 😊 I was hoping you'd find time to answer. And I'm not surprised it's as 'simple' as reversing the loop order; that usually seems to be the way of it when I'm stumped.

I briefly tried hiding elements, but the button jumble became such a headache that I just wanted to move on.

Are you able to summarize the alternative methods of accessing those buttons? I've been able to grasp this fairly decently so far, but I really struggled with the logic on this one. I read through a lot of official documentation and didn't find it helpful.

Steven Parker
Steven Parker
243,134 Points
Steven Parker
Steven Parker
243,134 Points

The "children" of an element are a live element collection. That means the contents will change along with anything done to the DOM. But other methods, like "querySelectorAll" return a static result, and it also lets you select just the button elements directly. Example:

const removeListButtons = items => {
  // this function takes all of the buttons away
  for (var li of items) {
    for (var button of li.querySelectorAll("button")) {
      li.removeChild(button);
    } 
  }
};

And the result of "querySelectorAll" also has a "forEach" method that you could use instead of a loop.

rhupp
rhupp
11,019 Points

That makes sense, and it's a better way. Between the indexing issue and "children" as live rather than static, I can see where I went wrong. I'll set aside time to refactor this. Thanks again!