# JS

* Follow language specific conventions
* js: `snakeCase` for variables `UPPER_CASE` for constants
* css/html: `kebab-case` for id, class names

* Code duplication
* Binding HTML elements in code
* Reset points and times, displaying points and values
* could be one function to update all values, which can be called on initial setup, user clicks and every second
* other function to reset the values

* Incoherent scoping
* The whole script is in `window.onload` function, every data and sub-functions are defined in the function scope.The sub-functions are accessing the data either from the parent scope or either getting as an argument.
* Accessing them from parent scope can be misleading while reading the function, because it is not always clear where the data is coming from.
* Besides the readiblity, it makes the refactoring harder, can be hard to move the function to another module, it is dependent on the outer scope.
* Also the clean code rule says it is a smell having more then 2/3 arguments for a function. If the data is not coming from arguments it hides this smell, suggesting it is correct while in reality it is too complex.

* `start_timer`, `start_game`, `draw_table` functions are too long
* `draw_table` iterates over the html elements 3 times, could be set everything in one step
* also if code duplications are moved to separate functions it would reduce the function length

* Better naming and redundant comments
* Always prefer good namings over explaining in comments

  // Starts the timer.  <-- redundant comment, the code should be self-explaining
  function start_timer() {

* Stick with the already existing namings (e.g. what is `original_color_properties`)

* The `game_welcome_box` is not defined (browser will create it on the `window` global object)
* no variable definition should be without keyword or with `var`
* always prefer `const` over `let`

* Level dataset
* Contants `round_counts_of_levels` and `table_sizes_by_levels` are tightly coupled, it could be represented in the data format also.
* Also if you want to add another level or change one of the current ones, it can pain to keep the two array in sync

  const levels = [{ size: 2, rounds: 4 }, { size: 3, rounds: 4 }, ...]

* The current level (`actual_match`) variable is confusing being an array for storing the current indexes
* could be also an object

    const actualLevel = {  // naming "level" instead of "match" could help because "levels" is already used for defining the games 
      currentRound: 1,
      size: 3
    };

* Avoid string comparison for internal logic. If the string needs to be changed there is no intellisense for the value, can be easily mistyped.
* the set_point function could be easily separated, then there is no need having an if statement in the function

  // Increases or decreases the points.
  function set_point(direction) {
    if (direction == 'increase') {
      point++;
    } else if (direction == 'decrease' && point != 0) {
      point--;
    }
    point_value.textContent = `${point}`;
  }
  ------
  function increasePoints() {
    point++;
    pointValue.textContent = `${point}`; // or having an updateValues() function
  }

  function decreasePoints() {
    if (point <= 0) {
      return;
    }

    point--;
    pointValue.textContent = `${point}`; // or having an updateValues() function
  }

* obj['key'] < obj.key
* It is considered not safe to use bracket notation for objects.
* https://github.com/nodesecurity/eslint-plugin-security/blob/HEAD/docs/the-dangers-of-square-bracket-notation.md

* You should consider using webpack
* It will transpile your javascript code and can find errors in the code during build and not during running time. Also can provide features which are not in browsers default.
* https://stackoverflow.com/a/60020448

* zoom = 75%
* it should be set in css

* (+1) The site is not accessible, however probably disabled people wouldn't play with a game where you have to see the difference on the screen :man-shrugging-emoji:
* img tag don't have "alt" attribute
* missing hover, active, disabled css states
* always use <button> tag if possible (browser supports tab navigation by default)
* If you are interested you can find some points which are required for it https://blog.hubspot.com/website/web-accessibility

# CSS

* Background rules
* common properties can be moved to a common class:

  .background-image {}
  #background-left {}
  #background-righ {}

* Color classes are unnecessary coupled to h1

  h1-dark -> dark
  h1-light -> light

* Use CSS variables for colors and constants
https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties

* note (my personal opinion)
* I like the utility first approach when it comes to CSS. The Tailwind CSS framework works this way, the concept is explained at their docs. I would not import the framework for this project, but would consider write helper css classes (e.g for text colors or for button designs ...)
* https://tailwindcss.com/docs/utility-first