software-tools-books / js4ds

JavaScript for Data Science
https://third-bit.com/js4ds/
Other
187 stars 32 forks source link

Sortable Lists: potential typo #212

Closed asbates closed 4 years ago

asbates commented 4 years ago

In the 'Sortable Lists' section of the 'Manipulating Pages' chapter, there may be a typo.

In the first script chunk, should lists be allLists, or vice versa?

const sortLists = () => {
  const allLists = Array.from(document.querySelectorAll('#sorted'))  // 'allLists' is used here
  lists.forEach((list) => {  // but 'lists' is used here
    // ...
  })
}

The second chunk for the script doesn't use allLists:

const sortLists = () => {
  const lists = Array.from(document.querySelectorAll('.sorted'))  //  this is 'allLists' in the first chunk
  lists.forEach((list) => { // this is the same as the first chunk
  // ...
  })
}

Since there is discussion of what does/does not work between the first and second chunks (i.e. .sorted vs. #sorted), I'm not sure if this is intentional. As in, is allLists -> lists meant to be another bug of the first pass at a solution? If so, it might be worth adding to the text.

tobyhodges commented 4 years ago

Thanks for reporting this, @asbates. I agree that this error appears to be unintentional, i.e. the variable names allLists and lists should be consistent. This needs to be adjusted in pages.tex and src/pages/sort-lists.js. I'll make the fix today or tomorrow, unless you'd like to submit a PR? We'd acknowledge your contribution in the introduction of the book.

asbates commented 4 years ago

I can submit a PR for this.