4
\$\begingroup\$

There are plenty of these online already but none of them quite fit what I wanted. I've been meaning to learn some more front-end stuff so I decided to make one myself. I know some JavaScript but would consider myself an intermediate. I am definitely a beginner with HTML and CSS. The code is here on my GitHub and hosted here through GitHub Pages. The code is below, but I've provided the links in case it is easier. In the interest of keeping the code concise I have removed much of the armor data from the code here, but it still behaves the same.

You do not need to understand the game mechanics to understand this webpage. A brief summary is that you check the boxes on the table corresponding to the armor piece and level of that armor piece you are interested in tracking. The webpage presents a list of the totals of all the items you need. If you check level 1 of Champion's Tunic you will see that you need 3 Silent Princess. When you check level 2 (which by itself needs 2 Shard of Farosh's Horn and 3 Silent Princess) you will see that you need a total of 2 Shard of Farosh's Horn and 6 Silent Princess.

I have used "responsive design" so please also be sure to try this in a full screen and adjust the width of the browser if you want to see that functionality. I'm not exclusively looking for feedback on that aspect or anything, but it was a tough part for me being new to CSS.

<!DOCTYPE html>

<head>
  <style>
    td {
      text-align: right;
    }

    .flex-container {
      display: flex;
      flex-flow: row wrap;
      justify-content: center;
    }

    .flex-item {
      --margin: 5px;
      display: flex;
      flex-flow: column nowrap;
      justify-content: flex-start;
      align-items: center;
      flex-grow: 1;
      max-width: calc(50% - 2 * var(--margin));
      flex-basis: 50%;
      margin: var(--margin);
    }

    .title {
      text-align: center;
    }

    @media (max-width: 768px) {
      .flex-container {
        flex-flow: column nowrap;
      }

      .flex-item {
        max-width: 100%;
      }
    }
  </style>
  <script>
    "use strict";
    function init() {
      // Populate the armor table
      let tableBody = document.getElementById("armors");

      Object.entries(armorData).forEach(([armorPiece, levels]) => {
        let newRow = tableBody.insertRow();
        let allCell = newRow.insertCell();
        allCell.innerHTML = makeCheckAll(armorPiece);
        let nameCell = newRow.insertCell();
        nameCell.innerHTML = armorPiece;
        for (let levelIndex = 0; levelIndex < levels.length; levelIndex++) {
          let cell = newRow.insertCell();
          cell.innerHTML = makeCheck(armorPiece, levelIndex);
        }
      })
      update();
    }
    function update() {
      // Populate the materials table
      let tableBody = document.getElementById("totals");

      // First, clear old data
      while (tableBody.rows.length > 0) {
        tableBody.deleteRow(0);
      }

      let matsNeeded = new Map();

      // Find which are needed
      let wholeTableChecked = true;
      let wholeColumnChecked = [true, true, true, true];
      Object.entries(armorData).forEach(([armorPiece, levels]) => {
        let wholeRowChecked = true;
        for (let levelIndex = 0; levelIndex < levels.length; levelIndex++) {
          if (isChecked(armorPiece, levelIndex)) {
            let level = levels[levelIndex];
            Object.entries(level).forEach(([material, quantity]) => {
              if (matsNeeded.has(material)) {
                let currentNeededQuantity = matsNeeded.get(material);
                matsNeeded.set(material, currentNeededQuantity + quantity);
              } else {
                matsNeeded.set(material, quantity);
              }
            })
          } else {
            wholeRowChecked = false;
            wholeTableChecked = false;
            wholeColumnChecked[levelIndex] = false;
          }
        }
        setChecked(armorPiece, "", wholeRowChecked); // TODO
      })
      setChecked("all", "", wholeTableChecked); // TODO
      for (let columnIndex = 0; columnIndex < wholeColumnChecked.length; columnIndex++) {
        setChecked("", columnIndex, wholeColumnChecked[columnIndex]); // TODO
      }

      // Sort by material
      matsNeeded = new Map([...matsNeeded].sort((a, b) => String(a[0]).localeCompare(b[0])))

      // Put them in the table
      matsNeeded.forEach((quantity, material) => {
        let newRow = tableBody.insertRow();
        newRow.insertCell().innerHTML = material;
        newRow.insertCell().innerHTML = quantity;
      })
    }

    function updateWholeRow(id) {
      Object.entries(armorData).forEach(([armorPiece, levels]) => {
        if (armorPiece === id) {
          for (let levelIndex = 0; levelIndex < levels.length; levelIndex++) {
            setChecked(armorPiece, levelIndex, isChecked(armorPiece, "")) // TODO
          }
        }
      })
      update();
    }

    function updateWholeColumn(levelIndex) {
      Object.entries(armorData).forEach(([armorPiece, levels]) => {
        setChecked(armorPiece, levelIndex, isChecked("", levelIndex)) // TODO
      })
      update();
    }

    function updateWholeTable() {
      Object.entries(armorData).forEach(([armorPiece, levels]) => {
        for (let levelIndex = 0; levelIndex < levels.length; levelIndex++) {
          setChecked(armorPiece, levelIndex, isChecked("all", "")) // TODO
        }
      })
      update();
    }

    function makeCheck(name, level) {
      // <input type="checkbox" id="Armor NameX" onchange="update()">
      return "<input type=\"checkbox\" id=\"" + name + level + "\" onchange=\"update()\">";
    }

    function makeCheckAll(name) {
      // <input type="checkbox" id="Armor NameX" onchange="update()">
      return "<input type=\"checkbox\" id=\"" + name + "\" onchange=\"updateWholeRow(this.id)\">";
    }

    function isChecked(name, level) {
      return document.getElementById(name + level).checked;
    }

    function setChecked(name, level, state) {
      document.getElementById(name + level).checked = state;
    }

    const armorData = {
      "Champion's Tunic": [
        {
          "Silent Princess": 3,
        },
        {
          "Silent Princess": 3,
          "Shard of Farosh's Horn": 2,
        },
        {
          "Silent Princess": 3,
          "Shard of Naydra's Horn": 2,
        },
        {
          "Silent Princess": 10,
          "Shard of Dinraal's Horn": 2,
        },
      ],
      "Sand Boots": [
        {
          "Molduga Fin": 5,
          "Hightail Lizard": 10,
        },
        {
          "Molduga Fin": 10,
          "Swift Carrot": 10,
        },
        {
          "Molduga Guts": 2,
          "Rushroom": 15,
        },
        {
          "Molduga Guts": 4,
          "Swift Violet": 15,
        },
      ],
      "Snow Boots": [
        {
          "Octorok Tentacle": 5,
          "Hightail Lizard": 10,
        },
        {
          "Octo Balloon": 5,
          "Swift Carrot": 10,
        },
        {
          "Octorok Eyeball": 5,
          "Rushroom": 15,
        },
        {
          "Naydra's Scale": 2,
          "Swift Violet": 15,
        },
      ],
      "Amber Earrings": [
        {
          "Amber": 5,
          "Flint": 3,
        },
        {
          "Amber": 10,
          "Flint": 3,
        },
        {
          "Amber": 20,
          "Flint": 3,
        },
        {
          "Amber": 30,
          "Flint": 3,
        },
      ],
      "Diamond Circlet": [
        {
          "Diamond": 2,
          "Flint": 3,
        },
        {
          "Diamond": 4,
          "Flint": 3,
        },
        {
          "Diamond": 6,
          "Star Fragment": 1,
        },
        {
          "Diamond": 10,
          "Star Fragment": 1,
        },
      ],
    }
  </script>
</head>

<body onload="init()">
  <h1 class="title">Armor upgrade materials tracker</h1>
  <p>
    Each checkbox of the first table represents each upgrade (represented in
    game by stars). The base version of the armor is not represented because
    this tool is only for tracking upgrade materials needed. That means things
    like the 5 Opals you need for the Opal Earrings are not tracked here.
  </p>
  <div class="flex-container">
    <div class="flex-item armor">
      <h2>Armors</h2>
      <p>
        Check the armors you <i>want</i> but do not <i>have</i>. If you already
        have an upgrade or just do not care about tracking the materials for it
        right now then <i>leave the box unchecked</i>.
      </p>
      <p>
        Checkboxes on the "all" side will check/uncheck the entire row. The one
        on the first row will do this for the entire table.
      </p>
      <table>
        <thead>
          <tr>
            <th>All? <input type="checkbox" id="all" onchange="updateWholeTable()"></th>
            <th>Name</th>
            <th>1★ <input type="checkbox" id="0" onchange="updateWholeColumn(this.id)"></th>
            <th>2★ <input type="checkbox" id="1" onchange="updateWholeColumn(this.id)"></th>
            <th>3★ <input type="checkbox" id="2" onchange="updateWholeColumn(this.id)"></th>
            <th>4★ <input type="checkbox" id="3" onchange="updateWholeColumn(this.id)"></th>
          </tr>
        </thead>
        <tbody id="armors"></tbody>
      </table>
    </div>
    <div class="flex-item">
      <h2>Materials needed</h2>
      <p>
        These are the materials you need to get for the <i>checked</i> upgrades
        above.
      </p>
      <table>
        <thead>
          <tr>
            <th>Material</th>
            <th>Amount</th>
          </tr>
        </thead>
        <tbody id="totals"></tbody>
      </table>
    </div>
  </div>
</body>
<footer>
  <hr>
  <a href="https://github.com/JacksonBailey/armor">GitHub repository</a>
</footer>

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

1 HTML

Your HTML markup is invalid. The W3C Validator returns 3 flags which include 2 red flags.

  1. You miss the <html> opening and closing tag which needs to wrap the head and body element
  2. You miss the lang attribute
  3. You miss the <title> tag within your head element
  4. Your <footer> tag is outside of the body and as such an invalid placement

Semantics and valid placement is important for accessibility. That again is not only important for disabled persons but also for SEO ratings. For the same reasons, the lang attribute and the title tag are needed.
The body element is not the main element of your website but the body of the document. It has to contain every visual placement. The body itself can be divided into <header>, <main>, and <footer>. <body> is not to be mistaken as the <main> element that comes between header and footer.

1.1 Conventions

  1. I see no usage of conventions and no comments in your entire document. Overall your code is harder than necessary to read
  2. The naming of your IDs and classes is mostly not self-explaining. They are not given with the entire context in mind. The class title should be an id while I see no reason in your code why it needs to be given in the first place. That element is your only h1 element and at least your first h1 element. As such could have been addressed directly without class or id. The class armor is a class that is used on a section for armor. As such it should be an id which would make the id armors redundant and not necessary. Especially the usage of ids with the name 0, 1, 2, 3, or 4 is not an useful name that would tell other developers what the ids are for. More useful is a name such as all-1-stars or all-2-stars. Names in general do not have to be short but self-explaining
  3. The code is well-indented but misses only gaps or line breaks to separate logical groups of elements. It is one of the main reasons that your code is hard to read
  4. Your CSS and JS and include directly in your document which not makes usage of caching performance as well as making the code harder to read. This also prevents the reuse of your code
  5. You are using an onload attribute to start your script. This makes maintenance harder and is not the modern way to solve the issue. The 2 modern solutions are either to use the defer attribute on the script tag or to use window.addEventListener('DOMContentLoaded', function). While the third "outdated" method is to put the script at the end of your body. Same rule applies to the onchange attribute. Instead of usign the onclick attribute over and over again, you could simply use an event listener and an event delegation

1.2 Accessibility & Semantics

With exception of the footer which has an invalid placement, you're not using the semantic tags. Overall, your code is inaccessible.

  1. You are basically dividing the main part of you content in 2 sections. As such a <section> should be used
  2. Your inputs missing a label and as such a screen reader would recognize a checkbox but a blind person would never know what those checkboxes are for
  3. The flex-container should be a <main> element while the title and description would be appropriate as a <header> element

1.3 Head Element

As already mentioned above, you should not write your entire CSS and JS code into your head element but connect the files there. Respectively for CSS using the <link>tag.

  1. You miss the <charset> tag which should always be used to set the correct character set that you are using
  2. You miss a <title> tag which will title your website and also be used when saving the website as favorites
  3. While not necessary, you should consider a favicon

1.4 Body Element

There is nothing more to add which hasn't been coverd yet.

2 CSS

Your CSS is super basic and does not add any real design choices. Your CSS is mainly used to align items. Which it self is a good start but would not thrill any user. While it makes the CSS in general short, you miss heavily on the UX/UI side that you completely ignored. It will probably not make any users sticking with your tool nor to give it any real attention.

2.1 Accessibility

You have not used any colors. While this might seem unappealing in terms of UI it will not influence accessibility as you have not to take care of contrast.

  1. Your tool is has limited accessibility for users with touch or eye-tracking controls. On touchscreens the inputs need to be larger to allow a easy and comfortable (un)checking with your fingers.
  2. Your different section are only split with a margin. Carefully placed board woulds increase a UI and UX as well as readability.
  3. Especially in larger tables you should color the rows so that they visually be differentiated

2.2 Responsiveness

While you believe that your website is responsive, you're actually far from it. Responsiveness does not necessarily mean to have different layouts at different screen widths. Responsive Web Design is a concept that its basic idea is an UX concept. It aims to provide the best experience in form of readability, accessibility and controlling or usability of your website / web application. For that, it has to adjust to the screen size. This however means, that it is not limited to changing your layout on different widths.

  1. Your design completely ignores usability on mobile devices
  2. The smallest screen where your layout will be displayed correctly is at 419px while mobile devices can start at 320px
  3. You do not take DPR (Device Pixel Ratio) into account. While testing your web application on a real mobile device, you will properly recognize undesired behavior.
  4. A good responsive design starts with mobile first, then tablets, then desktops

2.3 Structure

  1. I see no structure in your CSS. flex-container and flex-items are grouped correctly but you should order your CSS in some way, Either specificty weight, appearance or alphabetically
  2. While it is acceptable to declare CSS variables within an element in your case, you should in general consider to declare them at the top within the :root selector
  3. You are using flex-flow while in both instances you're declaring a default value which is redundant but not needed.
  4. flex-wrap is not needed in your case as you declare a direction and only have 2 elements. You either could solve the entire design with flex-wrap itself or by just declaring the flex-direction
  5. You setting a margin to the elements and then calculating the width depending on the margin. The easy and correct solution would be the usage of the gap property on the parent. You also could use CSS Grid which saves you the calculation

3 JS

While your JS is good for a beginner it lacks readability, formatting and modern approaches. Your techniques is a mixture of modern and outdated approaches. The usage of JS modules would have been helpful here. While I understand why you used an array of objects and not a database, it is something you should have included as a module and split from the main JS.

3.1 Conventions

Conventions make code easier to read and as such also easier to maintain.

  1. List global constants at the very top and use capital names to differentiate them from variables
  2. Make parameters visual by adding underscores to it such as --param. This helps to differentiate them from other variables or constants
  3. Your variable names are self-explaining. No issue here
  4. Don't spare with comments. While code should be self-explaining, carefully placed comments would greatly improve readability. Since you're missing comments, others will have a hard time to understand how your code would work
  5. Don't use innerHTML as they pose a security risk (XSS) and are slow (requires the reparsing of the DOM)

3.2 Logic & Coding Style

There is a lot to improve. While I not want to point out every single "issue" I want to just push you into a direction on what you should focus on.

  1. let wholeColumnChecked = [true, true, true, true]; is hard to understand. Using an array is not needed here at all. It also is not scalable at all. Make use of querySelectorAll('all inputs:checked') === querySelectorAll('all inputs').
  2. Learn to use backticks:

let name = "Armor";
let level = 4;

console.log(`<input type="checkbox" id="${name}${level}">`);

  1. Learn to use querySelector and querySelectorAll
  2. Learn to use an event delegation
  3. Learn to use data attributes

4 Rating

Especially since you are new you should read into the Dunning-Kruger-Effect and Mt. Stupid and Valley of Death.

With your statement:

I have used "responsive design" so please also be sure to try this in a full screen and adjust the width of the browser if you want to see that functionality. I'm not exclusively looking for feedback on that aspect or anything, but it was a tough part for me being new to CSS

you where actually quite wrong. Responsive Web Design and UX and UI should be your next big topic to learn. You underestimate the influence of the frontend part of those topics.

While you show a really good start, you need still to learn some of the basics especially with correctness and semantics in mind.

\$\endgroup\$
5
  • \$\begingroup\$ re: "mt. stupid", what would be the proper term for the two "columns" becoming one "column" when the pane is narrow besides "responsive design"? I was under the impression that was the term for layouts that adapt to screen size, is it not? \$\endgroup\$ Commented May 5, 2023 at 18:11
  • \$\begingroup\$ RWD (Responsive Web Design) is a whole large concept. Switching layouts on different screen sizes is one part of RWD but RWD is not soley limited to it. Making your website responsive does not mean to just swap you layout but allowing to read and control the content comfortably. \$\endgroup\$
    – tacoshy
    Commented May 6, 2023 at 11:27
  • \$\begingroup\$ I understand that but my question is what would you call that? \$\endgroup\$ Commented May 9, 2023 at 18:00
  • 1
    \$\begingroup\$ A partial integrationbof RWD by using an adaptive layout \$\endgroup\$
    – tacoshy
    Commented May 10, 2023 at 4:26
  • \$\begingroup\$ Thanks, "adaptive layout" is exactly what I was looking for. This will help me with searching for more info on the topic. \$\endgroup\$ Commented May 10, 2023 at 17:28

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.