eCommerce Mockup App in JS

Question

I have completed this mockup eCommerce app using Javascript. This version can add products into a shopping cart and automatically calculate the order summary in the shopping cart. It can also delete one item and all items in the shopping cart.

Checkout and login functions are not included.

Appreciate all comments and code review especially on Javascript and coding style and best practices. Thanks a lot in advance.

---

// Initialization of data and page variables - start.
// For actual system, get data from database via API call either in JSON format.
var productList = [
  { id: 101, product: "Logitech Mouse", unitprice: 45.0 },
  { id: 102, product: "Logitech Keyboard", unitprice: 50.0 },
  { id: 103, product: "HP Mouse", unitprice: 35.0 }
];

var cart = [];
cart.length = 0;
$shoppingCartContainer = document.getElementById("shoppingCartContainer");
$clearAll = document.getElementById("clearAll");
$shoppingCart = document.getElementById("shoppingCart");
$totalCartItems = document.getElementById("totalCartItems");
$summary = document.getElementById("summary");
// Initialization of data and page variables - end.

// Functions - start -------------------------------------
const createCartHTMLElements = object => {
  // Check type
  if (typeof object !== "object") return false;

  // Start our HTML
  var html = "<table><tbody>";

  // Loop through members of the object
  debugger;
  object.forEach(function(item) {
    html += `<tr class=productData><td class="productName">${item.product}</td>\
                <td class="productPrice">${item.unitprice.toFixed(2)}</td>\
                <td class="quantityProduct">\
                <button class="plus-btn" data-id="${item.id}">+</button>\
                <label class="quantity">${item.quantity}</label>\
                <button class="minus-btn" data-id="${item.id}">-</button>\
                </td>\
                <td class="total">${item.total.toFixed(2)}</td>\
                <td class="deleteProduct"><i class="fa fa-remove del" data-id="${
                  item.id
                }"></i></td>\
                </tr>`;
  });

  // Finish the table:
  html += "</tbody></table>";

  // Return the table
  return html;
};

const updateQuantity = (operation, productId, tr) => {
  // Update the quantity in UI
  let $quantity = tr.find(".quantity");
  let n = $quantity.html();
  let i;
  switch (operation) {
    case "plus":
      i = parseInt(n) + 1;
      break;
    case "minus":
      i = parseInt(n) - 1;
      if (i < 0) i = 0; // prevent negative quantity

      if (i == 0) {
        // Duplicate code with delete function
        cart = cart.filter(function(el) {
          return el.id != productId;
        });

        if (cart.length === 0) {
          $shoppingCart.innerHTML = "";
          debugger;
          $clearAll.style.display = "none";
          $summary.style.display = "none";
        }
        updateCartCount();
        updateOrderSummary();
        
        tr.closest("tr").remove();            
      }

      break;
  }

  $quantity.html(i);

  // Update the total price in UI
  let $price = tr.find(".productPrice");
  let price = parseFloat($price.html());
  let $total = tr.find(".total");
  let total = i * price;
  $total.html(total.toFixed(2));

  // Update the quantity and total in list object
  // Find index of specific object using findIndex method.
  let objIndex = cart.findIndex(obj => obj.id == productId);

  if (objIndex >= 0) {
    // Update object's name property.
    cart[objIndex].quantity = i;
    cart[objIndex].total = total;

    updateOrderSummary();
  }
};

const populateProducts = arrOfObjects => {
  // Start our HTML
  var html = "";

  // Loop through members of the object
  arrOfObjects.forEach(function(item) {
    html += `<div class="column"><div class="card">\
                <h2>${item.product}</h2>
                <p class="price">RM ${item.unitprice.toFixed(2)}</p>
                <p><button class=AddToCart data-id="${
                  item.id
                }">Add to Cart</button></p>\
                </div></div>`;
  });

  return html;
};

const updateOrderSummary = () => {
  document.getElementById("totalItem").innerHTML = cart.length + " item";

  const subTotal = cart
    .reduce(function(acc, obj) {
      return acc + obj.total;
    }, 0)
    .toFixed(2);

  const shippingFee = 10;

  document.getElementById("subTotal").innerHTML = subTotal;
  document.getElementById("shippingFee").innerHTML = shippingFee.toFixed(2);
  document.getElementById("total").innerHTML = (
    parseInt(subTotal) + shippingFee
  ).toFixed(2);
};

const updateCartCount = () => {
  $totalCartItems.innerHTML = cart.length;
};
// Functions - End  -------------------------------------

// Event listener - start -------------------------------------
const CreateAddToCartEventListener = () => {
  var addToCart = document.getElementsByClassName("AddToCart");
  updateCartCount();
  Array.prototype.forEach.call(addToCart, function(element) {
    element.addEventListener("click", function() {
      debugger;
      // Filter the selected "AddToCart" product from the ProductList list object.
      // And push the selected single product into shopping cart list object.
      productList.filter(prod => {
        if (prod.id == element.dataset.id) {
          debugger;

          // Update the quantity in list object
          // Find index of specific object using findIndex method.
          let objIndex = cart.findIndex(
            obj => obj.id == parseInt(element.dataset.id)
          );
          if (objIndex >= 0) {
            // Old item found
            cart[objIndex].quantity = cart[objIndex].quantity + 1;
            cart[objIndex].total = cart[objIndex].quantity * prod.unitprice;
          } else {
            // For new item
            prod.quantity = 1;
            prod.total = prod.unitprice;
            cart.push(prod);
          }

          $shoppingCart.innerHTML = createCartHTMLElements(cart);

          CreateDeleteEventListener();
          $totalCartItems.style.display = "block";
          $clearAll.style.display = "block";
          $summary.style.display = "block";
          updateCartCount();
          updateOrderSummary();
          return;
        }
      });
    });
  });
};

const CreateDeleteEventListener = () => {
  var del = document.getElementsByClassName("del");

  Array.prototype.forEach.call(del, function(element) {
    element.addEventListener("click", function() {
      // Duplicate code with minus quantity function
      // When quantity is zero, it will delete that item
      cart = cart.filter(function(el) {
        return el.id != element.dataset.id;
      });

      if (cart.length === 0) {
        $shoppingCart.innerHTML = "";
        debugger;
        $clearAll.style.display = "none";
        $summary.style.display = "none";
      }
      updateCartCount();
      updateOrderSummary();

      element.closest("tr").remove();
    });
  });
};

$(document.body).on("click", ".plus-btn", function() {
  let productId = $(this).attr("data-id");
  let $tr = $(this).closest("tr");
  updateQuantity("plus", productId, $tr);
});

$(document.body).on("click", ".minus-btn", function() {
  let productId = $(this).attr("data-id");
  let $tr = $(this).closest("tr");
  updateQuantity("minus", productId, $tr);
});

$clearAll.addEventListener("click", function() {
  $shoppingCart.innerHTML = "";
  cart.length = 0;
  $clearAll.style.display = "none";
  $summary.style.display = "none";
  updateOrderSummary();
  updateCartCount();
});

document.getElementById("cartIcon").addEventListener("click", function() {
  if ($shoppingCartContainer.style.display === "none") {
    $shoppingCartContainer.style.display = "block";
  } else {
    $shoppingCartContainer.style.display = "none";
  }
});

window.addEventListener("load", function() {
  $shoppingCartContainer.style.display = "none";
  window.setTimeout(function() {}, 1000); // prevent flickering
  document.getElementById("productRow").innerHTML = populateProducts(
    productList
  );

  CreateAddToCartEventListener();
});

// Event listener - end -------------------------------------

.card {
  box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2);
  margin: auto;
  text-align: center;
  font-family: arial;
  width: 18em;
}

.price {
  color: grey;
  font-size: 1.5em;
}

.card button {
  border: none;
  outline: 0;
  padding: 12px;
  color: white;
  background-color: #000;
  text-align: center;
  cursor: pointer;
  width: 100%;
  font-size: 1em;
}

.card button:hover {
  opacity: 0.7;
}

.productContainer {
  margin: 15px;
}

.summaryDetails {
  width: 100px;
  text-align: right;
}

#productRow {
  display: flex;
  flex-wrap: wrap;
}

.column {
  flex: 1;
  margin-top: 12px;
}

@media (max-width: 1333px) {
  .column {
    flex-basis: 33.33%;
  }
}

@media (max-width: 1073px) {
  .column {
    flex-basis: 33.33%;
  }
}

@media (max-width: 815px) {
  .column {
    flex-basis: 50%;
  }
}

@media (max-width: 555px) {
  .column {
    flex-basis: 100%;
  }
}

#header {
  width: 100%;
  display: flex;
  justify-content: space-between;
  height: 50px;
}

#left,
#right {
  width: 30%;
  padding: 10px;
  padding-right: 30px;
}

#right {
  text-align: right;
  position: relative;
}

#main {
  max-width: 1000px;
  border: 1px solid black;
  margin: 0 auto;
  position: relative;
}

#shoppingCartContainer {
  width: 400px;
  background-color: lightyellow;
  border: 1px solid silver;
  margin-left: auto;
  position: absolute;
  top: 50px;
  right: 0;
  padding: 10px;
}

.fa-shopping-cart {
  font-size: 36px;
  color: blue;
  cursor: pointer;
}

.fa-remove {
  font-size: 24px;
  color: red;
  cursor: pointer;
}

#totalCartItems {
  margin: 0px 0px -2px -7px;
  color: red;
  padding: 2px;
  border-radius: 25px;
  width: 10px;
  margin-left: auto;
  font-size: 1em;
  position: absolute;
}

.plus-btn img {
  margin-top: 2px;
}

tr.productData,
td.productName,
td.productPrice,
td.quantityProduct,
td.price,
td.deleteProduct {
  padding: 10px;
}

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8" />
    <title></title>
    <link
      rel="stylesheet"
      href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css"
    />

    <link rel="stylesheet" href="styles.css" />
  </head>
  <body>
    <div id="main">
      <div id="header">
        <div id="left">Login as Joe Doe, <a href="">Logout</a></div>
        <div id="right">
          <i id="cartIcon" class="fa fa-shopping-cart"> </i>
          <span id="totalCartItems"></span>
        </div>
      </div>

      <hr />
      <div class="productContainer">
        <div id="productRow"></div>
      </div>

      <div id="shoppingCartContainer">
        <div id="shoppingCart"></div>
        <button id="clearAll">Clear Cart</button>

        <div id="summary">
          <hr />
          <h3>Order Summary</h3>
          <table>
            <tr>
              <td>Subtotal (<span id="totalItem"></span>)</td>
              <td class="summaryDetails"><span id="subTotal"></span></td>
            </tr>
            <tr>
              <td>Shipping Fee</td>
              <td class="summaryDetails"><span id="shippingFee"></span></td>
            </tr>
            <tr>
              <td>Total</td>
              <td class="summaryDetails"><span id="total"></span></td>
            </tr>
          </table>

          <button>Proceed to checkout</button>
        </div>
      </div>
    </div>
  </body>
</html>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="app.js"></script>

Answer 1

General feedback

Overall this code isn’t bad but could be simplified quite a bit. The biggest thing I noticed is that jQuery is used for a few things but could be used a lot more - e.g. selecting DOM elements and manipulating them, adding event handlers, etc. There are some who believe jQuery isn't needed (see sites like youmightnotneedjquery.com. Some of the advice below comes after I read this article Stop Writing Slow Javascript - I know it is from a few years ago but the concepts are still relevant. It also touches on the usage of jQuery in todays web.

If you really want to clean things up, run your code through a linter like eslint, jshint, etc. One of the first warnings will likely be to include "use strict" at the top of the JavaScript.

Targeted feedback

JS

• functions doing more than necessary? I see the function CreateAddToCartEventListener() calls updateCartCount(). Is that really necessary when adding an event listener?
• scoping and DOM ready wrap JS in a DOM ready callback (I.e. $(function() {}) - formerly .ready()) or else an IIFE to limit scope of variables

• utilize jQuery more As mentioned above, it can be used a lot more - e.g. instead of:

   $clearAll = document.getElementById("clearAll");
  

  Just select it with the jQuery function:

  $clearAll = $(“#clearAll”);
  

  Note that such a variable would then contain a jQuery collection and not a single DOM reference anymore, so with that one can call jQuery methods on the collection like .hide() and .show()

  $clearAll.style.display = "none";
  

  Can be changed to:

  $clearAll.hide();
  

  Also I see things like this:

  const CreateAddToCartEventListener = () => {
    var addToCart = document.getElementsByClassName("AddToCart");
    updateCartCount();
    Array.prototype.forEach.call(addToCart, function(element) {
      element.addEventListener("click", function() {
  

  Why not simplify the event handler registration to the jQuery syntax, like the code already does for other elements (e.g. with class name plus-btn, minus-btn). Instead of looping through all elements with the class name .AddToCard like the code does above, just use this:

  $(document.body).on("click", ".AddToCart", function() {
  

  or event simpler with the .click() shortcut method:

  $(document).click('.AddToCart', function() {
  

• global variables - any variable not declared with var (inside a function), let or const e.g. $shoppingCartContainer will be stored as a global variable - on window. In a small application that likely wouldn’t be an issue but it is a good habit to avoid those - especially when you get into a larger application and the same name is used to hold different data.

• variables starting with dollar symbol when using jQuery, that can be a sign that a variable represents a jQuery collection, so if $shoppingCartContainer is not a jQuery collection, somebody trying to extend your code might think it is a jQuery collection and use jQuery methods on it, which would lead to an error. See more in this post.
• default to using const for variables that don’t need to be reassigned - e.g. $quantity and then switch to let if it is determined that reassignment is necessary.

• setting length of new array to zero - when would the length of a new array be something other than zero?

  var cart = [];
  cart.length = 0;
  

• use spread operator - since ecmascript-6 features like arrow operators are used then instead of calling:

  Array.prototype.forEach.call(addToCart, function(element) {
  

  The collection can be put into an array with the spread operator:

  [...addToCart].forEach( function(element) {
  

• I see this line

  window.setTimeout(function() {}, 1000); // prevent flickering
  

  but I am not convinced that calling an empty function after 1 second will prevent flickering. How did you reach that conclusion?

  EDIT: It might be better to wait for the DOMContentLoaded instead of the load event before starting to interact with the DOM. And as I mentioned above, the jQuery DOM ready function is available for this.

  If the flickering you are seeing is the shopping cart container, then add style="display: none" to that HTML element instead of waiting for the DOM to be loaded to hide it. Then it can still be displayed dynamically via JS. If that was in the CSS then it would make displaying it more challenging.

HTML

• The <script> tags are outside the <html> tags. They are typically placed in the <head> or at the end of the <body> tag. For more information on this, refer to this post.

CSS

• multiple max-width media queries with same style I see the following rulesets:

  @media (max-width: 1333px) {
    .column {
      flex-basis: 33.33%;
    }
  }
  
  @media (max-width: 1073px) {
    .column {
      flex-basis: 33.33%;
    }
  }
  

  The second ruleset is overriding the first, but it doesn't appear to do anything different.

• excess margin style: #totalCartItems has margin with top, right, bottom and left, but then there is a separate style for margin-left: auto. Why not move that auto into the first margin style if it is needed?

Answer 2

I am by no means a javascript/html/css developer, just somethings I noticed, and some things I would recommend as a customer. I invite someone who is more experienced with javascript to review this code, as I rarely touch on it in this review.

Tags

Unless you're using XHTML, which it doesn't look like it, <link> does not need to be closed (/>), as it's a "self closing" tag. The same goes with the <hr> tag and the <meta> tag.

Scaling

I would recommend putting this piece of html in the header as well:

<meta name="viewport" content="width=device-width, initial-scale=1.0">

This is used to set styles and other website attributes to render the site on different devices. This tag decides how to scale the website. If I open the website on my computer, it will scale accordingly. If I open the website on my phone, a much smaller screen (at the time of writing, you never know), it will scale according to that smaller screen size.

Customer Perspective

Here are some things I noticed, if I was a customer on this website:

• Shopping Cart: Let's say I wanted to buy two Logitech Mouse, one to use and another one as a backup if the first one craps out. I press the button twice, and check the cart, only to see one item in my cart! The cart should display how many total items are in the cart, regardless if they are the same item or not. With the way it's set up now, someone can keep pressing the Add to Cart button, not knowing at checkout they are buying 8 mice! Of course they'll notice the price, hopefully...
• Description: I would have a short description about the item, maybe even being redirected to this description after pressing Add to Cart. If you go down this route, consider changing the name of the button to View Product or something along those lines. I realize that this is a mockup, but it's something to consider.
• Joe Doe?: I see that you have a user already signed in, and a logout button available. If you intend on having users log in and buying these items, you have a whole other beast to conquer. Here is a simple login form using jQuery you might want to take a look at.
• Pizzazz: When creating a website, for any reason, it should be inviting. As it stands, your website is just black and white. Add a cool background image, some different colors, something! Just having a little color can go a long way. Even just a little color and having images for your products can make your website seem professional. Take a look at Amazon. Without the images and adds, their website is very simple. You don't need crazy colors and other out-of-this-world imagery to attract users. A splash of color here and there can do wonders.

Javascript

My one thing on javascript, that I notice in some java/c# programs too.

// Check type
if (typeof object !== "object") return false;

I'm sorry, but I can't stand not having brackets around one line if statements. It makes it look so separated from everything else, and just doesn't look appealing to me. In my opinion, this code should look like

//Check type
if (typeof object !== "object") {
    return false;
}

Yeah, it takes up two more lines, but it's two bytes more, and it makes it consistent with all the bracketed code around it. Those are my two cents on your javascript.

Mozilla Developer Network, a comprehensible, usable, and accurate resource for everyone developing on the Open Web (thanks to Sᴀᴍ Onᴇᴌᴀ for pointing this resource out.)