Minesweeper game implementation in (vanilla) JS

Question

I decided to write my own version of a Mine Sweeper game as an excuse for a Javascript/HTML/CSS project. Just to make the question self-contained: Mine Sweeper is a game with a rectangular board consisting of hidden cells. Mines are randomly placed on the board and the goal is to reveal the whole board, except for the mines! Each revealed cell will either have a mine, a number or be empty. The number in the cell tells us how many mines exist in the immediate vicinity of this cell (an empty cell is equivalent to a cell with a 0 in it).

My implementation currently supports:

• Selection of size and difficulty (number of mines).
• A timer of the game which stops when the game is over (win or lose), and resets when it starts.
• Auto-reveal when clicking an empty cell.
• The use of flags with a right-click (or long press on mobile).
• A mine counter showing the current amount of unflagged mines (this relies on flags so it might not be correct at a given moment).

I am pretty happy with the current state and I am posting here after a few iterations and changes, but since this is basically my first big JS/HTML/CSS project I wanted to get it reviewed especially on:

• The general use of scopes - Sometimes I found the need to use global variables although I'm used to them being considered a bad habit. Because of my lack of JS expertise, there might be a better way using them (and scoping in general).
• I implemented the timer essentially through combining this and this Stack Overflow questions. Not sure if it's the best use for this case.
• I used classes to signal some states or to pass information just because it seemed a good use of classes. I'm not sure if that's actually the best way to go (for example, see below the use of the visible class to signal that a cell has been clicked).
• Stating the obvious probably, but of course anything else that you see and I didn't mention. Specifically I only tagged JS as I feel is the main aspect I want reviewed, but of course if the use of HTML/CSS has some obvious wrongs I would like to know as well.

If anyone is interested, the game is also on GitHub.

const grid = document.getElementById("grid");
var gameOver = false;

var placeMines = function(difficulty) {
  const cells = document.getElementsByClassName("cell");
  const minesI = [];

  // Get unique random indexes to place the mines
  mines_count = Math.ceil(cells.length * difficulty);
  const mines_text = document.getElementById("mines");
  mines_text.setAttribute("data-mines", mines_count);
  mines_text.innerHTML = "Mines left: " + mines_count;
  hidden = N * N;
  while (minesI.length < mines_count) {
    var I = Math.floor(Math.random() * cells.length);
    if (minesI.indexOf(I) === -1) minesI.push(I);
  }

  // Set the cells with mines according to the indexes in the minesI array
  for (var i = 0; i < minesI.length; i++) {
    cells[minesI[i]].classList.add("mine");
    cells[minesI[i]].innerHTML = "X";
  }
};

var getImmediateNeighbors = function(cell) {
  const i = [...cells].indexOf(cell);
  let topRow = true;
  let bottomRow = false;
  const neighbors = [];

  if (i > N - 1) {
    topRow = false;
    neighbors.push(cells[i - N]);
  }
  if (i < N * (N - 1)) {
    neighbors.push(cells[i + N]);
  } else bottomRow = true;
  if (i % N != 0) {
    neighbors.push(cells[i - 1]);
    if (!topRow) neighbors.push(cells[i - N - 1]);
    if (!bottomRow) neighbors.push(cells[i + N - 1]);
  }
  if (i % N != N - 1) {
    neighbors.push(cells[i + 1]);
    if (!topRow) neighbors.push(cells[i - N + 1]);
    if (!bottomRow) neighbors.push(cells[i + N + 1]);
  }
  return neighbors;
};

var placeNums = function() {
  for (var i = 0; i < cells.length; i++) {
    if (cells[i].classList.contains('mine')) continue;

    let neighbors = getImmediateNeighbors(cells[i]);
    let count = 0;
    for (var j = 0; j < neighbors.length; j++) {
      count += neighbors[j].classList.contains('mine');
    }

    cells[i].innerHTML = count > 0 ? count : '';
  }
};

var resetBoard = function() {
  document.getElementById('text').innerHTML = "";
  document.getElementById("seconds").innerHTML = "00";
  document.getElementById("minutes").innerHTML = "00";

  const difficulty = document.getElementById("diff").value;
  N = Number(document.getElementById("size").value);

  grid.style.setProperty('grid-template-columns', 'repeat(' + N + ', 30px)');
  grid.style.setProperty('grid-template-rows', 'repeat(' + N + ', 30px)');

  grid.innerHTML = '';
  for (var i = 0; i < N * N; i++) {
    const cell = document.createElement("button");
    cell.className = 'cell';

    grid.appendChild(cell);
  }
  cells = grid.getElementsByClassName("cell");
  for (var i = 0; i < cells.length; i++) {
    cells[i].className = 'cell';
    cells[i].innerHTML = '';
    cells[i].addEventListener('click', clickCell, false);
    cells[i].oncontextmenu = clickFlag;
  }
  gameOver = false;

  placeMines(difficulty);
  placeNums();

  totalSeconds = 0;
  clearInterval(timer);
  timer = setInterval(setTime, 1000);
};

var clickFlag = function() {
  if (gameOver) return;
  if (this.classList.contains('visible')) return;

  mines_text = document.getElementById("mines");
  const mines_count = Number(mines_text.getAttribute("data-mines"));
  if (this.classList.contains('flag')) {
    this.classList.remove('flag');
    mines_text.setAttribute("data-mines", mines_count + 1);
    mines_text.innerHTML = "Mines left: " + (mines_count + 1);
  } else {
    this.classList.add('flag');
    mines_text.setAttribute("data-mines", mines_count - 1);
    mines_text.innerHTML = "Mines left: " + (mines_count - 1);
  }

  return false;
};

var clickCell = function() {
  if (gameOver) return;
  if (this.classList.contains('visible')) return;
  if (this.classList.contains('flag')) return;

  this.classList.add('visible');
  hidden = hidden - 1;

  if (this.classList.contains('mine')) {
    document.getElementById('text').innerHTML = "Game Over";
    endGame();
    return;
  }

  if (hidden === mines_count) {
    document.getElementById('text').innerHTML = "You Won!";
    endGame();
    return;
  }

  if (this.childNodes.length === 0) {
    let neighbors = getImmediateNeighbors(this);
    for (var i = 0; i < neighbors.length; i++) {
      neighbors[i].click();
    }
  }
};

function setTime() {
  ++totalSeconds;
  document.getElementById("seconds").innerHTML = pad(totalSeconds % 60);
  document.getElementById("minutes").innerHTML = pad(parseInt(totalSeconds / 60));
}

function pad(val) {
  var valString = val + "";
  if (valString.length < 2) {
    return "0" + valString;
  } else {
    return valString;
  }
}

function endGame() {
  gameOver = true;
  clearInterval(timer);
}

document.getElementById('reset').addEventListener('click', resetBoard);
resetBoard();

span {
  font-family: Arial, Helvetica, sans-serif;
}

#grid {
  display: grid;
  grid-template-columns: repeat(3, 30px);
  grid-template-rows: repeat(3, 30px);
}

.cell {
  background: LightBlue;
  font-size: 0;
  border: 1px solid Blue;
  border-radius: 2px;
  padding: 10px;
}

.visible {
  font-size: 16px;
  background: LightGrey;
  outline: none;
  -webkit-box-shadow: inset 0px 0px 5px #c1c1c1;
  -moz-box-shadow: inset 0px 0px 5px #c1c1c1;
  box-shadow: inset 0px 0px 5px #c1c1c1;
}

.flag {
  background: yellow;
}

.visible.mine {
  background: Red;
}

<!DOCTYPE html>
<html>

<head>
  <link rel="stylesheet" href="mysweeper.css">
  <script src="mysweeper.js" defer></script>
</head>

<body>
  <h1>My Mine Sweeper</h1>

  <label for="diff">Difficulty:</label>
  <select name="difficulty" id="diff">
    <option value=0.1>Easy</option>
    <option value=0.3>Medium</option>
    <option value=0.6>Hard</option>
  </select>
  <label for="size">Size:</label>
  <select name="size" id="size">
    <option value=4>Small</option>
    <option value=6>Medium</option>
    <option value=10>Big</option>
  </select>
  <button id="reset">Reset</button>

  <div id="mines" data-mines=""></div>
  <div id="timer"><label id="minutes">00</label>:<label id="seconds">00</label></div>
  <div id="grid">
  </div>

  <span id="text"></span>
</body>

</html>

Answer 1

Code review from the outside can be painful. From my own experience I will say that any code can be improved, even the one that seems perfect. However, congratulations on finding the will to figure it out.

HTML

As for Html, almost everything is fine with it. Except for the absence of important tags or attributes, as well as, in my opinion, unnecessary whitespace lines. I also noticed some semantic inaccuracy where span should have been used instead of label.

I would also like to note that attributes in html are exclusively quoted strings and nothing else, even if we are talking about numeric values, for example, value in input type="number" can be "0" (not 0)

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Document</title>
</head>
<body>
<div id="timer"><span id="minutes">00</span>:<span id="seconds">00</span></div>
</body>
</html>

CSS

As for css, here, too, everything is more or less, I would like to emphasize separately that it is better not to use #id in the rules, limit yourself to classes, since ids are intended for js, not for css. Also, if this is important to you, try to use a certain order of properties: size, position, font, color, etc.

.cell {
  padding: 10px;
  font-size: 0;
  background: LightBlue;
  border: 1px solid Blue;
  border-radius: 2px;
}

It's good practice to have the best naming, but this applies to everything, not just css

#grid -> .grid   (.battle-field)

JS

As for js. That although the code as a whole looks good, it has a lot of blunders. For example, using var and const in the same file does not look very neat.

Below are my recommendations:

'use strict'  // avoids many mistakes

var gameOver = false; -> let gameOver = false;

var => const or let

for (var i = 0;) =>  for (let i = 0;)

There is no point in how you declared functions, even your way is definitely worse than the standard approach

var placeMines = function(difficulty) { }  => function placeMines(difficulty) {}

I also noticed the periodic absence of a declaration in front of the variables, they are, that is, absent

hidden = N * N; -> const hidden = N * N;

The code is your domain, so structure it and it will be more pleasant for you to work with it.

const i = [...cells].indexOf(cell);     const i = [...cells].indexOf(cell);
let topRow = true;                      const neighbors = [];
let bottomRow = false;              =>  let bottomRow = false;
const neighbors = [];                   let topRow = true;

Something can be written more concisely and this will save time on reading and changing the code.

count > 0 ? count : '';   =>  count || '';

Try not to use innerHTML it looks like a hack but very ugly and sometimes dangerous.

element.innerHTML = "00";  -> element.textContent = "00";

Interesting approach, but it's much better to use css variables, it will make your code shorter and more elegant

grid.style.setProperty('grid-template-columns', 'repeat(' + N + ', 30px)'); --> css vars

#grid {
  display: grid;
  grid-template-columns: repeat( var(--grid-size), 30px);
  grid-template-rows: repeat(var(--grid-size), 30px);
}

General recommendation do not write long functions, they can always be broken down into components.

function resetBoard() { 40+ rows } => function resetBoard() { < 20 rows }

If you have to kill the timer this way, you may need to use something else or think about it so that there are no such overlays. Clearing the timer is fine, but this particular case looks like a hack again.

If you initialize the application once, it solves a lot of these things. It will be easier if the timer will run once but work depending on the conditions.

clearInterval(timer);                 =>   ??? setTimeout(setTime, 1000) 
timer = setInterval(setTime, 1000)

Conditions can be given names - in addition to being convenient, it is also easier to use them at the same time for one solution.

if (condition1) return;   =>  if (condition1 || condition2) return;                             
if (condition2) return;

Before writing your own implementation, find out if the language can do it.

function pad(val) {                   => `${0}`.padStart(2, "0")  // 00 
  var valString = val + "";           => (val + "") why ?
  if (valString.length < 2) {
    return "0" + valString;
  } else {
    return valString;
  }
}

If to get global variables that in one place.

const gameStatus = {
  fieldSize: 4, //(N)
  gameOver: false,
  totalSeconds: 0
}  

Bonus: Try to copy and run in console (automatic play):

[...document.querySelectorAll('.cell:not(.mine)')].forEach(e => e.click())

Don't use app logic elements in DOM. It's not safe.

My best regards!