How can I shorten this code?

var contect1 = document.getElementById("contect1");
		var contect2 = document.getElementById("contect2");
		var contect3 = document.getElementById("contect3");
		var contect4 = document.getElementById("contect4");
		var contect5 = document.getElementById("contect5");

		var dst = document.getElementById("dst");
		var ds = document.getElementById("ds");
		var oni = document.getElementById("oni");
		var cc = document.getElementById("cc");
		var ba = document.getElementById("ba");



		function openDST(){
			contect1.style.display = "grid";
			contect2.style.display = "none";
			contect3.style.display = "none";
			contect4.style.display = "none";
			contect5.style.display = "none";
			dst.style.color = "#2eb05b";
			ds.style.color = "#000";
			oni.style.color = "#000";
			cc.style.color = "#000";
			ba.style.color = "#000";
		}

		function openDS(){
			contect1.style.display = "none";
			contect2.style.display = "grid";
			contect3.style.display = "none";
			contect4.style.display = "none";
			contect5.style.display = "none";
			dst.style.color = "#000";
			ds.style.color = "#2eb05b";
			oni.style.color = "#000";
			cc.style.color = "#000";
			ba.style.color = "#000";
		}

		function openONI(){
			contect1.style.display = "none";
			contect2.style.display = "none";
			contect3.style.display = "grid";
			contect4.style.display = "none";
			contect5.style.display = "none";
			dst.style.color = "#000";
			ds.style.color = "#000";
			oni.style.color = "#2eb05b";
			cc.style.color = "#000";
			ba.style.color = "#000";
		}

		function openCC(){
			contect1.style.display = "none";
			contect2.style.display = "none";
			contect3.style.display = "none";
			contect4.style.display = "grid";
			contect5.style.display = "none";
			dst.style.color = "#000";
			ds.style.color = "#000";
			oni.style.color = "#000";
			cc.style.color = "#2eb05b";
			ba.style.color = "#000";
		}

		function openBA(){
			contect1.style.display = "none";
			contect2.style.display = "none";
			contect3.style.display = "none";
			contect4.style.display = "none";
			contect5.style.display = "grid";
			dst.style.color = "#000";
			ds.style.color = "#000";
			oni.style.color = "#000";
			cc.style.color = "#000";
			ba.style.color = "#2eb05b";
		}

Try this:

const elements = [
  { id: 'contect1', color: '#2eb05b' },
  { id: 'contect2', color: '#000' },
  { id: 'contect3', color: '#000' },
  { id: 'contect4', color: '#000' },
  { id: 'contect5', color: '#000' },
];

const tabs = [
  { id: 'dst', displayIndex: 0 },
  { id: 'ds', displayIndex: 1 },
  { id: 'oni', displayIndex: 2 },
  { id: 'cc', displayIndex: 3 },
  { id: 'ba', displayIndex: 4 },
];

function openTab(displayIndex) {
  elements.forEach((element, index) => {
    element.style.display = index === displayIndex ? 'grid' : 'none';
  });
  
  tabs.forEach((tab, index) => {
    tab.style.color = index === displayIndex ? '#2eb05b' : '#000';
  });
}

tabs.forEach((tab, index) => {
  const element = document.getElementById(tab.id);
  element.addEventListener('click', () => openTab(index));
});

Am I right in assuming you are working with a tabs component or accordion?

If the above doesn’t work, please post some HTML to accompany the JavaScript.

5 Likes

I believe this post will give more context. I suggested the JS could be improved :slight_smile:

2 Likes

Kind of similar to James’ approach…

			<button id="dst" onclick="openDST()">DST</button>
			<button id="ds" onclick="openDS()">DS</button>
			<button id="oni" onclick="openONI()">ONI</button>
			<button id="cc" onclick="openCC()">Cookie Clicker</button>
			<button id="ba" onclick="openBA()">Big Ambitions</button>

replace all those onclicks with nothing.

let buttons = document.querySelectorAll("button");
let tabs = document.querySelectorAll(".container > div");
buttons.forEach((button) => { button.addEventListener("click",doClass); });

function doClass() {
  let target = Array.from(buttons).indexOf(this);
  buttons.forEach((button) => { button.classList.remove("activebutton"); })
  buttons[target].classList.add("activebutton");
  tabs.forEach((tab) => { tab.classList.remove("activetab"); })
  tabs[target].classList.add("activetab");
}

CSS:

.activebutton { color: #2eb05b }
.activetab { display : grid }

(PS: You’ve misspelt container on your div (or it’s a different language? whatever). Threw my code off :stuck_out_tongue: Adjust as necessary.)

3 Likes

Thanks, Paul.

Then a little tweaking is required:

const elements = [
  { id: 'contect1', color: '#2eb05b' },
  { id: 'contect2', color: '#000' },
  { id: 'contect3', color: '#000' },
  { id: 'contect4', color: '#000' },
  { id: 'contect5', color: '#000' },
];

const tabs = [
  { id: 'dst', displayIndex: 0 },
  { id: 'ds', displayIndex: 1 },
  { id: 'oni', displayIndex: 2 },
  { id: 'cc', displayIndex: 3 },
  { id: 'ba', displayIndex: 4 },
];

function openTab(displayIndex) {
  elements.forEach((obj, index) => {
    const element = document.getElementById(obj.id);
    element.style.display = index === displayIndex ? 'grid' : 'none';
  });

  tabs.forEach((obj, index) => {
    const tab = document.getElementById(obj.id);
    tab.style.color = index === displayIndex ? '#2eb05b' : '#000';
  });
}

tabs.forEach((tab, index) => {
  const element = document.getElementById(tab.id);
  element.addEventListener('click', () => openTab(index));
});

Working example:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Tabs</title>
  <style>

    html:
    *{
      font-family: "Roboto";
      margin: 0px;
      padding-bottom: 10px;
    }

    p {
      font-size: 70px;
      padding-left: 10px;
      text-shadow: -2px 5px 15px #C2C2C2;
    }

    .games{
      width: 100%;
      height: 1800px;
      position: relative;
      overflow: hidden;
    }

    .tabs {
      display: flex;
      border-bottom: 1px solid #ccc;
    }

    .tabs button{
      background: transparent;
      border: none;
      outline: none;
      cursor: pointer;
      font-size: 25px;
      font-weight: bold;
      padding: 8px 12px;
      text-shadow: -2px 5px 15px #C2C2C2;
      transition: 0.2s;
    }

    .tabs button:hover {
      color: #2eb05b;
    }

    .tabs button:active {
      color:#00571e;
    }

    #sqaure {
      width: 350px;
      height: 450px;
      border-radius: 7%;
      box-shadow: rgba(100, 100, 111, 0.2) 0px 7px 29px 0px;
      align-content: center;
    }

    h1 {
      text-align: center;
      padding-top: 20px;
      text-shadow: 4px 3px 14px rgba(0,0,0,0.35);
    }

    .contect {
      grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
      display: none;
      align-items: center;
      width: 100%;
      padding-top: 30px;
      margin-left: 6px;
      padding-left: 10px;
      padding-right: 10px;
    }

    #contect1 {
      display: grid;
    }

    #dst{
      color: #2eb05b;
    }

    .tabs button:focus {
      outline: none;
    }

    .tabs button:hover {
      color: #2eb05b!important;
    }
  </style>
</head>
<body>
  <div class="games">
    <div class="tabs">
      <button id="dst">DST</button>
      <button id="ds">DS</button>
      <button id="oni">ONI</button>
      <button id="cc">Cookie Clicker</button>
      <button id="ba">Big Ambitions</button>
    </div>
    <div class="contanier">
      <div id="contect1" class="contect">
        <div id="sqaure">
          <h1>Find Base Location</h1>
        </div>
        <div id="sqaure">
          <h1>Find Base Location</h1>
        </div>
        <div id="sqaure">
          <h1>Find Base Location</h1>
        </div>
        <div id="sqaure">
          <h1>Find Base Location</h1>
        </div>
        <div id="sqaure">
          <h1>Find Base Location</h1>
        </div>
      </div>
      <div id="contect2" class="contect">
        <div id="sqaure">
          <h1>Sun Cool Emote</h1>
        </div>
        <div id="sqaure">
          <h1>Sun Cool Emote</h1>
        </div>
        <div id="sqaure">
          <h1>Sun Cool Emote</h1>
        </div>
        <div id="sqaure">
          <h1>Sun Cool Emote</h1>
        </div>
        <div id="sqaure">
          <h1>Sun Cool Emote</h1>
        </div>
      </div>
      <div id="contect3" class="contect">
        <div id="sqaure">
          <h1>Klei Blei Glei</h1>
        </div>
        <div id="sqaure">
          <h1>Klei Blei Glei</h1>
        </div>
        <div id="sqaure">
          <h1>Klei Blei Glei</h1>
        </div>
        <div id="sqaure">
          <h1>Klei Blei Glei</h1>
        </div>
        <div id="sqaure">
          <h1>Klei Blei Glei</h1>
        </div>
      </div>
      <div id="contect4" class="contect">
        <div id="sqaure">
          <h1>Very Golden Cookie</h1>
        </div>
        <div id="sqaure">
          <h1>Very Golden Cookie</h1>
        </div>
        <div id="sqaure">
          <h1>Very Golden Cookie</h1>
        </div>
        <div id="sqaure">
          <h1>Very Golden Cookie</h1>
        </div>
        <div id="sqaure">
          <h1>Very Golden Cookie</h1>
        </div>
      </div>
      <div id="contect5" class="contect">
        <div id="sqaure">
          <h1>Very Large Buinsess</h1>
        </div>
        <div id="sqaure">
          <h1>Very Large Buinsess</h1>
        </div>
        <div id="sqaure">
          <h1>Very Large Buinsess</h1>
        </div>
        <div id="sqaure">
          <h1>Very Large Buinsess</h1>
        </div>
        <div id="sqaure">
          <h1>Very Large Buinsess</h1>
        </div>
      </div>
    </div>
  </div>

  <script>
    const elements = [
      { id: 'contect1', color: '#2eb05b' },
      { id: 'contect2', color: '#000' },
      { id: 'contect3', color: '#000' },
      { id: 'contect4', color: '#000' },
      { id: 'contect5', color: '#000' },
    ];

    const tabs = [
      { id: 'dst', displayIndex: 0 },
      { id: 'ds', displayIndex: 1 },
      { id: 'oni', displayIndex: 2 },
      { id: 'cc', displayIndex: 3 },
      { id: 'ba', displayIndex: 4 },
    ];

    function openTab(displayIndex) {
      elements.forEach((obj, index) => {
        const element = document.getElementById(obj.id);
        element.style.display = index === displayIndex ? 'grid' : 'none';
      });

      tabs.forEach((obj, index) => {
        const tab = document.getElementById(obj.id);
        tab.style.color = index === displayIndex ? '#2eb05b' : '#000';
      });
    }

    tabs.forEach((tab, index) => {
      const element = document.getElementById(tab.id);
      element.addEventListener('click', () => openTab(index));
    });
  </script>
</body>
</html>

OP, please note that you have 25 elements with an ID of square. IDs should be unique to a page, so you should change this into a class.

3 Likes

Yeah, that’s neater. There are a couple of adjustments the OP will need to make to have your script work 100%.

Remove this CSS:

#contect1 {
  display: grid;
}

#dst{
  color: #2eb05b;
}

and add a line of JS to open the first tab:

buttons[0].click();

Which gives us:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Tabs</title>
  <style>
    *{
      font-family: "Roboto";
      margin: 0px;
      padding-bottom: 10px;
    }

    p {
      font-size: 70px;
      padding-left: 10px;
      text-shadow: -2px 5px 15px #C2C2C2;
    }

    .games{
      width: 100%;
      height: 1800px;
      position: relative;
      overflow: hidden;
    }

    .tabs {
      display: flex;
      border-bottom: 1px solid #ccc;
    }

    .tabs button{
      background: transparent;
      border: none;
      outline: none;
      cursor: pointer;
      font-size: 25px;
      font-weight: bold;
      padding: 8px 12px;
      text-shadow: -2px 5px 15px #C2C2C2;
      transition: 0.2s;
    }

    .tabs button:hover {
      color: #2eb05b;
    }

    .tabs button:active {
      color:#00571e;
    }

    .square {
      width: 350px;
      height: 450px;
      border-radius: 7%;
      box-shadow: rgba(100, 100, 111, 0.2) 0px 7px 29px 0px;
      align-content: center;
    }

    h1 {
      text-align: center;
      padding-top: 20px;
      text-shadow: 4px 3px 14px rgba(0,0,0,0.35);
    }

    .contect {
      grid-template-columns: 1fr 1fr 1fr 1fr 1fr;
      display: none;
      align-items: center;
      width: 100%;
      padding-top: 30px;
      margin-left: 6px;
      padding-left: 10px;
      padding-right: 10px;
    }

    .tabs button:focus {
      outline: none;
    }

    .tabs button:hover {
      color: #2eb05b!important;
    }

    .activebutton { 
      color: #2eb05b 
    }

    .activetab { 
      display : grid 
    }
  </style>
</head>

<body>
  <div class="games">
    <div class="tabs">
      <button id="dst">DST</button>
      <button id="ds">DS</button>
      <button id="oni">ONI</button>
      <button id="cc">Cookie Clicker</button>
      <button id="ba">Big Ambitions</button>
    </div>
    <div class="container">
      <div id="contect1" class="contect">
        <div class="square">
          <h1>Find Base Location</h1>
        </div>
        <div class="square">
          <h1>Find Base Location</h1>
        </div>
        <div class="square">
          <h1>Find Base Location</h1>
        </div>
        <div class="square">
          <h1>Find Base Location</h1>
        </div>
        <div class="square">
          <h1>Find Base Location</h1>
        </div>
      </div>
      <div id="contect2" class="contect">
        <div class="square">
          <h1>Sun Cool Emote</h1>
        </div>
        <div class="square">
          <h1>Sun Cool Emote</h1>
        </div>
        <div class="square">
          <h1>Sun Cool Emote</h1>
        </div>
        <div class="square">
          <h1>Sun Cool Emote</h1>
        </div>
        <div class="square">
          <h1>Sun Cool Emote</h1>
        </div>
      </div>
      <div id="contect3" class="contect">
        <div class="square">
          <h1>Klei Blei Glei</h1>
        </div>
        <div class="square">
          <h1>Klei Blei Glei</h1>
        </div>
        <div class="square">
          <h1>Klei Blei Glei</h1>
        </div>
        <div class="square">
          <h1>Klei Blei Glei</h1>
        </div>
        <div class="square">
          <h1>Klei Blei Glei</h1>
        </div>
      </div>
      <div id="contect4" class="contect">
        <div class="square">
          <h1>Very Golden Cookie</h1>
        </div>
        <div class="square">
          <h1>Very Golden Cookie</h1>
        </div>
        <div class="square">
          <h1>Very Golden Cookie</h1>
        </div>
        <div class="square">
          <h1>Very Golden Cookie</h1>
        </div>
        <div class="square">
          <h1>Very Golden Cookie</h1>
        </div>
      </div>
      <div id="contect5" class="contect">
        <div class="square">
          <h1>Very Large Buinsess</h1>
        </div>
        <div class="square">
          <h1>Very Large Buinsess</h1>
        </div>
        <div class="square">
          <h1>Very Large Buinsess</h1>
        </div>
        <div class="square">
          <h1>Very Large Buinsess</h1>
        </div>
        <div class="square">
          <h1>Very Large Buinsess</h1>
        </div>
      </div>
    </div>
  </div>

  <script>
    let buttons = document.querySelectorAll("button");
    let tabs = document.querySelectorAll(".container > div");
    buttons.forEach((button) => { button.addEventListener("click", doClass); });

    function doClass() {
      let target = Array.from(buttons).indexOf(this);
      buttons.forEach((button) => { button.classList.remove("activebutton"); })
      buttons[target].classList.add("activebutton");
      tabs.forEach((tab) => { tab.classList.remove("activetab"); })
      tabs[target].classList.add("activetab");
    }

    buttons[0].click();
  </script>
</body>
</html>

I fixed a couple of spelling mistakes and made the IDs a class.

2 Likes

Alternatively, give #dst the “activebutton” class and #contect1 the “activetab” class (the script will add/remove it from the elements as needed. Either way works though :slight_smile:

3 Likes

Lol, again that’s the better way, as then at least one tab will display if the user has JS disabled.

3 Likes

It has been more than 2 weeks and we have not heard from ron122, right?

I think it is not clear whether the intent is to make the code shorter for computer time or programmer time or just making it fewer lines.

1 Like

just making it fewer lines

There are many good reasons to use getElementById and many developers will criticize anyone that does not but your code will likely work the same if you remove the 10 lines that call getElementById. Instead of removing them you can comment them out to see what happens.

1 Like

That has been achieved in depth by the posts from @m_hutley and @James_Hibbard :slight_smile:

2 Likes

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.