Job listings with filtering using React

Community feedback

• 

  Luka•820

  @LukaKobaidzePosted 6 months ago

  Hello👋 Your solution on filtering jobs is nice, great job! There are a few things I would do differently, so I'm going to answer your last question and give you an example of how I would handle filtering by changing things up in your code.

  In your FilterableCards.js component I would remove all 5 useState hooks below...

  function FilterableCards({ cards }) {
    const [role, setRole] = useState(null);
    const [level, setLevel] = useState(null);
    const [languages, setLanguages] = useState([]);
    const [tools, setTools] = useState([]);
    const [filterCounter, setFilterCounter] = useState(0);
  ...
  

  And replace it with only one state, like this:

  function FilterableCards({ cards }) {
    const [filter, setFilter] = useState([]);
  ...
  

  This filter state is an array of strings that will contain any filtering option, including role, level, languages, and tools. This state also removes the need for having filterCounter state and two functions incrementCounterHandler(), decrementCounterHandler(), because now we can get the "count" by using filter.length.

  Now In the same component, I would remove filterJobs() function and filteredJobs variable and add the following:

  function FilterableCards({ cards }) {
    const [filter, setFilter] = useState([]);
    const [filteredJobs, setFilteredJobs] = useState(cards);
  
    useEffect(() => {
      if (filter.length > 0) {
        setFilteredJobs(
          cards.filter((card) => {
            const cardTags = [card.role, card.level, ...card.languages, ...card.tools];
  
            return filter.every((filterOption) => {
              return cardTags.includes(filterOption);
            });
          })
        );
      } else {
        setFilteredJobs(cards);
      }
    }, [filter, cards]);
  ...
  

  As you can see, I added one more state filteredJobs and a useEffect hook with dependency array that includes filter. Now, whenever we update filter, useEffect runs the code that's inside of it and updates the filteredJobs state.

  Next up, I would replace addTagHandler() and removeTagHandler() functions with the following:

  ...
  function addTagHandler(tag) {
    setFilter((state) => {
      return [...state, tag];
    });
  }
  
  function removeTagHandler(tag) {
    setFilter((state) => {
      return state.filter((filteringTag) => filteringTag !== tag);
    });
  }
  ...
  

  It works just like the functions in your solution, but now I only update filter state, because we save every type of tag in it.

  Now, instead of passing role, level, languages, tools props to the FilterPanel.js component, I only pass filter state as a prop.

  And to the Card.js component instead of passing role, level, languages, tools props, I only pass one prop to it tags={[card.role, card.level, ...card.languages, ...card.tools]}.

  And finally, I would update the way we render filter tags and map over the filter and tags props.

  I forked your repo and updated it just like I explained in this answer, so you can see every file I changed. You can click my latest commit to compare your solution with mine, link here!

  If something's not clear in my solution for filtering, feel free to point it out. Happy Coding! 🤗

  Marked as helpful

  0

  ---

  

  Miran Legin•680

  @miranleginPosted 6 months ago

  @LukaKobaidze - first of all thank you for taking the time and checking my solution.

  This is the best feedback i could have asked.

  You explained everything clearly but I just couldn't figure in out how to connect all pieces together to make it work. I repeated myself couple of times and used Tag component 4 times for role, level, languages and tools instead of making single one and use array of tags. Also including another useState and replacing the functions for increment/decrements seems logical.

  You understood what i wanted to achieve but you are more proficient in Javascript than me.

  I have a question though, do you think that my initial approach was wrong and should have thought about it in different way or i just needed to sit down and refactor my code because once i got the first filter up and running i just repeated the process for the rest of them. One of the problems was that filtering had only 4 options so redundancy was not a big deal for me. But adding additional 2 or 3 filters/tags would make my life so much difficult.

  Also could you have done it without useEffect hook using only useState in FIlterableCards.js component?

  Thank you a ton!

  Best Regards, Miran

  0

  

  Luka•820

  @LukaKobaidzePosted 6 months ago

  @miranlegin No problem😀 Thank you as well for asking great questions, I learn some new things too by answering them.

  To your first question, whether your initial approach was wrong or not, I'd say It wasn't really wrong because It worked perfectly when I previewed the live site. One thing I would say was wrong is having a filterCounter state, because you're not guaranteed that it'll be in sync with your filter tags. Yes, it works now, but in the future, you might want to add more features to it, then it'll be difficult to keep it in sync every time you want to update your filter tags. filterCounter state is only updated whenever other states (role, level, languages, tools) update, and it depends on those already existing filter values, so it's unnecessary to have another state filterCounter, it causes more rerenders. Because it depends on those states, we can just create a variable below states and calculate the length using all four states like this:

  ...
  const [role, setRole] = useState(null);
  const [level, setLevel] = useState(null);
  const [languages, setLanguages] = useState([]);
  const [tools, setTools] = useState([]);
  
  const filterCounter = (role ? 1 : 0) + (level ? 1 : 0) + languages.length + tools.length;
  ...
  

  Now, filterCounter updates whenever other states update, it saves extra rerender, and we don't need to have increment and decrement functions anymore.

  About your last question, whether useEffect hook was necessary or not, I'm glad you asked that question because we didn't really need to use useState and useEffect hooks in this case. We can remove useState for filteredJobs and useEffect, and instead use the useMemo hook. By this, we would avoid another rerender, because just like before, filteredJobs depends on filter state, it is only updated whenever filter state updates. Here's an example of how we would use the useMemo hook for filteredJobs:

  ...
    const filteredJobs = useMemo(() => {
      if (filter.length > 0) {
        return cards.filter((card) => {
          const cardTags = [card.role, card.level, ...card.languages, ...card.tools];
  
          return filter.every((filterOption) => {
            return cardTags.includes(filterOption);
          });
        });
      } else {
        return cards;
      }
    }, [filter, cards]);
  ...
  

  As you can see, we pass two arguments to the useMemo hook, the first argument is a function, and the second is a dependency array. Whatever we return from that function will be the value of the filteredJobs, And by passing the dependency array as the second argument, we make sure that function only runs whenever those values in that array update. We also could've used just a function and then called that function in a normal variable, just like you had in your initial solution, and that would still be correct, but useMemo is safer, because even if we added another completely different state to that component, useMemo would make sure to only run the function whenever only filter or cards changes, because those are the values we passed into dependency array. Also, keep in mind that useMemo isn't always the best to use. For example, you might have a state that updates asynchronously by fetching the data from an API, in that case, useState and useEffect is better. If you want to know more about this, check out this question on StackOverflow. Also, using just a normal function and calling it into the variable, like you had in your initial solution, is perfectly fine here. You will only need useMemo once you add more states to the component, which you probably won't do in this challenge, just a hook to keep in mind for the future, in case you need it.

  Marked as helpful

  0

  

  Miran Legin•680

  @miranleginPosted 6 months ago

  @LukaKobaidze

  Thank you Luka once again for your time and effort.

  This is very detailed and easy to understand answer. Haven't had a chance to use useMemo or useEffect hooks before so i'm a little rusty on those concepts.

  You definitely helped me to see other ways to approach and refactor my code and i will incorporate those concepts in my next challenges here on this platform.

  Thanks once again, I really appreciate that.

  Cheers, Miran

  0