Drum Machine FeedBack and Help turning it off (Solved)

Hello there.

Here’s my Drum-Machine-Project, and here’s the source code, it gives an error on the test, but you close it and can see that it passes all the tasks.

Any advice would be appreciated :smiley:


The only problem that i see and can’t resolve is that, when you turn off the drum machine it doesn’t sound by clicking the drum pads, but by pressing the keys it does.

I think that the problem resides in this function, but don’t how to fix it:


The useEffect() function in the DrumPad component have a weird behavior, when the drum machine the first time is off, when you press a key it logs the key normaly, but when it’s turn it on logs the key multiple times in the same event. On the other hand if you console log the power prop (false when the machine is off and true when is on), then press a key, it logs true and false at the same time when it only should return false.

Quick fix would be to use correct syntax for adding/removing listenters:

  useEffect(() => {
    const handleKeyPress = (e) => {
      const { key } = e;
      if (key.toUpperCase() === letter) {

    document.addEventListener("keydown", handleKeyPress);
    return () => document.removeEventListener("keydown", handleKeyPress);
  }, [letter, playAudio]);

Correct fix would be to move keydown listener into DrumMachine.js so you won’t be creating separate listener for each key.

Also, what’s with the useCallback everywhere? What problem are you trying to solve with them?

1 Like

I really like the original look of it. I do think there are a few issues though.

  • The drum pads need to show the letter at all times, not just on hover. If someone is using the keyboard only how will they know what keys to use?
  • Speaking of those drum pads, those should be <button>s instead of <div>s so that they are keyboard accessible. I know that you press the key to play the sound, but depending on the assistive tech used some people may not be able to do that because those keys may be mapped to other functionality and thus they will need to be able to Tab through each drum pad and use the enter/space bar to trigger the sound.
  • Any application that makes noise should have its own volume control.
  • You’ve got some responsiveness issues with large zoom. In Chrome, when I page zoom to 300% I can no longer see the top row of drum pads. In Firefox, when I text zoom only The power/bank toggles start to look funky and the text overflows the display.
  • Speaking of those toggles, they really should have a <label> but you could get away with giving them an aria-label attribute instead.
1 Like

Thanks for your help. You really saved me hahaha. I alredy did the quick and correct fix (the correct way was really a challenge, but nailed it). You can see the code and tell me how it look if you want.

The useCallback think, I use it only once now, in the playAudio() method, inside DrumMachine component, because if I don’t use it, the debugger gives this message:

The ‘playAudio’ function makes the dependencies of useEffect Hook (at line 94) change on every render. To fix this, wrap the definition of ‘playAudio’ in its own useCallback() Hook.eslintreact-hooks/exhaustive-deps

If I use the useCallback the app works well, but I prefer to use it just in case.

Really appreciate the advice. I have to take care of the accesibility more often haahah.

Here’s my todo list.

  • Quit the thing that only shows the letters on hover. Done :white_check_mark: . ( I leave it only for mobile because I think that you dont need the letter in that case).

  • DrumPad <button/>. Done :white_check_mark: .

  • Volume control. Done :white_check_mark: .

  • Zoom in Chome issue. Done :white_check_mark: . (But I dont know how to fix the Text zoom only thing, if you make only the text bigger, it’ill always overflow :confused: )

  • aria-label . Done :white_check_mark: .

I would suggest you always show the keyboard letters no matter what device you think the person is using. Mobile does not always mean “using a phone without a keyboard.”

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.