CODEWITHSUNDEEP
javascript
Pscx1
Pscx1

Reputation: 1

Better, shorter html, javascript

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";

}

Upvotes: 0

Views: 59

Answers (3)

Barmar
Barmar

Reputation: 780798

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";
});

Upvotes: 1

WillD
WillD

Reputation: 6512

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";
});

Upvotes: 0

XMehdi01
XMehdi01

Reputation: 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()");
}

Upvotes: 1

Related Questions