Changing the text of a few elements when a tile is clicked

Question

I have created a basic web page that contains a panel which displays some details about an object, and a panel which contains some tiles that are used to choose which object to view. The idea is straight-forward, and with plenty of C# experience, the learning curve was small. However, since I am relatively new to JavaScript, I'd like a brief code review of my JavaScript to determine if there are any gotchas or perhaps alternatives that are more graceful.

// The available titles and their associated data.
var titles = ["Pong", "Tetris", "Pac-Man", "Snake", "Super Mario", "Pokemon", "Match 3", "Blossom Blast", "Driven", "Terrible Zombies"];
var data = [
  ["Pong is the equivalent of Hello World in the game development industry.", "Vectors, Rendering", "Directions", "Adding obstacles.", "Add effects."]
  ["Tetris is an iconic title. Though I am unable to provide the greatest theme music of all time, I hope you thoroughly enjoy the remake!", "Multi-Dimensional Arrays", "Arrays of Arrays", "Creating new pieces.", "Add effects."]
  ["Pac-Man is an iconic title providing a lot of fun and some great concepts.", "Collision Detection", "Building the walls.", "Object pooling.", "Add more ghosts and effects."]
  ["Dialing it back down, Snake is another iconic title with great concepts but simple implementation.", "Collision Detection", "None", "Linked Lists", "None"]
  ["Another iconic title, Super Mario is known around the world.", "Parallaxing Backgrounds", "None", "None", "None"]
  ["Pokemon is a favorite of multiple generations, this remake is simple and for demonstrations only.", "None", "None", "None", "None"]
  ["This is a simple demonstration of the match three game logic.", "None", "None", "None", "None"]
  ["The beautiful game Blossom Blast was my inspiration to go mobile.", "None", "None", "None", "None"]
  ["Driven is a basic 3D driving game.", "None", "None", "None", "None"]
  ["Terrible zombies is just as the name states, a terrible zombie game.", "None", "None", "None", "None"]
];

function swapGame(domElement, title) {
  // The tile collection.
  var tileCollection = document.getElementsByClassName('tiles');
  var tiles = tileCollection[0].getElementsByTagName('li');

  // The labels for display.
  var lblTitle = document.getElementById('dTitle');
  var lblDescription = document.getElementById('dDescription');
  var lblConcepts = document.getElementById('dConcepts');
  var lblChallenges = document.getElementById('dChallenges');
  var lblAdvanced = document.getElementById('dAdvanced');
  var lblHomework = document.getElementById('dHomework');
  for (var i = 0; i < tiles.length; i++)
    tiles[i].className = '';

  domElement.classList.toggle('active');

  lblTitle.innerHTML = title;

  var id = titles.indexOf(title);
  lblDescription.innerHTML = data[id][0];
  lblConcepts.innerHTML = 'Concepts: ' + data[id][1];
  lblChallenges.innerHTML = 'Challenges: ' + data[id][2];
  lblAdvanced.innerHTML = 'Advanced: ' + data[id][3];
  lblAdvanced.innerHTML = 'Homework: ' + data[id][4];
}

.tile-container {
  width: 100%;
  height: 100%;
  display: flex;
}

.view-panel {
  width: 30%;
  box-shadow: 0px 5px 15px #111;
  background-color: #fff;
  color: #333;
  padding: 5px;
}

.launch-button {
  background-color: #fc0;
  border-radius: 5px;
  height: 50px;
  width: 95%;
  margin: auto;
  cursor: wait;
  transition: 0.5s all;
}

.launch-button:hover {
  background-color: #da0;
}

.launch-button a {
  text-decoration: none;
  color: #333;
  width: 100%;
  height: 100%;
  display: flex;
  align-items: center;
  justify-content: center;
}

.tiles {
  width: 70%;
  display: flex;
  align-items: flex-start;
  justify-content: flex-start;
}

.tile-view {
  list-style: none;
  width: 100%;
  display: flex;
  flex-wrap: wrap;
  align-items: flex-start;
  justify-content: center;
  padding: 0;
}

.tile-view li {
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  will-change: all;
  width: 150px;
  height: 150px;
  cursor: pointer;
  transition: 0.5s background-color;
  margin-left: 5px;
  margin: 5px;
  background-color: #222;
  color: #eee;
  box-shadow: 2px 2px 5px #111;
}

.tile-view li>a {
  color: #eee;
  text-decoration: none;
}

.tile-view li:hover {
  background-color: #fc0;
  box-shadow: 5px 5px 15px #111;
}

.tile-view li.active {
  background-color: #fc0;
}

.tile-view li.active,
.tile-view li.active>a,
.tile-view li:hover,
.tile-view li:hover>a {
  color: #333;
}

@media screen and (max-width: 750px) {
  .view-panel {
    display: none;
  }
  .tiles {
    width: 100%;
  }
  .tile-view li {
    width: 300px;
  }
}

<div class="tile-container">
  <div class="view-panel">
    <h1 id="dTitle">Pong</h1>
    <p id="dDescription">Pong is the equivalent of Hello World in the game development industry.</p>
    <ul>
      <li><span id="dConcepts">Concepts: Vectors, Rendering</span></li>
      <li><span id="dChallenges">Challenges: Directions</span></li>
      <li><span id="dAdvanced">Advanced: Adding obstacles.</span></li>
      <li><span id="dHomework">Homework: Add effects.</span></li>
    </ul>
    <div class="launch-button"><a href="#">Launch</a></div>
  </div>
  <div class="tiles">
    <ul class="tile-view">
      <li onclick="swapGame(this, 'Pong')" class="active">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Pong</a>
      </li>
      <li onclick="swapGame(this, 'Tetris')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Tetris</a>
      </li>
      <li onclick="swapGame(this, 'Pac-Man')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Pac-Man</a>
      </li>
      <li onclick="swapGame(this, 'Snake')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Snake</a>
      </li>
      <li onclick="swapGame(this, 'Super Mario')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Super Mario</a>
      </li>
      <li onclick="swapGame(this, 'Pokemon')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Pokemon</a>
      </li>
      <li onclick="swapGame(this, 'Match 3')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Match 3</a>
      </li>
      <li onclick="swapGame(this, 'Blossom Blast')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Blossom Blast</a>
      </li>
      <li onclick="swapGame(this, 'Driven')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Driven</a>
      </li>
      <li onclick="swapGame(this, 'Terrible Zombies')">
        <i class="fas fa-gamepad" aria-hidden="true"></i>
        <a href="#">Terrible Zombies</a>
      </li>
    </ul>
  </div>
</div>

Is there a reason why you want the text to be partly stored in the JavaScript code and partly in the HTML, rather than all in JavaScript or all in HTML? — 200_success
– 200_success, Commented Jan 25, 2019 at 2:50
@200_success Not really, I prefer consolidation but I haven't made it to the point of calling the setGame function once the DOM has loaded. — Taco
– Taco, Commented Jan 25, 2019 at 3:00
data is missing commas between arrays. Is .view-panel element and child nodes expected to not be displayed in the document? — guest271314
– guest271314, Commented Jan 25, 2019 at 3:59

Joseph · Accepted Answer · 2019-01-25 14:33:17Z

It could go either way, but for this small example, I would suggest moving all the content in the HTML. You could even skip the JS and use :target to show and hide stuff in conjunction with hashed link hrefs.

.panel {
  display: none
}

.panel:target {
  display: block
}

<div class="app">
  <div class="panels">
    <div class="panel" id="pong">
      <h1>Pong</h1>
      <p>Lorem ipsum sit dolor amet...</p>
    </div>
    <div class="panel" id="pokemon">
      <h1>Pokemon</h1>
      <p>Lorem ipsum sit dolor amet...</p>
    </div>
    <div class="panel" id="donkey-kong">
      <h1>Donkey Kong</h1>
      <p>Lorem ipsum sit dolor amet...</p>
    </div>
  </div>
  <nav class="menu">
    <ul>
      <li><a href="#pong">Pong</a></li>
      <li><a href="#pokemon">Pokemon</a></li>
      <li><a href="#donkey-kong">Donkey Kong</a></li>
    </ul>
  </nav>
</div>

Now onto your code...

I recommend using const over var. Nothing wrong with var, but const guarantees the value referenced by the variable never changes (i.e. you cannot reassign it). This ensures that whatever you set to it is the same thing later in code. It's also block-scoped, so if you're in ifs or fors, it scopes it in the block.

In JS, there's document.querySelector and document.querySelectorAll. These allow you to fetch DOM elements using CSS selectors. You target DOM elements in the same way you target them when writing CSS. This way, you can be more expressive instead of being limited to getElementById, getElementsByTagName, getElementsByClassName.

element.innerHTML is fine. But if you're just updating text, consider using element.textContent instead.

Instead of onclick on the HTML, use element.addEventListener in JavaScript instead. Inline scripts, while legit, are discouraged due to separation of concerns. Also, in inline scripts, the function is a global. Globals are to be avoided in JS to avoid clobbering stuff in the global namespace.

Avoid targetting HTML elements in your CSS selectors. For instance .tile-view li targets all descendant li elements under .tile-view. This is fine for small apps, but this is a bad habit to have when working on larger apps. On larger apps, where components are composed of smaller independent components, you never know what's in them. You may be hitting an li you did not originally anticipate to be under there.

Stack Exchange Network

Changing the text of a few elements when a tile is clicked

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Changing the text of a few elements when a tile is clicked

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions