\$\begingroup\$
\$\endgroup\$
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>
```
asked Aug 30, 2020 at 16:07
1 Answer 1
\$\begingroup\$
\$\endgroup\$
3
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;
}
answered Aug 30, 2020 at 22:17
-
1\$\begingroup\$ The was an interesting read, thanks for the feedback, I was not aware of this. \$\endgroup\$MauricioRobayo– MauricioRobayo2020年08月31日 15:55:43 +00:00Commented Aug 31, 2020 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– MauricioRobayo2020年08月31日 16:09:38 +00:00Commented Aug 31, 2020 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– GirkovArpa2020年09月03日 02:41:35 +00:00Commented Sep 3, 2020 at 2:41
lang-css