0

I have 9 buttons which i made mouve hover pointer and divs clickable which opens next php files. I have made them to lose pointer style after pressing and retrieve it after pressing other button and everything works fine. But is there some way to make this code shorter and better?

It's code for single button

function Postac()
{
 document.getElementById("Okno_Gry").innerHTML='<object data = "Postac.php" width = "100%" height = "100%" ></object>';
 document.getElementById('PrzyciskPostac').removeAttribute("onclick"), 
 document.getElementById("PrzyciskPostac").style.cursor = "default";
 
 document.getElementById("PrzyciskWarsztat").setAttribute("onclick", "Warsztat()");
 document.getElementById("PrzyciskWarsztat").style.cursor = "pointer";
 document.getElementById("PrzyciskTargowisko").setAttribute("onclick", "Targowisko()");
 document.getElementById("PrzyciskTargowisko").style.cursor = "pointer";
 document.getElementById("PrzyciskArena").setAttribute("onclick", "Arena()");
 document.getElementById("PrzyciskArena").style.cursor = "pointer";
 document.getElementById("PrzyciskWyprawy").setAttribute("onclick", "Wyprawy()");
 document.getElementById("PrzyciskWyprawy").style.cursor = "pointer";
 document.getElementById("PrzyciskLochy").setAttribute("onclick", "Lochy()");
 document.getElementById("PrzyciskLochy").style.cursor = "pointer";
 document.getElementById("PrzyciskGildia").setAttribute("onclick", "Gildia()"), 
 document.getElementById("PrzyciskGildia").style.cursor = "pointer";
 document.getElementById("PrzyciskZamek").setAttribute("onclick", "Zamek()");
 document.getElementById("PrzyciskZamek").style.cursor = "pointer";
 document.getElementById("PrzyciskKopalnie").setAttribute("onclick", "Kopalnie()");
 document.getElementById("PrzyciskKopalnie").style.cursor = "pointer";
}
asked Aug 12, 2022 at 20:07
1
  • I'd look at adding an event handler to the parent element and using a single delegated event to handle all of it. Without seeing the HTML it isn't possible to show how to do this. Commented Aug 12, 2022 at 21:07

3 Answers 3

1

You can wrap repeated code:

document.getElementById("PrzyciskWarsztat").setAttribute("onclick", "Warsztat()");
document.getElementById("PrzyciskWarsztat").style.cursor = "pointer";
 ... 
document.getElementById("PrzyciskTargowisko").setAttribute("onclick", "Targowisko()");
document.getElementById("PrzyciskTargowisko").style.cursor = "pointer";

into function and then you can call it:

function clickable(id, method) {
 document.getElementById(id).setAttribute("onclick", method);
 document.getElementById(id).style.cursor = "pointer";
}

Code at the end become short:

function Postac() {
 document.getElementById("Okno_Gry").innerHTML =
 '<object data = "Postac.php" width = "100%" height = "100%" ></object>';
 document.getElementById("PrzyciskPostac").removeAttribute("onclick"),
 (document.getElementById("PrzyciskPostac").style.cursor = "default");
 clickable("PrzyciskWarsztat", "Warsztat()");
 clickable("PrzyciskTargowisko", "Targowisko()");
 clickable("PrzyciskArena", "Arena()");
 clickable("PrzyciskWyprawy", "Wyprawy()");
 clickable("PrzyciskLochy", "Lochy()");
 clickable("PrzyciskGildia", "Gildia()"),
 clickable("PrzyciskZamek", "Zamek()");
 clickable("PrzyciskKopalnie", "Kopalnie()");
}
answered Aug 12, 2022 at 20:15
Sign up to request clarification or add additional context in comments.

Comments

1

Put all the click handler names in an array, then loop over it.

const click_handlers = ["Warsztat", "Targowisko", "Arena", ...];
click_handlers.forEach(name => {
 let el = document.getElementById(`Przycisk${name}`);
 el.setAttribute("onclick", `${name}()`);
 el.style.cursor = "pointer";
});
answered Aug 12, 2022 at 20:13

7 Comments

Missing a { in your arrow function
How it is supposed to remove attribute after pressing and add it back after pressing other button?
Why do you need to remove it?
I need button to be not clickable until other is pressed
You can set the button's disabled property.
|
0

In theory you could use a loop to shorten this up.

Also I replaced your onlick attribute set with a call to element.addEventListener();

const elementsAndCallbacks = [
 { id: "PrzyciskWarsztat", callback: Warsztat },
 { id: "PrzyciskTargowisko", callback: Targowisko },
 //etc
]
elementsAndCallbacks.forEach(item => {
 const el = document.getElementById(item.id);
 el.addEventListener('click', item.callback);
 el.style.cursor = "pointer";
});
answered Aug 12, 2022 at 20:15

Comments

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.