CODEWITHSUNDEEP
javascriptreactjs
Altair312
Altair312

Reputation: 348

React removeEventListener not being removed

I have already looked through a lot of Googling, but still can't manage to fix this issue.

I've got a list of buttons that I want to be able to be selected by keypresses. When pressing "A", you get "A: Xxxxxxxx" select selected. When pressed B, you get "B: yyyyyyyy" select selected.

  const d = useDispatch();
  const s = useSelector(select);
  const [selected, setSelected] = useState(s);

  const eventHandler = function (card, i, setSelected, d, set) {
    return e => {
      if (alphabet.indexOf(e.key) === i) {
        setSelected(card.key);
        d(set(card.key));
      }
    };
  };

  useEffect(() => {
    cards.map((card, i) => {
      window.addEventListener(
        "keypress",
        eventHandler(card, i, setSelected, d, set),
        false
      );
    });
    return () => {
      window.removeEventListener("keypress", eventHandler, false);
    };
  }, []);

Now, i found out, that you need to specify function references for them in order to work, but I can't get my head around how would I pass these variables from cards.map into the function eventHandler that handles the button presses? I'm really lost here...

Upvotes: 0

Views: 163

Answers (2)

Altair312
Altair312

Reputation: 348

In the end I came up with something similar. Accepting @epascarello's answer as accepted because he gave me the basic idea.

  export const alphabet = ["a", "b", "c", "d", "e", "f", "g", "h"];

  const d = useDispatch();
  const s = useSelector(select);
  const [selected, setSelected] = useState(s);

  const keyPressHandler = (event) => {
    const index = alphabet.indexOf(event.key);
    if (index >= 0) {
      setSelected(cards[index].key);
      d(set(cards[index].key));
    }
  };

  useEffect(() => {
    window.addEventListener("keypress", keyPressHandler, false);
    return () => {
      window.removeEventListener("keypress", keyPressHandler, false);
    };
  });

Upvotes: 0

epascarello
epascarello

Reputation: 207501

You are adding a bunch of event listeners to the window. You need to store the events into something and reference them to remove them. 

const myEvents = []
const eventHandler = function (card, i, setSelected, d, set) {
  const fnc = e => {
    if (alphabet.indexOf(e.key) === i) {
      setSelected(card.key);
      d(set(card.key));
    }
  };
  myEvents = fnc
  return fnc
};

useEffect(() => {
  cards.map((card, i) => {
    window.addEventListener(
      "keypress",
      eventHandler(card, i, setSelected, d, set),
      false
    );
  });
  return () => {
    myEvents.forEach(evt => 
      window.removeEventListener("keypress", evt, false))
  };
}, []);

In the end this code is really inefficient. You should be binding one event handler and looking up to see if the key matches any of the cards.

Basic idea would be

const lookUpCard = e => {
  const index = alphabet.indexOf(e.key)
  const myCard = cards[index]
})

window.addEventListener("keypress", lookUpCard)

Upvotes: 2

Related Questions