static-job-listings-master

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted over 3 years ago

  Hey, great work on this one. For the UI, the layout is good and I am sure by now, you are a fan of spiniru (that is what I call on small spinning things in web) and the mobile state is good as well.

  But the real problem is the filter does not properly runs. Though when clicking the clear all, it works just fine.

  Upon looking your javascript, well it is fine until this section which you highlighted.

  store.forEach(each=>{       
              const checkValue = tagArrays.includes(each);
              if(checkValue){
                  if(store.length <= 1){      // <<<<<<<<<<< this logic is definitely deficient
                      jobListingWrapper.classList.add('marked')
                  };
              }else if(jobListingWrapper.classList.contains('marked')){
                  jobListingWrapper.classList.remove('marked')
              }
          })
          if(jobListingWrapper.classList.contains('marked')){
              jobListingWrapper.style.display = "flex";
          }
  

  As you can see, the reason it won't filter properly because you loop for every item of the store right, but you are declaring const checkValue = tagArrays.includes(each); every time it runs. So meaning whatevery is the last value of the store is the one that will determine if the item will be filtered or not.

  Let's see an example.

  Imagine this is the filter bar [ Frontend, css, javascript, react, sass]

  If i removed the sass the code will run right, so it will loop for every item in the filter bar, which are Frontend, css, javascript, react. But as I said, only the last one will determine if an item will be filtered, because you are repeating the declaration of const checkValue = tagArrays.includes(each);. So everytime it runs, the result will change. So first run, the checkValue checks if tagArrays includes FrontEnd if it does then the if statement will run, the next loop it checks again if the tagArrays includes css. As you can see, only the last will determine the outcome.

  To fix this, remove that whole section, and instead add this.

  if (store.every(item => tagArrays.includes(item))) {
              jobListingWrapper.classList.add("marked");
              jobListingWrapper.style.display = "flex";
          } else {
              jobListingWrapper.classList.remove("marked");
          }
  

  This way, it will only run one time, and it will check if every item in the store is included in the tagArrays. If all 4 example, Frontend, css, javascript, react is present, then it runs, if it does not. Then it won't display the item.

  So in whole, this section will be present

  jobTagsGroup.forEach(index=>{   
          const jobListingWrapper = index.closest('.job-listing-wrapper')
          jobListingWrapper.style.display = "none";
  
          //convert tags list into readable string arrays
          const tagArrays = index.textContent.replace(/\s+/g, ' ').trim().split(' ') 
  
          if (store.every(item => tagArrays.includes(item))) {
              jobListingWrapper.classList.add("marked");
              jobListingWrapper.style.display = "flex";
          } else {
              jobListingWrapper.classList.remove("marked");
          }
      })
  

  But still, good work on this one okay and if need help, just drop it here okay^^

  2

  ---

  

  Jay•695

  @JunjiequanPosted over 3 years ago

  @pikamart

  I can't thank you enough for your efforts explaning details to me with examples.

  You are truly my savior.

  The exaplanation is really clear and thorough, it solved all of my problems regarding the filter logic.😆

  I can see now, const checkValue = tagArrays.includes(each) was the key point that causes all of the problems. I was thinking it loops along with the parent loop and returns boolen value for each one of the arrays .

  I've had spent over 10 hours trying to figure out how to make the filter and ended up with that logic deficient filter which is not even working properly .😂

  btw, regarding the spinner wheel embeded, which is actually a color theme button, but I guess it wasn't a optimal way to present a color theme changer like that.

  anyways, Thank you sooooooo much.😆

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted over 3 years ago

  @a331998513 Hey, glad that it worked just fine and you're welcome on that one, that's what we do here in FEM^^

  0