Drum Machine Review Request

Code Feedback
1

Drum Machine App

Built as a solution to a FreeCodeCamp challange, the Build a Drum Machine challenge. Best viewed with a webkit browser (Sorry old edge users!). Sound doesn’t work right in anything besides webkit, definitely a fail.

Not entirely sure that I have followed best practice for the structure of the react components (state from children to parents and vice-versa).

What do you guys think? Check the code out on Github

Codepen link: codepen

2

Using Firefox, all the sounds are working for me except for ‘D’. A few things:

  • When I manually increase the text size the letters start to shift right and a few of them start to break out of the black button. Eventually the far right column of buttons overlaps the display.
  • Speaking of the display, the content breaks out of the display box when the text size is increased.
  • Also with increased text size the header at the top starts to shift left and up and eventually parts of it leave the window and there is no horizontal scroll bar to get it back and I can’t scroll up far enough to see it all either.

Basically, you’ve got some responsiveness issues with text size.

1 Like
3

Thanks for the feedback, I’ll look into the responsiveness issues. I’ll be honest, responsiveness wasn’t a high priority as I was building it out. Maybe I should not be so lazy! haha

Strange that only ‘D’ would not work :confused:

4

I would make responsiveness a very high priority in everything you do. It helps make your project work on any device and at any text size. Nowadays you can never know how people will be looking at your project. Imagine if you used this project in your professional portfolio and you were applying for a job and the person evaluating your projects had bad eyesight and had their text size cranked up to 250%.

One of the first things I do when evaluating a page is crank up the text size and start narrowing the browser window. If a page is robust enough to handle that then I know the developer knows a thing or two about making pages responsive and accessible.

2 Likes
5

Good point, especially re text size. I hadn’t considered that. I actually use a larger text size at work and it frustrates me with how many sites are not that usable with a larger size.

6

@bbsmooth I’ve spent some time adjusting items and making the page a bit more responsive to zoom and smaller screens. Still a slight issue with some text overflow on the display element. It appears to me as the browser interprets the text as one long string (no word boundaries in the text) it doesn’t wrap it around, so it overflows.

I noticed a small big in the way my JS handles the text anyway, I send a different text string to the display component depending on whether I click the button or use a keypress.

Anyway, appreciate some more feedback if possible, cheers mate.

7

Hey, the responsiveness is definitely improving. I think for narrow widths in which the display is underneath the buttons you are solid. I cranked the text size up to 300% and everything looks good. It’s when you shift the display to the right of the buttons you are still running into problems. At certain widths the right column of buttons still overlap the display. Also, I see you added hyphens to get the content in order to get it to line break but some of the words still break out of the display. Is there a reason you are making the display box so narrow? Have you considered just keeping the display underneath the buttons at all times? I think maybe trying to force the display to the right is causing more problems than it is solving.