4
\$\begingroup\$

I've been learning front-end web design, and I'm looking to start diving into the back end soon. Before I do that, I want to make sure I have my HTML/CSS/JS skills down pretty solid. Here's a cricket (darts) scoreboard that I made, and I'd love suggestions on how I could improve the code and/or the design. 

var images = [
  'url("none")',
  'url("https://i.imgur.com/25qbHoC.png")',
  'url("https://i.imgur.com/9pcneSb.png")',
  'url("https://i.imgur.com/Vfxu8ap.png")'
]

$('.xfield').click(function() {
  var x = $(this).data("counter") || 1;
  $(this).css('background-image', images[x%4]);
  $(this).data('counter', x+1);
});

$('#continue').click(function() {
  var name1 = $('#name1').val();
  var name2 = $('#name2').val();

  $('#name1field').text(name1);
  $('#name2field').text(name2);

  $('.names').fadeOut();
  $('.scoreboard').fadeIn();
});
* {
  margin: 0;
  padding: 0;
  font-family: 'Karma', sans-serif;
}


.main-table-header {
  text-align: center;
}

td, th {
  border: black 1px solid;
  width: 200px;
  height: 100px;
  background-size: 100px 100px;
  background-position: center;
  background-repeat: no-repeat;
}

.scoreboard {
  text-align: center;
  font-size: 2em;
  margin: 1em auto;
  height: 25em;
  display: none;
}

.number {
  font-size: 1.3em;
  margin: 0;
}

.namesField {
  margin: 20px auto;
  width: 100%;
  text-align: center;
}

.names {
  position: absolute;
  text-align: center;
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  height: 100%;
  width: 100%;
  margin-top: -7em;
}

.names input {
  height: 2em;
  margin: 20px;
  font-size: 1.5em;
  border: none;
  text-align: center;
  border-bottom: 2px solid black;
  user-select: none;
  outline: none;
}

.names h1 {
  font-size: 4em;
  text-align: center;
  margin-top: 15px;
}

.names p {
  font-size: 1.5em;
  text-align: center;
  margin-top: 20px;
}

.begin-button {
  position: relative;
  border: 2px solid black;
  height: 2.5em;
  width: 5em;
  user-select: none;
  background: none;
  outline: none;
  cursor: pointer;
  font-size: 1.5em;
  border-radius: 4px;
  letter-spacing: 2px;
  transition: color .3s;
}

.begin-button:hover {
  color: white;
}

.begin-button::after {
  content: "";
  position: absolute;
  background-color: black;
  top: 0; left: 0; right: 100%; bottom: 0;
  z-index: -1;
  transition: right .3s;
}

.begin-button:hover::after {
  right: 0;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Cricket Scoreboard</title>
  <link href="https://fonts.googleapis.com/css?family=Karma" rel="stylesheet">
  <link rel = "stylesheet" type = "text/css" href = "style.css"/>
</head>
<body>
  <table class = "scoreboard" cellspacing = "0" cellpadding = "0">
    <tr>
      <th contenteditable="true" id = "name1field">Name #1</th>
      <th>
        <h2>SCORE</h2>
      </th>
      <th contenteditable="true" id = "name2field">Name #2</th>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">20</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">19</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">18</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">17</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">16</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">15</td>
      <td class = "xfield"></td>
    </tr>
    <tr>
      <td class = "xfield"></td>
      <td class = "number">Bulls Eye</td>
      <td class = "xfield"></td>
    </tr>
  </table>

  <div class = "names">

    <h1> Cricket Scoreboard </h1>
    <p> Enter your names: </p>

    <div class="namesField">
      <input type = "text" placeholder = "Name #1" id = "name1"/>
      <input type = "text" placeholder = "Name #2" id = "name2"/>
    </div>

    <button id = "continue" class = "begin-button">BEGIN</button>

  </div>

  <script
  src="http://code.jquery.com/jquery-3.3.1.min.js"
  integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8="
  crossorigin="anonymous"></script>
  <script src="script.js"></script>
</body>
</html>

Note: I'm aware that this is not really responsive at all, and that there should be @media query breaks. That being said, I didn't do that for this project because I only use it on the computer next to my dart board.

\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Feedback

The code looks pretty good. The styles are laid out pretty well, and the JS is pretty succinct. I like the use of data attributes and modulus division for the counter values.

As far as the design goes, it looks good. Possible improvements would include adding total scores and allowing one user to add scores on a single number until their opponent closes it.

Suggestions

CSS

The style text-align: center appears to be used for a majority of selectors, and none use any other alignment, so move that up to *.

Javascript

Update class instead of style directly

Instead of storing the images in Javascript, put them in CSS classes. Then update the class (using jQuery's addClass() and removeClass() methods) based on the counter value. That way, the CSS can be updated without having to update the JavaScript code.

N.B. _Initially I was thinking an advantage here would be that it would be less expensive but most browsers will still have a reflow when changing the element's class attribute as often as the original code. Read more about reflow here.

Use an event delegate

Instead of adding a click handler to each element with class xfield and the button with id continue, use event delegation to handle all clicks with one function. Then check to see if the event target's id matches the that of the continue button, otherwise if it has the class xfield.

For more suggetions about Javascript, see this post - I know it is a few years old and bases jQuery but has some good information.

Rewrite

See these suggestions implemented in the rewrite below

$(function() { //DOM ready callback - limits scope, like an IIFE
  $(document).click(function(event) {
    var target = $(event.target);
    if (target.attr('id') == 'continue') {
      var name1 = $('#name1').val();
      var name2 = $('#name2').val();

      $('#name1field').text(name1);
      $('#name2field').text(name2);

      $('.names').fadeOut();
      $('.scoreboard').fadeIn();
    }
    else if (target.hasClass('xfield')) {
      var x = target.data("counter") || 0;
      target.removeClass('count' + (x % 4)).addClass('count' + ((x + 1) % 4));
      target.data('counter', x + 1);
    }
  });
});
* {
  margin: 0;
  padding: 0;
  font-family: 'Karma', sans-serif;
  text-align: center;
}

td,
th {
  border: black 1px solid;
  width: 200px;
  height: 100px;
  background-size: 100px 100px;
  background-position: center;
  background-repeat: no-repeat;
}

.scoreboard {
  font-size: 2em;
  margin: 1em auto;
  height: 25em;
  display: none;
}

.number {
  font-size: 1.3em;
  margin: 0;
}

.namesField {
  margin: 20px auto;
  width: 100%;
}

.names {
  position: absolute;
  display: flex;
  flex-direction: column;
  align-items: center;
  justify-content: center;
  height: 100%;
  width: 100%;
  margin-top: -7em;
}

.names input {
  height: 2em;
  margin: 20px;
  font-size: 1.5em;
  border: none;
  border-bottom: 2px solid black;
  user-select: none;
  outline: none;
}

.names h1 {
  font-size: 4em;
  margin-top: 15px;
}

.names p {
  font-size: 1.5em;
  margin-top: 20px;
}

.begin-button {
  position: relative;
  border: 2px solid black;
  height: 2.5em;
  width: 5em;
  user-select: none;
  background: none;
  outline: none;
  cursor: pointer;
  font-size: 1.5em;
  border-radius: 4px;
  letter-spacing: 2px;
  transition: color .3s;
}

.begin-button:hover {
  color: white;
}

.begin-button::after {
  content: "";
  position: absolute;
  background-color: black;
  top: 0;
  left: 0;
  right: 100%;
  bottom: 0;
  z-index: -1;
  transition: right .3s;
}

.begin-button:hover::after {
  right: 0;
}

.count0 {
  background-image: url("none");
}

.count1 {
  background-image: url("https://i.imgur.com/25qbHoC.png");
}

.count2 {
  background-image: url("https://i.imgur.com/9pcneSb.png");
}

.count3 {
  background-image: url("https://i.imgur.com/Vfxu8ap.png");
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Cricket Scoreboard</title>
  <link href="https://fonts.googleapis.com/css?family=Karma" rel="stylesheet">
  <link rel="stylesheet" type="text/css" href="style.css" />
</head>

<body>
  <table class="scoreboard" cellspacing="0" cellpadding="0">
    <tr>
      <th contenteditable="true" id="name1field">Name #1</th>
      <th>
        <h2>SCORE</h2>
      </th>
      <th contenteditable="true" id="name2field">Name #2</th>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">20</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">19</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">18</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">17</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">16</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">15</td>
      <td class="xfield"></td>
    </tr>
    <tr>
      <td class="xfield"></td>
      <td class="number">Bulls Eye</td>
      <td class="xfield"></td>
    </tr>
  </table>

  <div class="names">

    <h1> Cricket Scoreboard </h1>
    <p> Enter your names: </p>

    <div class="namesField">
      <input type="text" placeholder="Name #1" id="name1" />
      <input type="text" placeholder="Name #2" id="name2" />
    </div>

    <button id="continue" class="begin-button">BEGIN</button>

  </div>

  <script src="http://code.jquery.com/jquery-3.3.1.min.js" integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>
  <script src="script.js"></script>
</body>

</html>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.