Filter option is not working in my project

Hello everyone, I’m curently creating a players profile card and I’d like to filter these players when their position is selected (coach, goalkeeper, midfielder, forward) using Javascript… I also want all the players profile cards to be displayed when the ‘team’ option is selected.

I have made a lot of attempt to get the filter option working but it isn’t.

Y’all help will be greatly appreciated.

Here is the link to the codepen… Sorry the images are not displaying as I only have them locally.

https://codepen.io/Que0/pen/eYzaYOo

Well just to get you started, it was failing due to inconsistent use of upper/lowercase in the position names e.g.

‘Defender’ does not equal ‘defender’.

If you look through your players hashmap, some of the positions start with uppercase e.g. ‘Midfield’ and some don’t e.g. ‘forward’

You obviously want to fix your hashmap so that it is consistent, but a slight amend to filter does work

const card = players.filter(function (option) {
        if (option.position.toLowerCase() === position) {
            return option;
        }
    })

Still more to do though…

In your html options you have ‘midfielder’ and in your hashmap ‘Midfield’. That needs to be fixed one way or the other. ‘Midfield’ sounds the right one to me :slight_smile:

Your displayTeamCard function has two parameters that are never used

function displayTeamCard (managerCards, playerCards)

so instead the code just references the hashmaps ‘managers’ and ‘players’ from outside of the function, rather than the ones being passed in.

a change to this fixes it

function displayTeamCard (managers = [], players = [])

then we can change the last bit of code from

if (position === 'team') {
    displayTeamCard(managers, players);
} else {
    displayTeamCard(card)
}

to

if (position === 'team') {
    displayTeamCard(managers, players);
} else {
    // empty array for managers this time?
    displayTeamCard([], card)
}

A start, but still need to fix if the option is ‘coach’ though

1 Like

@rpg_digital, Thanks for taking your time to point out my errors.

I tried fixing if the option is ‘coach’ like this and I wasn’t okay with the output

let coachCard = managers.filter( function (options) {
        if (options.position.toLowerCase() === 'coach') {
            //console.log (option);
            return options;
        };
    })

when i passed in the coachCard as an argument here,

if (position === 'team') {
        displayTeamCard(managers, players);
    } else {
        displayTeamCard(coachCard, card)
    }

If any position option is selected, ‘coach’ is displayed too and also some properties is added to the coach card when displayed that return undefined.

Your help will be appreciated, thanks once again

@adefesoq

Here you go.

// Note: 'change' works better than 'click' for options
selectOption.addEventListener('change', function (e) {
  const position = e.currentTarget.value

  // No filtering needed with 'team' so display and return
  if (position === 'team') {
    displayTeamCard(managers, players)
    return // finished
  }

  // One match to check, filter, display and return
  if (position === 'coach') {
    const cards = managers.filter(function (squadMember) {
      if (squadMember.position.toLowerCase() === position) {
        return squadMember
      }
    })
    // No players, so passing cards(managers) and an empty array(players)
    displayTeamCard(cards, [])
    return // finished
  }

  // We have got here, so must be players
  const cards = players.filter(function (squadMember) {
    if (squadMember.position.toLowerCase() === position) {
      return squadMember
    };
  })
  // No managers, so passing an empty array and cards
  displayTeamCard([], cards)
})

You still need to change your html for midfield to something like this e.g. midfield not midfielder

<option value="midfield" class="filterbtn">Midfielder</option>

@rpg_digital, thank you again, that was helpful… But I have some dumb questions to ask you.

  • Curiosity made me try to check for ‘players’ just like you did for ‘coach’ above, the code didn’t work… Why is it that way?

  • Secondly, in JavaScript, I learned that variable declared using const cannot be changed… But I noticed in the code that const cards was declared twice inside the conditions for ‘coach’ and and also for player… Can you pls explain this to me? Thank you!!

What if (position === 'players') or if (position === 'forward') ?

‘players’ obviously wouldn’t work as it’s not a position

You would want to use a regex or something else

const playersRx = /forward|midfield|defender|goalkeeper/

if (playersRx.test(position.toLowerCase())) // true if a match

Don’t forget we are also looking through ‘managers’ in that function managers.filter(function... and not ‘players’

This is a good point. I don’t think what I have done there is necessarily good practice.

The reason it works is that ‘let’ and ‘const’ are block scoped so the following will work

if (something) {
  const x = 5 // only exists inside of this if block
}

const x = 10 // a separate 'x' here

I think that will lead to confusion though, which isn’t good.

I have had a go at refactoring your code.

A general rule of thumb I would say is that you want to keep functions relatively short and simple. It makes them a lot easier to read, debug and reuse.

Templates

One way to do that is to move your templates out of your display function like so — These could even be imported in as their own views module.

const coachTemplate = function (squadMember) {
  return `<div class="card">
    <div class="profile-img">
      <img src=${squadMember.profileImg} alt=${squadMember.name} />
    </div>
    <div class="info">
      <p class="card-info"><span>Name:</span> ${squadMember.name} </p>
      <p class="card-info"><span>age:</span> ${squadMember.age} </p>
      <p class="card-info"><span>position:</span> ${squadMember.position}</p>
      <div class="country">
        <p><span>Country:</span> ${squadMember.country}</p>
        <img class="flag" src=${squadMember.flag} alt=${squadMember.flag} />
      </div>
    </div>
  </div>`
}

const playerTemplate = function (squadMember) {
  return `<div class="card">
    <div class="profile-img">
      <img src=${squadMember.profileImg} alt=${squadMember.name} />
    </div>
    <div class="info">
      <p class="card-info"><span>Name:</span> ${squadMember.name} </p>
      <p class="card-info"><span>age:</span> ${squadMember.age}</p>
      <p class="card-info"><span>height:</span> ${squadMember.height}</p>
      <p class="card-info"><span>position:</span> ${squadMember.position}</p>
      <p class="card-info"><span>shirt no:</span> ${squadMember.shirtNo}</p>
      <div class="country">
        <p><span>Country:</span> ${squadMember.country}</p>
        <img class="flag" src=${squadMember.flag} alt=${squadMember.flag} />
      </div>
    </div>
  </div>`
}

renderTeamCards

The first thing I have done is simplify the parameters down from ‘managers’ and ‘players’ to just ‘squad’.

I have also made use of array.reduce instead of ‘map’. It is a great tool for assembling strings and worth looking into.

It saves us using map twice, joining and concatenating.

I have also made use of the ternary operator instead of ifs and elses

// Display the team cards
function renderTeamCards (squad = []) {
  // We can use array's reduce to build our entire HTML
  const squadCards = squad.reduce(
    function (squadHTML, squadMember) {

      return (squadMember.position === 'coach')
        ? `${squadHTML}${coachTemplate(squadMember)}`
        : `${squadHTML}${playerTemplate(squadMember)}`
    },
    '' /* squadHTML starts as an empty string */
  )

  sectionCenter.innerHTML = squadCards
}

This reduces our displayTeamCards down from 45 lines to a more manageable 14 lines

optionsHandler

For the selectOption eventListener I have created a separate handler function instead of the anonymous function.

so instead of

selectOption.addEventListener('change',function(....

we have

function optionsHandler (event) {
   ...code here
}

selectOption.addEventListener('change', optionsHandler)

Oh and lastly I have wrapped the code inside of your ‘DomContentLoaded’ callback. This way we keep your code out of the global namespace. Preventing it from overwriting other code/apps etc or from being overwritten itself.

Final code

// ---- managers and players objects ---
// ... objects here

// ---- Squad Templates ----
// ... templates from above here


// Wrap our code and keep it out of the Global namespace
window.addEventListener('DOMContentLoaded', function () {

  // selectors
  const sectionCenter = document.querySelector('.section-center')
  const selectOption = document.querySelector('select')

  // making this a separate function we can reuse it as we want
  // instead of copying the same code over and over.
  // It also keeps optionsHandler cleaner and easier to read.
  function filterPositions (position) {
    return function (squadMember) {
      if (squadMember.position.toLowerCase() === position) {
        return squadMember
      }
    }
  }

  // options event handler
  function optionsHandler (event) {
    const position = event.currentTarget.value

    // if 'team' then display and return
    if (position === 'team') {
      // using ...spread to concatenate the arrays
      renderTeamCards([...managers, ...players])
      return
    }

    const cards = (position === 'coach')
      ? managers.filter(filterPositions(position))
      : players.filter(filterPositions(position))

    renderTeamCards(cards)
  }

  // Display the team cards
  function renderTeamCards (squad = []) {
  // We can use array's reduce to build our entire HTML
    const squadCards = squad.reduce(
      function (squadHTML, squadMember) {

        return (squadMember.position === 'coach')
          ? `${squadHTML}${coachTemplate(squadMember)}`
          : `${squadHTML}${playerTemplate(squadMember)}`
      },
      '' /* squadHTML starts as an empty string */
    )

    sectionCenter.innerHTML = squadCards
  }

  // Assign our handler to our change event
  selectOption.addEventListener('change', optionsHandler)
  renderTeamCards([...managers, ...players])
})

There are still a few things to possibly breakdown :slight_smile:

edit: This is turning into a lengthy post you never asked for, so please feel free to run for the hills.

Reduce example

I thought a further example of reduce might be useful

const fruit = ['apples', 'bananas', 'cantaloupe', 'donuts', 'elderberry']

fruit.reduce(function (accumulator, currentValue) {
  return accumulator + '<div>' + currentValue + '</div>'
}, '' /* <- This is accumulator to start with */)

/* for each iteration of the loop
return '' + '<div>apples</div>'
return '<div>apples</div>' + '<div>bananas</div>'
return '<div>apples</div><div>bananas</div>' + '<div>cantaloupe</div>'
return '<div>apples</div><div>bananas</div><div>cantaloupe</div>' + '<div>donuts</div>'
return '<div>apples</div><div>bananas</div><div>cantaloupe</div><div>donuts</div>' + '<div>elderberry</div>'
*/

// Refactor above with a template string
fruit.reduce(function (accumulator, currentValue) {
  return `${accumulator}<div>${currentValue.toUpperCase()}</div>\n`
}, '')
1 Like

@rpg_digital, thank you so much… I’m finding it hard grasping the concept and real life usage of the ternary operator and reduce method but this long note of yours😊 was a helper and a booster…

-can you take a look at these code for me

//SEARCH BAR
const searchInput = document.getElementById('search');
console.log(searchInput);

//event listener
searchInput.addEventListener('keyup', searchInputClickHandler);

//function
//i filter, check and return
function searchInputClickHandler (e) {
    let searchString = e.target.value.toLowerCase();
    console.log(searchString)
    const filteredCharacters = players.map (function (character) {
        //console.log(character)
        return (character.name.toLowerCase().includes(searchString))

    })
   console.log(filteredCharacters)
  displayTeamCard([], filteredCharacters)
 }

Thank you

1 Like

Another long one, sorry. It’s compulsive and I do get carried away.

Some fixes…

As has been pointed out to me the above is flawed, and should return a boolean. Mixing my map and my filter there.

function filterPositions (position) {
  return function (squadMember) {
    return squadMember.position.toLowerCase() === position 
  }
}

optionsHandler fix

With the optionsHandler rather than working with ‘if managers’ and ‘if players’ it would make more sense to work with the squad as a whole. After all we can filter coaches and players with the same ‘position’ key value.

In your script you make use of concat to join the managers and players strings together. We can do the same with arrays e.g.

const squad = managers.concat(players)

or with ES6 we can use the spread operator to do the same

const squad = [...managers, ...players]

So the following would be an improvement

// options event handler
function optionsHandler (event) {
  const position = event.currentTarget.value
  // concat managers and players into one array
  const squad = [...managers, ...players]

  // if position is team make cards the the whole squad
  // otherwise filter through the squad for the given position
  const cards = (position === 'team')
    ? squad
    : squad.filter(
      (squadMember) => squadMember.position.toLowerCase() === position
    )

  renderTeamCards(cards)
}

As we are only using a simple filter function once in the above code, we can drop the filterPositions code entirely.

Partial application and bind

An optional extra and getting a bit more advanced here, but to save the handler having to concatenate managers and players every time we make a change, we could bind a reference to squad to the options handler using function’s bind method

Like this

const squad = [...managers, ...players]
selectOption.addEventListener(
  'change', optionsHandler.bind(selectOption, squad)
)

We need to then make a slight change to the handler like so

function optionsHandler (squad /* bound argument */, event) {
  const position = event.currentTarget.value

  // if position is team make cards the the whole squad
  // otherwise filter through the squad for the given position
  const cards = (position === 'team')
    ? squad
    : squad.filter(
      (squadMember) => squadMember.position.toLowerCase() === position
    )

  renderTeamCards(cards)
}

Amend to final script

window.addEventListener('DOMContentLoaded', function () {
  // selectors
  const sectionCenter = document.querySelector('.section-center')
  const selectOption = document.querySelector('select')

  // options event handler
  function optionsHandler (squad, event) {
    const position = event.currentTarget.value

    const cards = position === 'team'
      ? squad
      : squad.filter(
          (squadMember) => squadMember.position.toLowerCase() === position
        )

    renderTeamCards(cards)
  }

  // Display the team cards
  function renderTeamCards (squad) {

    const squadCards = squad.reduce(
      (squadHTML, squadMember) =>
        `${squadHTML}${
          squadMember.position === 'coach'
            ? coachTemplate(squadMember)
            : playerTemplate(squadMember)
        }`,
      ''
    )
    sectionCenter.innerHTML = squadCards
  }

  const squad = [...managers, ...players]

  selectOption.addEventListener(
    'change',
    optionsHandler.bind(selectOption, squad)
  )
  renderTeamCards(squad)
})

codepen here

1 Like

It was a long post and forgive me for that. but I am glad to hear you are getting something out of it :slight_smile:

Will keep this one short. I’m sure the search algorithm could be improved, but as a simple search your code seems to be pretty much there.

I have added a debounce, just to give you time to type before processing the search.

// Wrap our code and keep it out of the Global namespace
window.addEventListener('DOMContentLoaded', function () {
  // selectors
  const sectionCenter = document.querySelector('.section-center')
  const selectOption = document.querySelector('select')
  const searchBar = document.querySelector('#search')

  // allows us to type a series of letters before processing the search
  function debounce (func, wait = 250) {
    let timeout = null

    return (...args) => {
      const later = () => {
        timeout = null
        func(...args)
      }
      clearTimeout(timeout)
      timeout = setTimeout(later, wait)
    }
  }

  // options event handler
  function optionsHandler (squad, event) {
    const position = event.currentTarget.value

    const cards = position === 'team'
      ? squad
      : squad.filter(
        (squadMember) => squadMember.position.toLowerCase() === position
      )

    renderTeamCards(cards)
  }

  // search event handler
  function searchHandler (squad, event) {
    const name = event.target.value.toLowerCase()

    const cards = squad.filter(
      (squadMember) => squadMember.name.toLowerCase().includes(name)
    )

    renderTeamCards(cards)
  }

  // Display the team cards
  function renderTeamCards (squad) {

    const squadCards = squad.reduce(
      (squadHTML, squadMember) =>
        `${squadHTML}${
          squadMember.position === 'coach'
            ? coachTemplate(squadMember)
            : playerTemplate(squadMember)
        }`,
      ''
    )
    sectionCenter.innerHTML = squadCards
  }

  const squad = [...managers, ...players]

  selectOption.addEventListener(
    'change',
    optionsHandler.bind(selectOption, squad)
  )

  searchBar.addEventListener(
    'keyup',
    debounce(searchHandler.bind(searchBar, squad))
  )
  renderTeamCards(squad)
})

codepen here

@rpg, I appreciate your efforts and I’m taking notes of yours replies and trying to understand each lines of code… Can you explain why squad is set to an empty array here even though it isn’t?

const coachTemplate = function (squadMember) {
//Code goes here
}

const playerTemplate = function (squadMember) {
//Code goes here
}
  • I noticed that the coachTemplate and the playerTemplate have the same parameter squadMember… Won’t there be a clash issue?

That is a default value for the function parameter. If the function is called with no parameters, the squad value will default to an empty array. Your code always calls the function with a parameter so that default will not end up being used.

The default value does serve two main benefits though.

  1. When the function is called with no parameters, the function won’t break because it starts off with an empty array.
  2. More importantly, it informs you the programmer that squad is supposed to be an array, without forcing you to scan elsewhere throughout the code to find out what it is supposed to be.

Those are fine because the internals of each function is a different context. Parameter values from one function have no impact on another separate function.

2 Likes

It was a default parameter.

So if we pass nothing to renderTeamCards(), squad will be an empty array and the script won’t break.

A bit like

function renderTeamCards (squad) {
    var squad = squad || []
    ... etc
}

In the amended script I removed it.

No, remember I have combined managers and players into a complete squad. We have a conditional there that says if the position is ‘coach’ then send what we have to the coach template or else send it to the playerTemplate.

That conditional makes sure we send the correct object to the correct template.

The codepens show it working :slight_smile:

2 Likes

@Paul_Wilkins @Paul_Wilkins, it’s all crystal clear now, thanks… Can you recommend me a website, docs or videos I can use to read up on the above?

1 Like
2 Likes

When it comes to default parameters and other ES6 techniques, I’ve found that the ES6-Features site provides reliable access to new and improves features. Here’s what they have to show about default parameter values.

In regard to scoping there are many sources of info. One that looks to present in an easy to understand manner is at https://dmitripavlutin.com/javascript-scope/

2 Likes