FCC Drum Machine React Project Feedback

Hey All!

I have just completed my Drum Machine project and would appreciate any feedback you’d like to give me :smiley:

This has been made completely in React, I do intend to go back and improve it after i have completed all 5 projects but for now im rather happy with how this has turned out.

Please find the link below -

https://rturner1989.github.io/FCC-Drum-Machine-React/

Please find the source code below -

1 Like

Hello,

It doesn’t work for me. The test shows 5/8 pass.

Is this not the case for you?

Hi Chris,

Thanks for looking over it for me! This is due to the power switch being off, when you toggle the power on all tests pass.

Can you give it a go for me and let me know this works for you :slight_smile:

Thanks!

When I tired on my phone last night the power toggle wouldn’t work at all!

I’ve just tried from desktop and it works fine. I’ve just tried from my phone again and it now works too.

Must have been my phone playing up!

Looks good!

Ha that’s typical isn’t it, but glad it’s working well for you!

Il keep an eye on that as it may be a bug I haven’t seen :slightly_smiling_face:

Thanks!

My phone display was also completely ignoring media queries at the same time.

That also appears to have rectified itself. I wonder what caused that!?

Hey, this looks real nice and overall works very well. I have a few accessibility issues to make you aware of.

  • While I can work both the power and bank toggles with the keyboard there is no focus indicator for them (there is however a very good focus indicator for the volume control).
  • The <label>s on the power/bank toggles are not really doing anything because they don’t have any text in them. I would suggest you move the “Power:” and “Bank:” text inside the labels. The volume control does not have a <label> at all.
  • This is very nit-picky but if I start with my browser as skinny as it will go and gradually widen it, at some point the drum pad actually gets skinnier again. It’s not a huge change but it seems a little odd to me that the drum pad would get skinnier as I widen the browser window.

Overall, very nice job.

Hello,

Thanks for the freedback!

I shall have a look through and make sure the labels are redone (no point having something if its not correct :smiley: )

I have actually made this with no media query, i was trying to get it responsive through all levels as an experiment so thats why there is the jump at roughly 765px width, I do agree its rather jarring so will take a look and see if i can polish that up!

thank you very much :smiley:

I wouldn’t call it jarring, it’s not much of a change if you ask me, it just struck me as unnecessary.

Isn’t there a media query break point?

@media (max-width: 768px) {
    #drum-machine {
        width: 80vw;
    }
}

Personally, I would not place a width on the #drum-machine. You have a max-width on it. Why would you want to limit the width?

Ah! That’s a good point I did! I was originally planning to see if I could do it without but then found for smaller screens i really needed to.

I found the by taking the width property out and leaving it with max width that when the text changed to a longer length, ie “FCC Drum Machine” the width of the machine changed with it and then went small when the display was blank. Having the width property kept it consistent but presented different issues :slight_smile: as usual!

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