Please review the Challenge Drum Machine

Please review the Challenge Drum Machine
0.0 0

#1

Here is the link to the demo.
https://chintuyadav.github.io/Javascript30/Day-1%20Drum%20Kit/


#2

MAH DUDE! Well done, really nice project, so cool.

The layout is great for desktop, but for mobile elements are so small(I think), and there is a too much space between screen edge(left and right) to content, check:
image

In mobile if you could apply less margin to content could be better to use more space for the keys

I also realized the key content(text) are not center correctly, and they are more to left a little. I found the issue is about .key css rule.
You set the padding value as padding: 2rem 0rem 1rem 1rem; as the 0 value is for right dear, not the bottom. simple fix could be padding: 2rem 1rem 1rem 1rem;, now the txt is center correctly

One small bug(not critical) is when in desktop user press one note key (let say A), and not release the key (assume press and hold A for 2 second).
I think it repeats the sound in a very high rate. This could be great if it waits for more while (for example 500 ms) and again start the sound.
Much better nad pro approach is allowing user set the repeat sound speed. so fats like what is right now, or so slow(maybe 1 per 1 second)
Another issue is when you release the key after a while, sound is stopped as expected, but the CSS rule you apply for the key won’t fallback to normak .key. just check:


I pressed A, S and F and hold them for some second and then release them, you may try and see.
I believe you can fix this small issue.

Another issue(bad one), is the keys are not responsible to mouse click/user tap!? this is not good. user should be able to play a key by mouse click or tap. Mouse hover work in desktop, but we don’t have anything like mouse hover in mobile. So keys should be clickable.
Tip: when user double click to get double sound of a key, browser thinks user is trying to select the text. if you could use the button tag instead of div could fix this issue.

For the mobile view, I have some suggestions.

First, try to apply less body/container margin to content, so you can use more space for it.
Also suggest you remove the name of the key (example boom, tink, …) to have more room for key character (A, F, S,…) And you may add one simple list after the keys to tell user A is clap for instance and so on…

Another solution I like you bring(could be a little too much) is letting the user setup the position it’s own simply by drag and moving each key at any place she/he likes. You may use absolute position for this. And you should have some check if user try to move a key out of the screen, so limit it to screen portion(think about it, and it’s easy to code)
For start, you may allow the user swap one key with another one she/he desires in current layout.

And lats suggestion is about bring more theme colour. This will be awesome.

I say this is very good, try to bring better layout for mobile, same more great contrasted colouring.

Keep going on great work dude, happy programming.


#3

Hey @NULL_dev , thanks for your time and review. I hope I will do whatever you said.
Thank you.