Search code examples
javascriptreactjsuse-effectuse-state

useEffect re-rendering twice on every other click


I am trying to have a simple 4 button component that toggles the class of each so that only 1 is active at one time. I am using the following code on my buttons.

<NavLink
          to="/"
          className={On ? "nav-link-active" : "nav-link"}
          onClick={() => {
            setOn(true);
          }}
        >

And I am using state on all 4 buttons like so

const [On, setOn] = useState(false);
  const [On2, setOn2] = useState(false);
  const [On3, setOn3] = useState(false);
  const [On4, setOn4] = useState(false);

And finally I am using a simple if statement to ensure that only 1 of the buttons can stay active at one time.

useEffect(() => {
    console.log("useeffect");
    if (On === true) {
      setOn2(false);
      setOn3(false);
      setOn4(false);
    }
    if (On2 === true) {
      setOn(false);
      setOn3(false);
      setOn4(false);
    }
    if (On3 === true) {
      setOn2(false);
      setOn(false);
      setOn4(false);
    }
    if (On4 === true) {
      setOn(false);
      setOn3(false);
      setOn2(false);
    }
  }, [On, On2, On3, On4]);
// I know there's probably a better way to do this, it was just a quick fix.

This works for the most part, except the problem is that the useEffect() will run twice on every other click. Which then resets the buttons.

I have not worked with UseEffect() too much, so I am unsure why this is happening. How do I stop it from re-rendering twice? My full code is viewable here.


Solution

  • Use only a single state variable instead of multiple so you don't need any useEffect. You could have an index indicating the currently "on" button, 0 to 3, or -1 for none (following the usual JS numbering conventions):

    const [buttonOn, setButtonOn] = useState(-1);
    

    Then, for the JSX in the question, use instead:

    <NavLink
              to="/"
              className={buttonOn === 0 ? "nav-link-active" : "nav-link"}
              onClick={() => {
                setButtonOn(0);
              }}
            >
    

    And the next Nav would have buttonOn === 1 ? and setButtonOn(1), etc.

    Depending on the other navs and their contents, you might be able to make it less repetitive by using the index in a mapper function instead of writing out every nav.