From 845e3dcea3ae40470d96500a62f41d24284d0634 Mon Sep 17 00:00:00 2001
From: Rachel Stiansen <rachelks@ntnu.no>
Date: Thu, 17 Sep 2026 13:21:43 +0200
Subject: [PATCH] feat: improve book card layout consistency (#40)

---
 src/__snapshots__/App.test.tsx.snap           |  75 +++++++------
 src/components/BookCard.css                   | 101 +++++++++++++----
 src/components/BookCard.test.tsx              |  67 ++++++++++-
 src/components/BookCard.tsx                   |  81 ++++++++------
 .../__snapshots__/BookCard.test.tsx.snap      | 104 ++++++++++++------
 5 files changed, 304 insertions(+), 124 deletions(-)

diff --git a/src/__snapshots__/App.test.tsx.snap b/src/__snapshots__/App.test.tsx.snap
index 9ac41dc..20eeb61 100644
--- a/src/__snapshots__/App.test.tsx.snap
+++ b/src/__snapshots__/App.test.tsx.snap
@@ -119,37 +119,50 @@ exports[`App > matches the snapshot 1`] = `
           <div
             class="book-card-content"
           >
-            <h2>
-              A Wizard of Earthsea
-            </h2>
-            <p
-              class="book-card-authors"
-            >
-              Ursula K. Le Guin
-            </p>
-            <p>
-              First published: 
-              1968
-            </p>
-            <p
-              class="book-card-rating"
-            >
-              Rating: 
-              4.8
-               / 5
-               (20 ratings)
-            </p>
-            <ul
-              aria-label="Subjects, showing 2 of 2"
-              class="book-card-subjects"
-            >
-              <li>
-                fantasy
-              </li>
-              <li>
-                magic
-              </li>
-            </ul>
+            <div
+              class="book-card-metadata"
+            >
+              <h2>
+                A Wizard of Earthsea
+              </h2>
+              <p
+                class="book-card-authors"
+                title="Ursula K. Le Guin"
+              >
+                Ursula K. Le Guin
+              </p>
+              <p
+                class="book-card-publication"
+              >
+                First published: 1968
+              </p>
+              <p
+                class="book-card-rating"
+              >
+                Rating: 
+                4.8
+                 / 5
+                 (20 ratings)
+              </p>
+            </div>
+            <div
+              class="book-card-subjects-section"
+            >
+              <ul
+                aria-label="Subjects, showing 2 of 2"
+                class="book-card-subjects"
+              >
+                <li>
+                  fantasy
+                </li>
+                <li>
+                  magic
+                </li>
+              </ul>
+              <div
+                class="book-card-subjects-controls"
+              />
+            </div>
             <button
               aria-label="Add A Wizard of Earthsea to favorites"
               aria-pressed="false"
diff --git a/src/components/BookCard.css b/src/components/BookCard.css
index 0052604..d47fc0e 100644
--- a/src/components/BookCard.css
+++ b/src/components/BookCard.css
@@ -11,8 +11,10 @@
 
 .book-card-cover {
   display: grid;
-  align-self: stretch;
-  min-height: 12rem;
+  align-self: center;
+  width: 100%;
+  height: 18rem;
+  min-width: 0;
   place-items: center;
   background-color: #edf2f7;
   color: #52606d;
@@ -20,9 +22,12 @@
 }
 
 .book-card-cover img {
-  width: 100%;
-  height: 100%;
-  object-fit: cover;
+  display: block;
+  max-width: 100%;
+  max-height: 100%;
+  width: auto;
+  height: auto;
+  object-fit: contain;
 }
 
 .book-card-cover p {
@@ -30,21 +35,36 @@
 }
 
 .book-card-content {
+  min-width: 0;
   padding: 1.25rem 1.25rem 1.25rem 0;
 }
 
+.book-card-metadata {
+  height: 7.5rem;
+  overflow: visible;
+}
+
 .book-card-content h2 {
   margin: 0;
+  max-height: 2.4em;
   color: #243b53;
+  line-height: 1.2;
   overflow-wrap: anywhere;
+  overflow: hidden;
 }
 
 .book-card-content p {
-  margin: 0.5rem 0 0;
+  margin: 0.25rem 0 0;
+  line-height: 1.2;
   color: #52606d;
 }
 
 .book-card-authors {
+  height: 1.2em;
+  min-width: 0;
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
   font-weight: 600;
 }
 
@@ -53,13 +73,45 @@
   font-weight: 600;
 }
 
+.book-card-publication {
+  min-height: 1.2em;
+  overflow: visible;
+}
+
+.book-card-subjects-section {
+  position: relative;
+  display: flex;
+  flex-direction: column;
+  height: 9rem;
+  min-height: 9rem;
+  margin-top: 1.5rem;
+}
+
+.book-card-subjects-section.is-expanded {
+  height: 9rem;
+  min-height: 9rem;
+}
+
 .book-card-subjects {
   display: flex;
+  flex: 0 0 7rem;
   flex-wrap: wrap;
   gap: 0.5rem;
-  margin: 1rem 0 0;
+  align-content: flex-start;
+  height: 7rem;
+  min-height: 7rem;
+  margin: 0;
   padding: 0;
   list-style: none;
+  overflow-x: hidden;
+  overflow-y: hidden;
+}
+
+.book-card-subjects.is-expanded {
+  flex: 1 1 auto;
+  height: auto;
+  min-height: 0;
+  overflow-y: auto;
 }
 
 .book-card-subjects li {
@@ -70,31 +122,34 @@
   color: #243b53;
   padding: 0.25rem 0.625rem;
   font-size: 0.875rem;
-  overflow-wrap: anywhere;
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
 }
 
-.book-card-subjects .book-card-subjects-more {
-  padding: 0;
-  background-color: #edf2f7;
-  color: #52606d;
-  font-weight: 600;
+.book-card-subjects-controls {
+  display: flex;
+  flex: 0 0 2rem;
+  align-items: flex-start;
+  padding-top: 0.5rem;
 }
 
-.book-card-subjects-more button {
+.book-card-subjects-controls button {
   max-width: 100%;
   border: 0;
-  border-radius: inherit;
-  background: transparent;
-  color: inherit;
+  border-radius: 999px;
+  background-color: #edf2f7;
+  color: #52606d;
   padding: 0.25rem 0.625rem;
   font: inherit;
+  font-weight: 600;
   overflow-wrap: anywhere;
   cursor: pointer;
 }
 
-.book-card-subjects-more button:focus-visible {
-  outline: 2px solid #243b53;
-  outline-offset: 2px;
+.book-card-subjects-controls button:focus-visible {
+  outline: 0.1875rem solid #2c3e50;
+  outline-offset: 0.125rem;
 }
 
 @media (max-width: 32rem) {
@@ -103,14 +158,14 @@
   }
 
   .book-card-cover {
-    min-height: 0;
-    aspect-ratio: 4 / 3;
+    width: 100%;
+    height: 16rem;
   }
 
   .book-card-content {
     position: relative;
     z-index: 1;
-    padding: 0 1.25rem 1.25rem;
+    padding: 1rem 1.25rem 1.25rem;
     border-top: 1px solid #d5dde5;
     background-color: #ffffff;
   }
diff --git a/src/components/BookCard.test.tsx b/src/components/BookCard.test.tsx
index 473d9be..db58b51 100644
--- a/src/components/BookCard.test.tsx
+++ b/src/components/BookCard.test.tsx
@@ -1,4 +1,4 @@
-import { render, screen } from "@testing-library/react";
+import { fireEvent, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { describe, expect, it } from "vitest";
 import type { Book } from "../types";
@@ -48,6 +48,7 @@ describe("BookCard", () => {
     expect(screen.queryByText(/^Rating:/)).not.toBeInTheDocument();
     expect(screen.getByText("No cover available")).toBeInTheDocument();
     expect(screen.queryByRole("img")).not.toBeInTheDocument();
+    expect(screen.getByText("No cover available").closest(".book-card-cover")).toBeInTheDocument();
     expect(container).toMatchSnapshot();
   });
 
@@ -66,6 +67,61 @@ describe("BookCard", () => {
     expect(screen.queryByRole("button", { name: /subjects/ })).not.toBeInTheDocument();
   });
 
+  it("shows the fallback when a cover image fails to load", () => {
+    render(<BookCard book={fullBook} />);
+
+    const image = screen.getByRole("img", { name: "Cover of A Wizard of Earthsea" });
+    fireEvent.error(image);
+
+    expect(screen.queryByRole("img")).not.toBeInTheDocument();
+    const fallback = screen.getByText("No cover available");
+    expect(fallback).toBeInTheDocument();
+    expect(fallback.closest(".book-card-cover")).toBeInTheDocument();
+  });
+
+  it("keeps long author lists on one line with the full text available", () => {
+    const longAuthors = [
+      "Ursula K. Le Guin",
+      "Another Author",
+      "A Third Author",
+      "A Fourth Author",
+    ];
+
+    render(<BookCard book={{ ...fullBook, authors: longAuthors }} />);
+
+    const authorText = longAuthors.join(", ");
+    const authorElement = screen.getByText(authorText);
+    expect(authorElement).toHaveClass("book-card-authors");
+    expect(authorElement).toHaveAttribute("title", authorText);
+  });
+
+  it("keeps the subject area fixed while expanding and resets its scroll on collapse", async () => {
+    const user = userEvent.setup();
+    const bookWithManySubjects: Book = {
+      ...fullBook,
+      subjects: Array.from({ length: 12 }, (_, index) => `Subject ${index + 1}`),
+    };
+
+    render(<BookCard book={bookWithManySubjects} />);
+
+    const subjects = screen.getByRole("list", { name: "Subjects, showing 3 of 12" });
+    expect(subjects).toHaveClass("book-card-subjects");
+    expect(subjects.querySelector("button")).not.toBeInTheDocument();
+    expect(screen.getByRole("button", { name: "Show 9 more subjects" })).toBeInTheDocument();
+
+    await user.click(screen.getByRole("button", { name: "Show 9 more subjects" }));
+
+    expect(screen.getByRole("list", { name: "Subjects, showing 12 of 12" })).toBe(subjects);
+    expect(subjects).toHaveClass("is-expanded");
+    expect(screen.getByRole("button", { name: "Show fewer subjects" })).toBeInTheDocument();
+
+    subjects.scrollTop = 100;
+    await user.click(screen.getByRole("button", { name: "Show fewer subjects" }));
+
+    expect(subjects.scrollTop).toBe(0);
+    expect(screen.getByRole("list", { name: "Subjects, showing 3 of 12" })).toBe(subjects);
+  });
+
   it("expands and collapses long subject lists", async () => {
     const user = userEvent.setup();
     const bookWithManySubjects: Book = {
@@ -108,4 +164,13 @@ describe("BookCard", () => {
     expect(screen.queryByText("Earthsea")).not.toBeInTheDocument();
     expect(screen.getByRole("button", { name: "Show 3 more subjects" })).toBeInTheDocument();
   });
+
+  it("keeps long subject text in the DOM for CSS truncation", () => {
+    const longSubject = "A very long subject name that should stay on one line";
+    render(<BookCard book={{ ...fullBook, subjects: [longSubject] }} />);
+
+    const subject = screen.getByText(longSubject);
+    expect(subject).toBeInTheDocument();
+    expect(subject.closest("li")).toBeInTheDocument();
+  });
 });
diff --git a/src/components/BookCard.tsx b/src/components/BookCard.tsx
index 8c02bc0..f01901a 100644
--- a/src/components/BookCard.tsx
+++ b/src/components/BookCard.tsx
@@ -1,4 +1,4 @@
-import { useState } from "react";
+import { useRef, useState } from "react";
 import { coverUrl } from "../api/openLibrary";
 import type { Book } from "../types";
 import { FavoriteButton } from "./FavoriteButton";
@@ -13,63 +13,78 @@ interface BookCardProps {
 }
 
 export function BookCard({ book, isFavorite = false, onFavoriteToggle }: BookCardProps) {
+  const [coverError, setCoverError] = useState<{ bookKey: string; failed: boolean }>({
+    bookKey: book.key,
+    failed: false,
+  });
   const [subjectState, setSubjectState] = useState({ bookKey: book.key, isExpanded: false });
+  const subjectsRef = useRef<HTMLUListElement>(null);
   const imageUrl = coverUrl(book.coverId);
   const authors = book.authors.length > 0 ? book.authors.join(", ") : "Unknown author";
+  const hasCoverLoadFailed = coverError.bookKey === book.key && coverError.failed;
   const isSubjectsExpanded = subjectState.bookKey === book.key && subjectState.isExpanded;
   const visibleSubjects = isSubjectsExpanded
     ? book.subjects
     : book.subjects.slice(0, MAX_VISIBLE_SUBJECTS);
   const remainingSubjectCount = book.subjects.length - visibleSubjects.length;
 
+  function toggleSubjects() {
+    const nextExpanded = !isSubjectsExpanded;
+    setSubjectState({ bookKey: book.key, isExpanded: nextExpanded });
+    if (!nextExpanded && subjectsRef.current) {
+      subjectsRef.current.scrollTop = 0;
+    }
+  }
+
   return (
     <article className="book-card">
       <div className="book-card-cover">
-        {imageUrl ? (
-          <img src={imageUrl} alt={`Cover of ${book.title}`} />
+        {imageUrl && !hasCoverLoadFailed ? (
+          <img
+            src={imageUrl}
+            alt={`Cover of ${book.title}`}
+            onError={() => setCoverError({ bookKey: book.key, failed: true })}
+          />
         ) : (
           <p>No cover available</p>
         )}
       </div>
       <div className="book-card-content">
-        <h2>{book.title}</h2>
-        <p className="book-card-authors">{authors}</p>
-        {book.firstPublishYear !== null && <p>First published: {book.firstPublishYear}</p>}
-        {book.averageRating !== null && (
-          <p className="book-card-rating">
-            Rating: {book.averageRating.toFixed(1)} / 5
-            {book.ratingsCount !== null && ` (${book.ratingsCount} ratings)`}
+        <div className="book-card-metadata">
+          <h2>{book.title}</h2>
+          <p className="book-card-authors" title={authors}>
+            {authors}
           </p>
-        )}
-        {book.subjects.length > 0 && (
+          <p className="book-card-publication">
+            {book.firstPublishYear !== null && `First published: ${book.firstPublishYear}`}
+          </p>
+          {book.averageRating !== null && (
+            <p className="book-card-rating">
+              Rating: {book.averageRating.toFixed(1)} / 5
+              {book.ratingsCount !== null && ` (${book.ratingsCount} ratings)`}
+            </p>
+          )}
+        </div>
+        <div className={`book-card-subjects-section${isSubjectsExpanded ? " is-expanded" : ""}`}>
           <ul
-            className="book-card-subjects"
+            ref={subjectsRef}
+            className={`book-card-subjects${isSubjectsExpanded ? " is-expanded" : ""}`}
             aria-label={`Subjects, showing ${visibleSubjects.length} of ${book.subjects.length}`}
           >
             {visibleSubjects.map((subject) => (
               <li key={subject}>{subject}</li>
             ))}
-            {book.subjects.length > MAX_VISIBLE_SUBJECTS && (
-              <li className="book-card-subjects-more">
-                <button
-                  type="button"
-                  onClick={() =>
-                    setSubjectState({ bookKey: book.key, isExpanded: !isSubjectsExpanded })
-                  }
-                  aria-label={
-                    isSubjectsExpanded
-                      ? "Show fewer subjects"
-                      : `Show ${remainingSubjectCount} more subjects`
-                  }
-                >
-                  {isSubjectsExpanded
-                    ? "Show fewer subjects"
-                    : `Show ${remainingSubjectCount} more subjects`}
-                </button>
-              </li>
-            )}
           </ul>
-        )}
+          <div className="book-card-subjects-controls">
+            {(isSubjectsExpanded || remainingSubjectCount > 0) && (
+              <button type="button" onClick={toggleSubjects} aria-expanded={isSubjectsExpanded}>
+                {isSubjectsExpanded
+                  ? "Show fewer subjects"
+                  : `Show ${remainingSubjectCount} more subjects`}
+              </button>
+            )}
+          </div>
+        </div>
         {onFavoriteToggle && (
           <FavoriteButton
             bookTitle={book.title}
diff --git a/src/components/__snapshots__/BookCard.test.tsx.snap b/src/components/__snapshots__/BookCard.test.tsx.snap
index bf9dd87..73d9a7a 100644
--- a/src/components/__snapshots__/BookCard.test.tsx.snap
+++ b/src/components/__snapshots__/BookCard.test.tsx.snap
@@ -16,37 +16,50 @@ exports[`BookCard > renders a complete book 1`] = `
     <div
       class="book-card-content"
     >
-      <h2>
-        A Wizard of Earthsea
-      </h2>
-      <p
-        class="book-card-authors"
+      <div
+        class="book-card-metadata"
       >
-        Ursula K. Le Guin, Another Author
-      </p>
-      <p>
-        First published: 
-        1968
-      </p>
-      <p
-        class="book-card-rating"
+        <h2>
+          A Wizard of Earthsea
+        </h2>
+        <p
+          class="book-card-authors"
+          title="Ursula K. Le Guin, Another Author"
+        >
+          Ursula K. Le Guin, Another Author
+        </p>
+        <p
+          class="book-card-publication"
+        >
+          First published: 1968
+        </p>
+        <p
+          class="book-card-rating"
+        >
+          Rating: 
+          4.2
+           / 5
+           (100 ratings)
+        </p>
+      </div>
+      <div
+        class="book-card-subjects-section"
       >
-        Rating: 
-        4.2
-         / 5
-         (100 ratings)
-      </p>
-      <ul
-        aria-label="Subjects, showing 2 of 2"
-        class="book-card-subjects"
-      >
-        <li>
-          Fantasy
-        </li>
-        <li>
-          Magic
-        </li>
-      </ul>
+        <ul
+          aria-label="Subjects, showing 2 of 2"
+          class="book-card-subjects"
+        >
+          <li>
+            Fantasy
+          </li>
+          <li>
+            Magic
+          </li>
+        </ul>
+        <div
+          class="book-card-subjects-controls"
+        />
+      </div>
     </div>
   </article>
 </div>
@@ -67,14 +80,33 @@ exports[`BookCard > renders a placeholder for a minimal book 1`] = `
     <div
       class="book-card-content"
     >
-      <h2>
-        Untitled Book
-      </h2>
-      <p
-        class="book-card-authors"
+      <div
+        class="book-card-metadata"
       >
-        Unknown author
-      </p>
+        <h2>
+          Untitled Book
+        </h2>
+        <p
+          class="book-card-authors"
+          title="Unknown author"
+        >
+          Unknown author
+        </p>
+        <p
+          class="book-card-publication"
+        />
+      </div>
+      <div
+        class="book-card-subjects-section"
+      >
+        <ul
+          aria-label="Subjects, showing 0 of 0"
+          class="book-card-subjects"
+        />
+        <div
+          class="book-card-subjects-controls"
+        />
+      </div>
     </div>
   </article>
 </div>