I need help with javascript.

Question

Hello 👋 I have created a flex layout with 5 bulbs and I want to achieve is that when I click on each of the bulb I want them to glow 💡 individually and when I click again it should turn off.

Please look at the code for better explanation. Thank you!
https://code.sololearn.com/WKQpfWQSf3bT/?ref=app

Answer 1

You could also just pass the function changeImage to the addEventListener as a callback, then you will be able to use (this) in changeImage() to select the proper element

https://code.sololearn.com/Wa13a4A14a4A

Answer 2

to benefit of 'this' set to the event target, you need to NOT use arrow function but 'normal' function (either expression / declaration / anonymous or not):

imageElement.addEventListener('click', function() {
// function body
});

with both arrow and 'normal' functions, the event object is available as first argument... on this object you can read the 'target' and 'currentTarget' properties: former is the event target (on wich is attached the event listener), later is the event wich receive/raise the event (could be a nested element in the 'target' element) ;)

Answer 3

kannan None of the other solutions are using jQuery either. 🤔 Was that your impression?

Anyway... your updated solution now has a dependency on the order of each div.panel element explicitly matching its corresponding position in the list of elements. This is actually worse than using the id approach in your original version.

The code is now more brittle to minor changes over time as new panels are manually and/or dynamically added, moved around, or removed. All panel elements would unnecessarily require being updated to reflect their new ordinal positions.

Tight coupling the onclick attribute to a function further makes this brittle in that all instances would need to be updated if changes are made to the function name.

Therefore, IMHO, while the intentions with this update are good, it's still advocating some very bad practices that should just be avoided regardless of the level of a beginner.

Answer 4

visph thanks for the brief, it explains well.

Answer 5

I'm a bit partial to something like this in the JS tab:

(() => {
const main = () => document
.querySelectorAll(".panel")
.forEach(panel =>
panel.addEventListener("click", changeImage))

const bulb_on =
"https://i.ibb.co/Tq56dxM/pic-bulbon.gif"

const bulb_off =
"https://i.ibb.co/sWppp3w/pic-bulboff.gif"

const changeImage = e => {
const img = e.currentTarget
.querySelector("img")

img.src = img.src === bulb_off
? bulb_on : bulb_off
}

window.addEventListener("load", main)
})()

Answer 6

panel.addEventListener("click", () => {
for(i=0;i<panels.length;i++){
panels[i].children[0].id=""
}
panel.children[0].id="lightbulb"
changeImage();
});

Answer 7

kannan Your version only works if clicking directly on the image. It doesn't trigger the changeImage() function if clicking on the other parts of the panel.

While this may work if moving the onclick attribute to the img parent, the approach is still a bit naive as it promotes an antipattern that unnecessarily and explicitly couples the event handler with each HTML element ID and markup tag.

Take a moment to study the previous answers to see how this can be further abstracted. 😉

Answer 8

Scooby if you want to go through panel then you need to use index.

for(i=0;i<panels.length;i++){
for(j=0;j<panels[i].children.length;j++){
if(panels[i].children[j].nodeName=="IMG"){

panels[i].children[j].id=""
}
}
}

for(k=0;k<panel.children.length;k++){
if(panel.children[k].nodeName=="IMG"){
panel.children[k].id="lightbulb"
}
}

The above code will work no matter the position of img tag in panel div.

If this is what you don't want maybe try to clarify a little more about what you are looking for !

Answer 9

Scooby add id property to all the 5 img tags and use onClick property to call js function. Here I have used position of bulbs to refer their individual IDs. Do take a look at the code
https://code.sololearn.com/WsIWjpEL0prP/?ref=app

Answer 10

David Carroll yes. It's right. But I thought of providing simple solution without using jQuery. I have optimized it further by removing those IDs. Do take a look on the updated code. Thanks