skip to Main Content

i need to simplify these lines of code of react, i see is a bit repetitive, but i dont know if theres any way to improve this

Any idea will be received!!!

useEffect(() => {
    document.addEventListener("scroll", () => {
      // Prices
      if(document.querySelector("#prices").getBoundingClientRect().top == 78){
        document.querySelector(".pricesArr").style.opacity = 1
        document.querySelector(".pricesArr").classList.add("pricesArrAnim")
      }
      else if(document.querySelector("#prices").getBoundingClientRect().top > 660){
        document.querySelector(".pricesArr").style.opacity = 0
        document.querySelector(".pricesArr").classList.remove("pricesArrAnim")
      }
      // Contact
      if(document.querySelector("#contact").getBoundingClientRect().top == 78){
        document.querySelector(".contact").style.opacity = 1
        document.querySelector(".contact").classList.add("contactAnim")
      }
      else if(document.querySelector("#contact").getBoundingClientRect().top > 660){
        document.querySelector(".contact").style.opacity = 0
        document.querySelector(".contact").classList.remove("contactAnim")
      }
      // Moreinfo
      if(document.querySelector("#moreinfo").getBoundingClientRect().top == 78){
        document.querySelector(".moreinfo").style.opacity = 1
        document.querySelector(".moreinfo").classList.add("moreinfoAnim")
      }
      else if(document.querySelector("#moreinfo").getBoundingClientRect().top > 660){
        document.querySelector(".moreinfo").style.opacity = 0
        document.querySelector(".moreinfo").classList.remove("moreinfoAnim")
      }
    })
  })

2

Answers


  1. Unless you plan on adding a whole bunch more then no need to prematurely optimise, nothing wrong with a few if else statements. But if you are going to be adding a bunch more cases, you can do something like this (not tested):

    useEffect(() => {
      document.addEventListener("scroll", () => {
        const select = document.querySelector;
        [
          { idEl: select("#prices"), classEl: select(".pricesArr"), newClass: "pricesArrAnim" },
          { idEl: select("#contact"), classEl: select(".contact"), newClass: "contactAnim" },
          { idEl: select("#moreinfo"), classEl: select(".moreinfo"), newClass: "moreinfoAnim" }
        ].forEach(obj => {
          if(obj.idEl.getBoundingClientRect().top == 78){
            obj.classEl.style.opacity = 1
            obj.classEl.classList.add(obj.newClass)
          }
          else if(obj.idEl.getBoundingClientRect().top > 660){
            obj.classEl.style.opacity = 0
            obj.classEl.classList.remove(obj.newClass)
          }
        });
    
      })
    })
    

    So in this case, if you wanted to add another case you just add another object to the array with the idEl, classEl and newClass you need and it’ll do that too. One line to add another case.

    Login or Signup to reply.
  2. Something like this would do the trick, just adjust your classNames a bit to match id/class selectors.

    const determineVisibility = useCallback(selector => {    
        const boundaries = {
            visible: 78,
            transparent: 660
        }
    
        const elemToCheck = document.getElementById(selector);
        const elemToModify = document.querySelector("." + selector);
        const {top} = elemToCheck.getBoundingClientRect();
        
        if(top == boundaries.visible){
            elemToModify.style.opacity = 1
            elemToModify.classList.add(selector + "Anim")
        } else if(top > boundaries.transparent){
            elemToModify.style.opacity = 0
            elemToModify.classList.remove(selector + "Anim")
        }
    }, [])    
    
    useEffect(() => {
        document.addEventListener("scroll", () => {    
            ["prices", "contact", "moreinfo"].forEach(determineVisibility)    
        })
    }, [determineVisibility])
    

    Also, I would suggest you no to run this function from the anonynmous event handler, it makes it problematic to cleanup the listener in the future.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search