Drum Machine Feedback

[yunsuklee]

Hey!
I’ve just finished my drum machine!! I’m really excited about it as it took me quite a bunch of effort
I would appreciate if anyone has got the time to review it and five some feedback upon where I could improve and if it is convenient to use redux within this project.

Thanks in advance!!

Here’s the link to my project:
https://codepen.io/yunsuklee/pen/VwxLRpw

[RodrigoSilva5]

Hi yunsuklee, how is going ?
I check your my drum machine, and i think is really good congratulations !!!

1 - your design is good, maybe change the background white from some texture.
2 - you can use arrow functions, when declaring functions inside a class component, this way the function will be bind automatically
EX:

class App extends React.Component {
  constructor(props) {
    super(props);
    this.myFunction = this.myFunction.bind(this);
  }}
myFunction(){
//do something
}
render(){
return (<div onClick={ () = this.myFunction()}></div>
}

// becomes 

class App extends React.Component {
myFunction = () => {
//do something
}
render(){
return (<div onClick={ () = this.myFunction()}></div>
}

3 - when declaring state, you dont need to declare constructor, instead you can use state = { myState: “”} `
EX:

class App extends React.Component {
  constructor(props) {
    super(props);
this.state = {
myState: "string"
}
  }}

render(){
return (<div>{this.state.myState}</div>
}

// becomes 

class App extends React.Component {
  state = {
myState: "string"
}

render(){
return (<div>{this.state.myState}</div>
}

this are just small things, and your work is amazing !!!

[yunsuklee]

Oh wow, any reason in particular regarding why arrow functions dont need binding? That looks pretty neat!
And with the constructor any general rule I can follow to know wether its necessary or not?

Thanks a lot! I really apreciate it

[bbsmooth]

Hey, nice job, this seems to work well. I’m going to give you a few accessibility issues to work on since that is sort of my thing.

• You are using divs for almost all of your inputs (except the volume control) and then adding click handlers to those divs, which is not accessible. You should always use the most semantically correct elements. I would suggest button for the drum pads. There are a variety of ways to do the power toggle. Check out this article and especially the reference links at the bottom. For the bank switch, you could use a toggle method discussed in that article but I think it might make more sense to use two radio buttons in a fieldset/legend.
• And as far as the volume control is concerned, it needs to have an accessible name. Usually you would add this by using a label for the input.
• I cannot read the black text on dark gray background in the display box. The contrast between the text and the background needs to be much greater. You can use WebAIM’s contrast checker to make sure the display is accessible.
• I would not make the display text automatically disappear like that. Doing so is an accessibility failure of WCAG 2.2.1. The best way to make this accessible would be to provide a method for the user to turn off this behavior. But I’m not sure why you shouldn’t just remove it to begin with.

[RodrigoSilva5]

Basically this do not exist in the context of arrow function, a normal function expression have its own this, arguments, super, or new.target.
you can read this

about the constructor, i really don`t know i think that NOT required at all.
because you can initialize state, and can use arrow functions.

I try to search but can find a consensus.
Sorry

[yunsuklee]

Thanks sir! This is great, hadn’t had these stuff in mind, I’ll dive right into it!

Regarding the display text that disappears I did a quick read upon the link you shared. You would recommend that either I remove the automatic clearing or I allow users to adjust the clearing timing so that it may improve user experience right?
I was trying to mimic some hardware functioning, such as electronic pianos where the display will notify the most recent change and eventually clear it.