Split the Bill and calculate the Tip

Question

I have created a Bill Splitter page that asks for the total bill amount, tip percentage, and the number of people. Then it displays the tip and the total amount (including the tip) each person has to pay out of that bill. I have used simple vanilla JavaScript to complete this project. Being a beginner in front-end development, I want to adapt to all the best practices. I am looking forward to all your feedback.

My Website should have the following features:

• Users should be able to view the optimal layout for the app depending on their device's screen size
• See hover states for all interactive elements on the page
• Calculate the correct tip and total cost of the bill per person

let bill_amount = document.getElementById("amount");
let humans = document.getElementById("people");
let tip_buttons = Array.from(document.getElementsByClassName("tip"));
let reset_button = document.getElementById("reset");
let custom_tip_button = document.getElementById("custom");
let tip_percentage, tip_person, total_bill_person;
let bill_amountFilled = false;
let humansFilled = false;
let buttonClicked = false;

tip_buttons.forEach(function(button) {
  button.addEventListener("click", function(event) {
    tip_buttons.forEach(function(otherbtns) {
      otherbtns.classList.remove("btn-selected");
    })
    button.classList.add("btn-selected");
    tip_percentage = parseFloat(button.innerHTML);
    buttonClicked = true;
    calculateTip();
  });
});

custom_tip_button.addEventListener("input", function(event) {
  tip_buttons.forEach(function(button) {
    button.classList.remove("btn-selected");
  })
  tip_percentage = parseFloat(event.target.value);
  buttonClicked = true;
  calculateTip();
});

function calculateTip() {
  if (bill_amountFilled && humansFilled && buttonClicked) {
    tip_person = (parseFloat(bill_amount.value) * (0.01 * parseFloat(tip_percentage))) /
      parseFloat(humans.value);
    total_bill_person = parseFloat(bill_amount.value) / parseFloat(humans.value) +
      tip_person;
    document.getElementById("tipPerPerson").value = tip_person.toFixed(2);
    document.getElementById("totalPerPerson").value = total_bill_person.toFixed(2);
  }
}

bill_amount.addEventListener("input", function(event) {
  if (bill_amount.value.trim() !== "") {
    bill_amountFilled = true;
    calculateTip();
  } else {
    bill_amountFilled = false;
  }
});

humans.addEventListener("input", function(event) {
  if (humans.value.trim() !== "") {
    humansFilled = true;
    calculateTip();
  } else {
    humansFilled = false;
  }
});

reset_button.addEventListener("click", function() {
  location.reload();
});

document.addEventListener("click", function(event) {
  event.preventDefault();
});

body {
  background-color: hsl(185, 41%, 84%);
  font-family: "Space Mono";
}

h4 {
  color: hsl(183, 100%, 15%);
  letter-spacing: 15px;
  margin: 5% 50% 0% 45%;
}

h5 {
  font-size: small;
  color: #fff;
  display: inline-block;
}

h6 {
  margin: 3%;
  color: hsl(185, 22%, 45%);
}

.line2 {
  margin-top: 0%;
}

.calculator {
  display: flex;
  margin: 4% 15% 0% 20%;
  background-color: #fff;
  border-radius: 2.5%;
  z-index: 1;
  width: 60%;
  padding: 0.9%;
}

.left-box {
  float: left;
  background-color: #fff;
  width: 50%;
  padding: 2%;
  box-sizing: border-box;
}

.left-input {
  background-color: hsl(189, 41%, 97%);
  border: none;
  margin-bottom: 5px;
  text-align: right;
  color: hsl(183, 100%, 15%);
  width: 80%;
}

.left-input:focus {
  outline: none !important;
  border: 1px solid aquamarine;
  box-shadow: 0 0 10px #719ECE;
}

#custom {
  display: inline-block;
  background-color: hsl(189, 41%, 97%);
  color: hsl(183, 100%, 15%);
  width: 25%;
  height: 11%;
  text-align: center;
  border-radius: 5%;
  cursor: pointer;
}

.btn {
  width: 80px;
  margin-right: 20px;
  margin-bottom: 10px;
  background-color: hsl(183, 100%, 15%);
  border: none;
}

.btn-selected {
  background-color: hsl(166, 61%, 39%);
}

.custom:focus {
  background-color: hsl(161, 28%, 62%);
  color: #fff;
}

.right-box {
  float: right;
  width: 50%;
  box-sizing: border-box;
  background-color: hsl(183, 100%, 15%);
  padding: 2.5%;
  border-radius: 5%;
}

.right-input {
  width: 50%;
  background-color: hsl(183, 100%, 15%);
  border: none;
  outline: none;
  color: hsl(161, 42%, 58%);
  text-align: right;
  height: 30px;
  font-size: xx-large;
}

.btn-lg {
  background-color: hsl(161, 42%, 58%);
  width: 100%;
  margin-top: 100px;
}

@media only screen and (max-width: 600px) {
  h4 {
    margin: 5% 30% 5% 40%;
  }
  .calculator {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
  .left-box {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
  .right-box {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
}

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <!-- displays 
      site properly based on user's device -->
  <link rel="icon" type="image/png" sizes="32x32" href="./images/favicon-32x32.png">
  <title>Tip Calculator App</title>
  <!--Font Awesome Links-->
  <script src="https://kit.fontawesome.com/fd6cc398e6.js" crossorigin="anonymous">
  </script>
  <!--Google Fonts-->
  <link rel="preconnect" href="https://fonts.googleapis.com">
  <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
  <link href="https://fonts.googleapis.com/css2? 
    family=Space+Mono:ital,wght@0,700;1,700&display=swap" rel="stylesheet">
  <!--Bootstrap Links-->
  <link href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384- 
      rbsA2VBKQhggwzxH7pPCaAqO46MgnOM80zW1RWuH61DGLwZJEdK2Kadq2F9CUG65" crossorigin="anonymous">
</head>

<body>

  <div class="top-heading">
    <h4 class="line1">SPLI</h4>
    <h4 class="line2">TTER</h4>
  </div>

  <main>
    <div class="calculator">

      <div class="left-box">
        <h6>Bill</h6>
        <span class="text-addon"><i class="fa-solid fa-dollar-sign"></i></span>
        <input type="text" class="left-input" id="amount">
        <h6>Select Tip %</h6>
        <button type="button" class="btn btn-primary tip">5%</button>
        <button type="button" class="btn btn-primary tip">10%</button>
        <button type="button" class="btn btn-primary tip">15%</button>
        <button type="button" class="btn btn-primary tip">25%</button>
        <button type="button" class="btn btn-primary tip">50%</button>
        <input type="text" class="left-input" id="custom" placeholder="Custom">
        <h6>Number of People</h6>
        <span class="people-addon"><i class="fa-solid fa-person"></i></span>
        <input type="text" class="left-input" id="people">
      </div>

      <div class="right-box">
        <h5>Tip Amount / person</h5>
        <input type="text" id="tipPerPerson" class="right-input" readonly placeholder="$0.00"><br><br><br>
        <h5>Total / person</h5>
        <input type="text" id="totalPerPerson" class="right-input" style="margin-left: 
             40px;" readonly placeholder="$0.00"><br><br><br>
        <button type="button" class="btn btn-primary btn-lg" id="reset">Reset</button>
      </div>

    </div>
  </main>
</body>

Live Site URL

Problem Statement

Solution Repository

Answer 1

1 HTML

Your markup is completely valid according to W3C and WHATWG markup rules. The Markup validation return no flags. However, I have to flag, that the code you submitted for review differs from the code you used on your web server. If I would purely review the code you submitted, I would need to flag the missing <!DOCTYPE HTML>, <html> tags, and the lang attribute.

1.1 Conventions

1. I see no usage of conventions and only a few comments within the <head> element. Overall your code is freestyle and harder than necessary to read.
2. The naming of your IDs and classes for large parts is not self-explaining. left-input is a poor name. Neither from the naming nor from the visual example I would understand what the classes are for.
3. You using Bootstap-5 but your class names indicate that for most parts you are not using it and rather rely on a long custom CSS file. While it is acceptable to mix both you rather should decide if you really need Bootstrap and if, then you should use it to the full extent and cut down your custom CSS to that part that can't be solved with Bootstrap.
4. As you already use Bootstrap and custom CSS you should stick to them. You should never mix them with inline-style!

1.2 Accessibility & Semantics

Your code is for most parts inaccessible. With the exception of <main> I see no specific semantic tag or attribute.

1. <div class="top-heading> would be a <header>
2. In your header you decided to split your headline into 2 lines by using 2 separate headline elements. You rather should split it to the next line with either <br> or <wbr>. As of now, a screen reader would recognize it as 2 independent headlines that will be pronounced incorrectly: headline: S P L I /pause/ headline: Double T E R
3. Your Select Tip % section could be improved. Semantically, you should wrap them in a fieldset and use Select Tip % as the legend. Also, you should only be able to make one selection. As such the usage of radio buttons would have been more appropriate than the usage of different buttons and inputs. It also would improve the accessibility as then the percentages would be directly connected to the legend.
4. Inputs should always have a label connected to them. Even if you hide the label within CSS, you should have them for screen readers.
5. You are using incorrect input types. Your inputs are supposed to be type=number to only allow numbers. For people, you need integers as you can't have 6.5 people. For money amounts, you want inputs with a max of 2 decimal places.
6. Neither Bill (should be Bill amount) nor Number of People or the other h5 is a headline. It is a label for the input.
7. You used inputs and set them to readonly while those elements are actually <output>s.

1.3 Head Element

1. IMHO your <head> is overfilled. You using a lot of dependencies for a very small web application.
2. In the code you submitted you miss the custom CSS link
3. In the code you submitted you miss the custom JS link while on the web server, it is at the end of the body. You can't make use of asynchronous loading and overall it would have been better to include it also within your head element.
4. <meta name="viewport" content="width=device-width, initial-scale=1.0"> should only be used if you actually know what it does and how you counter the downsides of it. It is not a tool to properly display the site on every device. It is a tool that removed DPR (Device Pixel Ratio) features to make it easier for developers as they don't need actual devices to test for the cost of poorer UX. Especially in your case, it adds no responsivness but poor UX on high-resolution mobile devices.

Also if you are intending to operate within the EU, you would also be required to provide a disclaimer for your Google API usage.

1.4 Body Element

Within semantics and accessibility, I already said a lot about your markup and element decisions.

1. What I still want to add are your buttons. As said before, you should not use buttons here in the first place. If you do, then don't get the value by reading out their text node. Add a data attribute instead and read out that.
2. your container <div class="calculator"> seems unnecessary to me. Why not use the <main> element directly?
3. <br> is a linebreak to break a text instantly to the next line. It is not a tool to create visual spacing! Visual spacings have to be included through CSS.

2 CSS

Your CSS is unordered and repetitive. Your design choices are well thought through but missing accessibility. Overall they could be improved on your coding methods.

2.1 Accessibility

1. The inputs have a slightly different background color and especially for colo-blind will not be recognized as such. a stronger contrast would have been better.
2. the buttons and the input for the tip amount have different sizes. This is caused by their nature and does not look smooth. You should decide on a uniform size and form to look the same with exception of the background color.

2.2 Responsivness

1. Your web application is not responsive at all or at least has never been correctly tested. While I think you read about responsive web design you properly misunderstood the rules or pieces of advice. While you should use relative units, % is not a unit that should be used for everything. em and rem would have been more appropriate in most cases and would have caused fewer issues.
2. A good responsive design starts with mobile first, then tablets, then laptops, then desktops.

2.3 Structure

1. While it is correct to start with the general tags, you should continue then with classes and then with ids and those in the order of their first appearance. Alternatively, you should block the classes, IDs, and tags according to their first appearance to have at least some kind of logical order.
2. A lot of your code is repetitive. select multiple classes, tags, or IDs and apply all the styles to them that are the same for all those elements.

As example:

before:

@media only screen and (max-width: 600px) {
  h4 {
    margin: 5% 30% 5% 40%;
  }
  .calculator {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
  .left-box {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
  .right-box {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
}

after:

@media only screen and (max-width: 600px) {
  h4 {
    margin: 5% 30% 5% 40%;
  }
  .calculator,
  .left-box,
  .right-box {
    display: inline-block;
    width: 100%;
    margin-left: 0%;
  }
}

3. Make usage of the :root selector and then use CSS variables. Especially on things like colors, margins/paddings, and gaps you could include your CSS variable which would make maintenance a lot easier.
4. Use properties only within their scope. float is for floating an element within a text-block. Something like pictures in a newspaper. float is not a property to align items next to each other!

3 JS

Overall your JS is very basic and still acceptable to read while it still could be improved.

3.1 Conventions

1. Make use of constants. Especially if you reference elements then it is to be expected that the elements do not change. As such a const would be appropriate.
2. declare const at the very top of the JS and use capital names to differentiate them from variables.
3. be consequent with the mentioned points from above. At the end you still used document.element.value = ... which is harder to read while you could have declared the element itself as const at the top.
4. Make parameters visual by adding underscores to it __param. This makes the code easier to read and as such easier to maintain by others.
5. choose names that are self-explaining even if they are longer
6. don't spare with comments. While your code should be self-explaining while reading it, carefully placed comments with short descriptions will significantly improve readability.
7. Do not use innerHTML as they pose a security risk (XSS) and are slow (require the reparsing of the DOM).

3.2 Input validation

1. Inputs should always be validated. Validated for their correctness and plausibility. As an example you can insert twenty as the value of the Bill amount. twenty however, is a string and will return a NaN (Not a Number) error. Same I could insert a 20e10 which is a valid number. Or I could include a negative number which also is a valid number. So you should always check if the input is plausible. If it is not, throw an error message at the user.

3.3 Logic

1. You have a major logic error. If I have to divide a 100$ bill through 3 persons then every person would need to pay 33.33$. By division rules, it would be correct after rounding. However, if you multiply 33.33$ by 3 then you would be 1 cent short. You always need to round up here as otherwise, you might be short because of the rounding. It is legal to pay a few cents more than needed but it is illegal to pay less than the bill.
2. If you select no tip which is completely plausible then the program will not start.
3. Only if bill amount, tip, and amount of persons is filled out correctly, the calculation will start. If not, the program will simply do nothing. You should always throw an error message at the user and explain to him why the program does not start.

3.4 Coding Style

1. I do not understand why you used Array.from(document.getElementsByClassName('tip')) which is unnecessary and code-wise ugly. getElementsByClassName would return an HTML Collection Object while querySelectorAll would return a Node List. Both are array-like objects that can be iterated over without converting them specifically to an array.
2. location.reload() is to reload a document. It should never be used to reset inputs and calculations to the original state. It will spam the history of a user for no reason.

4 Rating

4.1 Hobby Programmer

This web application shows sufficient planning. In general, it seems to me that a hobby programmer knew what he did. I would assume that a hobby programmer codes to the best of his abilities but was missing heavily on cleanness, simplicity, and necessity. A good approach to improve would be to overthink the resources of learning and by getting regular supervision from a senior developer.

Grade: B- - C+

4.2 Developer Trainee

For a Trainee the planning phase would still be sufficient. However, a trainee should have already learned about the economic effect of programming. The code misses not only cleanness, simplicity, and necessity but also up-to-date coding. Missing conventions on top will have a harder impact on the economic value which even for a trainee is not acceptable. A trainee should know better and know that coding is the smallest part of program development.

Grade: D

4.3 Junior Developer

A Junior Developer should have already rock solid foundations. Especially the methods implemented as well as the technologies are not to be expected by a Junior Developer. Missing input validation is not excusable for a Junior Developer.

Grade E - E-

Answer 2

Regarding the HTML

• the heading could use the word break opportunity element so that wouldn't need to use two elements (<h4>SPLI<wbr>TTER</h4>)
• style definitions should be inside CSS, not inline
• never use HTML tags for styling (<br><br><br> is about the same as a CSS margin-bottom: 3em style definition for preceding element)
• user input controls should generally be inside a form, allowing the use of HTMLFormElement API
• using <input type="number"> would be more semantically correct (also allowing to use its method valueAsNumber to skip manual parsing)
• there exists a <input type="reset"> to return the form to its initial state
• there exists a <label> element to give textual description to inputs (I would suggest following structure: <label><span>desc</span><input ... /></label> so you can target it with CSS: label span { ... })
• could the tip percentage selection work better as a range slider? (<input id="tip" type="range" min="5" max="50" step="5" />). I'm not living in a tipping culture so I'm not sure if that would make sense.
• I'd suggest to order element attributes by specifity, so <tag id="..." class="..." type="..." etc="..." />

Regarding the Javascript

• you can listen the input events on the form itself, and it will propagate down to contained controls, simplifying the listener setup.
• use let for variables which will change, and const for those that won't
• using previously mentioned HTMLFormElement API would simplify much of the JS
• what is the variable named location?