Build a Drum Machine - React Question/ Feedback

Build a Drum Machine - React Question/ Feedback
0

#1

My code currently passes all tests, but I had a question specifically around how I handle the display.

I have two React components - App (Parent) and Sound (Sound should probably be renamed to something more along the lines of DrumPad for the sake of better readability). I have my event listeners set up within the Sound component to capture keypress, as well as my handleClick function to trigger a drumpad and capture which drumpad was triggered. These functions capture the data I feed back to display which DrumPad is being triggered.

The state representing which DrumPad was last triggered (and consequently displayed, per user story #7) is held in the App (parent) component. I have a function that is declared in the parent called “GetDisplayKey” that essentially allows me to pass a value up from a child Sound component as a property, and then use that to set the state in the parent DrumPad.

The state update is handled entirely within the parent App component (via GetDisplayKey). I am simply using data fed up from the child Sound component to inform what the modification should be.

Am I breaking React best practices by passing data up from a child component that is used to modify state in the parent component? If so, what are the disadvantages of doing this? How would you recommend re-architecting my code to adhere to best practices?

My codepen is here (NOTE: still working through style and UI, so please disregard that): https://codepen.io/bloo0178/pen/QVbyrP


#2

I will let someone else comment on the best practices part, but since bank is an array of objects and you are using Bootstrap, you could change your render from:

  render() {
    return (
      <div id="drum-machine" className="container">
        <div className="row mx-auto">
          <Sound value="Q" sendDisplayKey={this.getDisplayKey}/> 
          <Sound value="W" sendDisplayKey={this.getDisplayKey}/>
          <Sound value="E" sendDisplayKey={this.getDisplayKey}/>
        </div>
        <div className="row mx-auto">
          <Sound value="A" sendDisplayKey={this.getDisplayKey}/>
          <Sound value="S" sendDisplayKey={this.getDisplayKey}/>
          <Sound value="D" sendDisplayKey={this.getDisplayKey}/>
        </div>
        <div className="row mx-auto">
          <Sound value="Z" sendDisplayKey={this.getDisplayKey}/>
          <Sound value='X' sendDisplayKey={this.getDisplayKey}/>
          <Sound value="C" sendDisplayKey={this.getDisplayKey}/>
        </div>
        <div className="row mx-auto">
          <div id="display" className="display">
            <p>{this.state.displayKey}</p>
          </div>         
        </div>
      </div>
    );
  }

to something like below:

  render() {
    const pads = bank.map( ({keyTrigger}) => <Sound key={keyTrigger} className="col-4" value={keyTrigger} sendDisplayKey={this.getDisplayKey}/>);
     return (                           
      <div id="drum-machine" className="container">
        {pads}
        <div className="row mx-auto">
          <div id="display" className="display">
            <p>{this.state.displayKey}</p>
          </div>
        </div>
      </div>
    );
  }

#3

Thanks for the feedback, Randell! Much appreciated.