2
\$\begingroup\$

I built a quite simple webpage to display a random quote from an array of quotes.

I would love to have some feedback on the JS, CSS, and HTML, for example, feedback about best practices, maintainability, readability, accessibility, semantic markup, performance, or anything you can spot that could be improved.

This is it:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Quotes to Live By</title>
    <link
      href="https://fonts.googleapis.com/css2?family=Montserrat:wght@700;900&display=swap"
      rel="stylesheet"
    />
    <style>
      body {
        margin: 0;
        background-color: hsl(24, 100%, 50%);
        font-family: "Montserrat", sans-serif;
        min-height: 100vh;
        text-align: center;
        display: flex;
        flex-direction: column;
        align-items: center;
        justify-content: center;
      }
      figure {
        margin: 2rem 0;
        display: flex;
        justify-content: center;
        flex-direction: column;
        flex: 1;
        cursor: pointer;
      }
      blockquote {
        margin: 0 auto 2rem;
        font-weight: 900;
        font-size: clamp(2rem, 8vw, 4rem);
        color: white;
        max-width: min(1400px, 90%);
      }
      figcaption {
        font-weight: 700;
        color: hsl(0, 0%, 20%);
        font-size: clamp(1rem, 4vw, 2rem);
      }
      figcaption::after, figcaption::before {
        content: "〰";
        margin: 0.5rem;
      }
      footer {
        font-size: 0.85rem;
        padding: 1rem;
        color: hsl(0, 0%, 20%);
        text-transform: uppercase;
      }
      footer a {
        color: inherit;
        text-decoration: none;
        padding: 0.5rem;
      }
      footer a:hover {
        text-decoration: underline wavy currentColor;
      }
      /* loader https://loading.io/css/ */
      .lds-ripple {
        display: inline-block;
        position: relative;
        width: 80px;
        height: 80px;
      }
      .lds-ripple div {
        position: absolute;
        border: 4px solid #fff;
        opacity: 1;
        border-radius: 50%;
        animation: lds-ripple 1s cubic-bezier(0, 0.2, 0.8, 1) infinite;
      }
      .lds-ripple div:nth-child(2) {
        animation-delay: -0.5s;
      }
      @keyframes lds-ripple {
        0% {
          top: 36px;
          left: 36px;
          width: 0;
          height: 0;
          opacity: 1;
        }
        100% {
          top: 0px;
          left: 0px;
          width: 72px;
          height: 72px;
          opacity: 0;
        }
      }
      /* end loader */
    </style>
  </head>
  <body>
    <figure>
      <div id="loader" class="lds-ripple">
        <div></div>
        <div></div>
      </div>
    </figure>
    <footer>
      <a href="https://github.com/MauricioRobayo/quotes-to-live-by"
        >Source code</a
      >
    </footer>
    <script>
      (function () {
        function createElement(type, textContent) {
          const el = document.createElement(type);
          el.textContent = textContent;
          return el;
        }

        function createQuoteElement({ quote, author = "", cite = "" }) {
          const blockquote = createElement("blockquote", quote);

          if (cite.trim()) {
            blockquote.setAttribute("cite", cite);
          }

          return blockquote
        }

        async function getQuotes() {
          const SESSION_STORAGE_KEY = "quotes";

          const sessionStorageQuotes = sessionStorage.getItem(
            SESSION_STORAGE_KEY
          );
          if (sessionStorageQuotes) {
            return JSON.parse(sessionStorageQuotes);
          }

          const response = await fetch(
            "https://raw.githubusercontent.com/MauricioRobayo/quotes-to-live-by/master/quotes-to-live-by.json"
          );
          if (!response.ok) {
            throw new Error(
              `Error fetching quotes: ${response.status} ${response.statusText}`
            );
          }
          const quotes = await response.json();

          sessionStorage.setItem(SESSION_STORAGE_KEY, JSON.stringify(quotes));

          return quotes;
        }

        function shuffle(quotes) {
          return quotes.sort(() => Math.random() - 0.5);
        }

        function render(container, ...children) {
          container.innerHTML = "";
          container.append(...children);
        }

        function renderQuote(container, quote) {
          const blockquote = createQuoteElement(quote)

          if (quote.author.trim()) {
            render(container, blockquote, createElement("figcaption", quote.author));
          } else {
            render(container, blockquote);
          }
        }

        function renderError(container, error) {
          render(container, createElement("div", error));
        }

        function loadQuote(container) {
          getQuotes()
          .then((quotes) => {
            renderQuote(container, shuffle(quotes)[0]);
          })
          .catch((error) => {
            renderError(container, error);
          });
        }

        const container = document.querySelector("figure");
        loadQuote(container);
        container.addEventListener("click", function() { loadQuote(container) });
      })();
    </script>
  </body>
</html>
```
\$\endgroup\$
3
\$\begingroup\$

This naive shuffle function is biased.

function shuffle(quotes) {
  return quotes.sort(() => Math.random() - 0.5);
}

There's an interesting account of a real-life example of the problem here. The correct algorithm is called the Fisher–Yates Shuffle:

function shuffle(quotes) {
    for (let i = quotes.length - 1; i > 0; i--) {
        const j = ~~(Math.random() * (i + 1));
        [quotes[i], quotes[j]] = [quotes[j], quotes[i]];
    }
    return quotes;
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ The was an interesting read, thanks for the feedback, I was not aware of this. \$\endgroup\$ – MauricioRobayo Aug 31 '20 at 15:55
  • 2
    \$\begingroup\$ What about choosing just a single quote from the array every time: quotes[Math.floor(Math.random() * quotes.length)]. We just need one quote every time so might be better just to choose a random one every time... do you see any issues on that approach instead of shuffling the array? \$\endgroup\$ – MauricioRobayo Aug 31 '20 at 16:09
  • 1
    \$\begingroup\$ I agree it's a better idea to pick a random element instead of shuffling the entire array unnecessarily. \$\endgroup\$ – GirkovArpa Sep 3 '20 at 2:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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