Project Feedback

[friendo9876]

@KatieMFritz Can you take a look at this? It's hosted here and meets the following criteria:

• [x] At least 3 button elements are included in your page
• [x] When each of the buttons are hovered over (mouseenter event) or clicked on (click event), a unique sound effect plays from an audio element (try the .play() method) -- you may use these drum kit sounds, create ones from this sound effects generator, or use any other sounds that won't hurt/irritate the people who listen to them
• [x] Each button is well-labeled, corresponding to the sound it will play
• [x] All audio elements are invisible (no controls attribute), so that they can only be played with the buttons
• [x] All of your JavaScript is in a separate file ending with .js

[KatieMFritz]

Mateen, I can see you've got some experience as a developer - there are a lot of really awesome things about your project! 👍

• Great commit messages. 🔥
• HTML is valid and well-indented and commented ✔️
• Nice use of a Bootstrap button group - this is one of best styled versions of this project I have seen. 👀
• Audio files are in their own folder to keep them organized and out of the way. 📁
• The app itself works exactly as intended, from the user's perspective 🥁

However, this project is intended to help you learn/practice vanilla JavaScript. jQuery is often very helpful, but it's important that you understand the basics of JavaScript for situations where jQuery isn't available or appropriate. In a project like this where it's not necessary, jQuery makes the page bigger and slower to download.

Please refactor your JS for this project to use plain JavaScript. Specifically, we want you to be using addEventListener() instead of jQuery events. If you have any questions about the project requirements, feel free to reach out to me or another mentor on Slack - we're around. 🙂

In the future, if you'd like to try doing a project using different skills/tools than the ones taught in that lesson, reach out to us ahead of time. We want you to get as much as you can out of this course, so we're definitely open to variations! But we want to be on the same page. Sound good?

[friendo9876]

I have updated my code, please look again

[KatieMFritz]

Great! 🎉

Reduce repeated code with variables

I notice you're repeating document.getElementById() for each of your event listeners. Any time you're repeating the exact same document.getElementById(), you'll want to create a variable for it instead.

So instead of

document.getElementById('snare').addEventListener('click', snarefunction1)
document.getElementById('snare').addEventListener('mouseover', snarefunction2)

you'd use something like

var snareButton = document.getElementById('snare')
snareButton.addEventListener('click', snarefunction1)
snareButton.addEventListener('mouseover', snarefunction2)

This way, if you decide later to change the id in the HTML, for example, you only have to change it in the JS once, instead of twice - and whenever you have to change the same code more than once, it opens the door to bugs. 🐛 🐞

Well-named variables also make your code easier to skim and understand. snareButton lets you know exactly what part of the user interface we're referring to.

---

Descriptive Variable Names

Your variables with numbers at the end, like snarefunction1 and snarefunction2, don't tell me what they actually do. Rename them to be more explicit. Don't forget camelCase! 🐫

---

mouseenter vs. mouseover

I'm excited to see that you know about even more events than we've covered in the lessons! We don't recommend using mouseover instead of mouseenter though, as there's an important difference. If you check out the demo at the bottom of this page, you'll see that while mouseenter fires only when initially entering the element, mouseover will also fire whenever entering or leaving a child element, which I've never actually found useful - and can actually cause unintended behavior in many cases.

Right now, there are no other elements inside of your buttons, but you never know when you might want to add child elements. For example, you might decide to add icons that match each drum sound. Because of this, I would recommend just staying away from mouseover. 😄

---

Question about difference between click and mouseover behavior

This isn't something you necessarily have to change, but I'm curious why you decided to use different behavior for mouseover vs. click on your buttons. What's your reasoning? 🖱

It's cool how instead of waiting for the sound to finish playing, I can keep playing the sound with click - it feels more real that way! I had a little jam session on your app. 😁 🎧 🥁

---

Let me know if you have any more questions! Otherwise, after you’ve made your changes and pushed them to GitHub and your hosted site, give it a once-over to make sure it looks right, then comment back here and I’ll take another look.

Thanks! :rocket:

[friendo9876]

I have pushed my changes, so please look at it again. The reason why on mouseenter it doesn't restart when you go over again is that I didn't want the user have the sound restart every time the hovered over the same button again because I did that frequently and drove me nuts

[KatieMFritz]

LOL, okay! I'm all for not driving yourself nuts. 😁

Nice job using variables to reduce duplicate code. 👍

:shipit:

About your variable names - they're definitely better now that they're not using numbers. 😄 However, variables like bassClickFunction, and bassHoverFunction are still are describing what the function is, not what it does. What if you decided to swap the behavior for click and mouseenter? You'd have to rename or rewrite those functions, too. In this situation, it would help to name the functions after the different sound behaviors (I'm not sure how to describe them off the top of my head, but if you want to brainstorm together, we can).

Naming stuff is a deceptively tricky part of coding. I've been working a lot lately on how I name variables in my own code. It forces you to think really hard about what your functions are doing, which is annoying, but also helpful. 😆

[friendo9876]

I have renamed the function name so please take a look again