Merge pull request 'Room under the homepage titles, and the site stops flashing on every click' (#12) from home-titles-and-page-transitions into main

Reviewed-on: #12

src/main/resources/templates/catering.html

@@ -17,7 +17,7 @@
         mind, and we will bake to it.
       </p>
 
-      <div class="mt-10" th:replace="~{fragments/opening :: divider}"></div>
+      <div th:replace="~{fragments/opening :: divider('mt-10')}"></div>
 
       <div class="mt-12 max-w-6xl mx-auto space-y-16 md:space-y-20">
 

src/main/resources/templates/contact.html

@@ -81,13 +81,11 @@
       <aside class="space-y-6">
         <div class="panel p-6 md:p-8">
           <h2 class="h4 text-bakery-900">Our hours</h2>
-          <div class="mt-4 text-bakery-800"
+          <div th:replace="~{fragments/visit :: hours('mt-4', 'text-bakery-800')}"></div>
-               th:replace="~{fragments/visit :: hours('text-bakery-800')}"></div>
         </div>
         <div class="panel p-6 md:p-8">
           <h2 class="h4 text-bakery-900">Find us</h2>
-          <div class="mt-4 text-bakery-800"
+          <div th:replace="~{fragments/visit :: address('mt-4 text-bakery-800', 'link-plain')}"></div>
-               th:replace="~{fragments/visit :: address('link-plain')}"></div>
         </div>
         <div class="panel-quiet p-6 md:p-8">
           <h2 class="h4 text-bakery-900">Ordering for a crowd?</h2>

src/main/resources/templates/fragments/footer.html

@@ -20,11 +20,10 @@
 
       <div>
         <h3 class="label text-bakery-300 mb-4 font-adbhashitha">Visit</h3>
-        <div th:replace="~{fragments/visit :: address('link-on-dark')}"></div>
+        <div th:replace="~{fragments/visit :: address('', 'link-on-dark')}"></div>
         <!--/* The hours are the thing people come to a bakery's site for, and the footer is on every
                page. */-->
-        <div class="mt-4 text-sm text-bakery-200"
+        <div th:replace="~{fragments/visit :: hours('mt-4', 'text-sm text-bakery-200')}"></div>
-             th:replace="~{fragments/visit :: hours('text-sm text-bakery-200')}"></div>
       </div>
     </div>
 

src/main/resources/templates/fragments/opening.html

@@ -54,8 +54,9 @@
   <p th:if="${!#strings.isEmpty(blurb)}" class="mt-3 text-bakery-100 leading-relaxed" th:text="${blurb}">A line.</p>
 </div>
 
-<!--/* The branch from the wordmark, between one part of a page and the next. */-->
+<!--/* The branch from the wordmark, between one part of a page and the next. Takes its own spacing,
-<div th:fragment="divider" class="flex justify-center" aria-hidden="true">
+       because the tag a caller writes the th:replace on is discarded along with any class on it. */-->
+<div th:fragment="divider(className)" th:class="'flex justify-center ' + ${className}" aria-hidden="true">
   <span class="mark mark-r w-12 h-12 text-bakery-400"></span>
 </div>
 </body>

src/main/resources/templates/fragments/visit.html

@@ -11,16 +11,21 @@
 
   Both fragments take the classes that vary by where they sit — cream in the footer, sage on white in a
   card — so the content is shared without the styling being averaged into something that suits neither.
+
+  They take TWO classes each, and the outer one is not a convenience. A caller cannot put a class on the
+  tag it writes the th:replace on: th:replace swaps that whole tag out for the fragment, so the attribute
+  is dropped and nothing says so. Every call site here had tried it — an mt-4 that never spaced anything,
+  a text-bakery-800 that left the address a shade darker than the hours beside it.
 */-->
 
-<ul th:fragment="hours(rowClass)" class="space-y-3">
+<ul th:fragment="hours(className, rowClass)" th:class="'space-y-3 ' + ${className}">
   <li th:each="span : ${hours}" th:class="'flex justify-between gap-4 ' + ${rowClass}">
     <span th:text="${span.days}">Tuesday – Friday</span>
     <span class="font-medium whitespace-nowrap" th:text="${span.hours}">7:00am – 2:00pm</span>
   </li>
 </ul>
 
-<address th:fragment="address(linkClass)" class="not-italic space-y-3">
+<address th:fragment="address(className, linkClass)" th:class="'not-italic space-y-3 ' + ${className}">
   <p th:text="${shopStreet}">215 E Main Street</p>
   <p th:text="${shopTown}">Princeville, IL 61559</p>
   <p>

src/main/resources/templates/home.html

@@ -45,14 +45,18 @@
   <!--/* What people come in for */-->
   <section class="section bg-bakery-50">
     <div class="container">
-      <div class="mb-12" th:replace="~{fragments/opening :: section-head('What people come in for', '')}"></div>
+      <!--/* The gap under a section head belongs to the element BELOW it, the way catering's price grids
-      <div class="card-grid">
+             already do it. It cannot live on this tag: th:replace swaps the whole element out for the
+             fragment, so a class written here never reaches the page — which is what happened to the
+             mb-12 that used to sit here, leaving the title flush against the cards. */-->
+      <div th:replace="~{fragments/opening :: section-head('What people come in for', '')}"></div>
+      <div class="card-grid mt-14 md:mt-18">
         <div th:each="item : ${ {'Cinnamon Rolls', 'Sugar Cookies', 'Cakes'} }"
              class="panel overflow-hidden text-center">
           <!--/* The filenames carry the spaces in these names; SitePhotos encodes them. */-->
           <img th:src="${photos.of('gallery/' + item + '.webp')}" th:alt="${item}" width="600" height="400" loading="lazy"
                class="w-full h-56 object-cover">
-          <h3 class="panel-head border-b-0 border-t h4 md:text-2xl text-bakery-800 py-6" th:text="${item}">Cinnamon Rolls</h3>
+          <h3 class="panel-head border-b-0 border-t h4 md:text-2xl text-bakery-800 px-4 py-8" th:text="${item}">Cinnamon Rolls</h3>
         </div>
       </div>
     </div>
@@ -103,16 +107,16 @@
   <!--/* Visit us */-->
   <section id="visit" class="section bg-bakery-50 scroll-mt-24">
     <div class="container">
-      <div class="mb-12" th:replace="~{fragments/opening :: section-head('Visit us', '')}"></div>
+      <!--/* Same as above: the margin goes on the grid, not on a tag th:replace is about to discard. */-->
-      <div class="grid grid-cols-1 md:grid-cols-2 gap-8 md:gap-12 measure-wide">
+      <div th:replace="~{fragments/opening :: section-head('Visit us', '')}"></div>
+      <div class="grid grid-cols-1 md:grid-cols-2 gap-8 md:gap-12 measure-wide mt-14 md:mt-18">
         <div class="panel p-6 md:p-8">
-          <h2 class="h3 text-bakery-900 mb-6">Our hours</h2>
+          <h2 class="h3 text-bakery-900 mb-8">Our hours</h2>
-          <div th:replace="~{fragments/visit :: hours('text-bakery-800')}"></div>
+          <div th:replace="~{fragments/visit :: hours('', 'text-bakery-800')}"></div>
         </div>
         <div class="panel p-6 md:p-8">
-          <h2 class="h3 text-bakery-900 mb-6">Find us</h2>
+          <h2 class="h3 text-bakery-900 mb-8">Find us</h2>
-          <div class="text-bakery-800"
+          <div th:replace="~{fragments/visit :: address('text-bakery-800', 'link-plain')}"></div>
-               th:replace="~{fragments/visit :: address('link-plain')}"></div>
         </div>
       </div>
     </div>

src/main/styles/base.css

@@ -1,6 +1,6 @@
 /*
- * Element defaults: the page background, the body type, the fade-in, and the two small resets the site
+ * Element defaults: the page background, the body type, how one page gives way to the next, and the two
- * needs. Anything with a class name belongs in components.css instead.
+ * small resets the site needs. Anything with a class name belongs in components.css instead.
  */
 
 html {
@@ -14,23 +14,32 @@ body {
   color: var(--color-bakery-900);
   font-family: var(--font-sans);
   overflow-x: hidden;
-  opacity: 0;
+}
-  animation: fadeIn 0.5s ease-in forwards;
+
+/*
+ * BETWEEN PAGES.
+ *
+ * The body used to open at `opacity: 0` and fade in over half a second on every page. That belonged to
+ * the SPA, where it ran ONCE, when React booted — the rest of the site's navigation happened inside a
+ * document that had already faded in. Server-rendered, every nav link and every filter chip is a new
+ * document, so the same two lines fired on every click: the page went blank, then slid 10px and faded
+ * back. A navigation that was already fast read as the whole site reloading.
+ *
+ * A cross-document view transition is what that fade was reaching for. The browser holds the old page
+ * up until the new one is ready and cross-fades between the two, so nothing is ever blank — and where
+ * it isn't supported you get a plain, immediate page load, which is the correct fallback and is still
+ * better than what this replaced.
+ */
+@view-transition {
+  navigation: auto;
 }
 
 @media (prefers-reduced-motion: reduce) {
   html { scroll-behavior: auto; }
-  body { animation: none; opacity: 1; }
+  /* Cut the cross-fade, keep the navigation. */
-}
+  ::view-transition-old(root),
-
+  ::view-transition-new(root) {
-@keyframes fadeIn {
+    animation: none;
-  from {
-    opacity: 0;
-    transform: translateY(10px);
-  }
-  to {
-    opacity: 1;
-    transform: translateY(0);
   }
 }
 
@@ -109,8 +118,6 @@ select.field {
   body {
     background: #fff;
     color: #000;
-    opacity: 1;
-    animation: none;
   }
 
   /* Sage-on-white blocks become plain text: the words matter, the ink does not. */

src/test/java/com/itsthevine/web/TemplateHygieneTest.java

@@ -0,0 +1,94 @@
+package com.itsthevine.web;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * A class on a tag that carries {@code th:replace} is a class that never reaches the page.
+ *
+ * <p>{@code th:replace} substitutes the whole host element for the fragment — the fragment's own
+ * {@code class} attribute is what gets rendered, and anything written on the calling tag is dropped.
+ * Silently: Thymeleaf does not warn, the page still renders, and it looks nearly right. That is what
+ * makes it worth a test rather than a code review.
+ *
+ * <p>It had happened seven times before anyone went looking. Two homepage section titles were written
+ * with an {@code mb-12} and rendered flush against the grid below them; the contact page's two headings
+ * had the same; the catering divider's {@code mt-10} and the footer's went the same way; and the
+ * address rendered a shade darker than the hours beside it because its {@code text-bakery-800} was
+ * dropped too. Every one of those is invisible in the source and obvious on the page.
+ *
+ * <p>The fix in each case is to give the fragment a parameter for the class, which is why
+ * {@code visit :: hours} and {@code visit :: address} take the element's own class as well as the
+ * row's or the link's. This test is the thing that stops the next one being written.
+ *
+ * <p>No Spring context and no database: it reads the templates off disk, so it costs nothing.
+ * {@code th:insert} is deliberately not checked — that one keeps the host tag, so a class on it is
+ * fine.
+ */
+class TemplateHygieneTest {
+
+    private static final Path TEMPLATES = Path.of("src/main/resources/templates");
+
+    /** Any tag, including one written across several lines. */
+    private static final Pattern TAG = Pattern.compile("<[a-zA-Z][^>]*?>", Pattern.DOTALL);
+
+    /** A plain {@code class=}, not {@code th:class=} and not {@code th:classappend=}. */
+    private static final Pattern PLAIN_CLASS = Pattern.compile("(?<!th:)\\bclass\\s*=");
+
+    @Test
+    @DisplayName("no template writes a class on a tag that th:replace is about to discard")
+    void noClassOnAReplacedTag() throws IOException {
+        List<String> offenders = new ArrayList<>();
+
+        try (Stream<Path> files = Files.walk(TEMPLATES)) {
+            files.filter(p -> p.toString().endsWith(".html")).sorted().forEach(file -> {
+                String source = read(file);
+                Matcher tag = TAG.matcher(source);
+                while (tag.find()) {
+                    String element = tag.group();
+                    if (element.contains("th:replace") && PLAIN_CLASS.matcher(element).find()) {
+                        long line = source.substring(0, tag.start()).chars().filter(c -> c == '\n').count() + 1;
+                        offenders.add("%s:%d — %s".formatted(file, line, element.replaceAll("\\s+", " ")));
+                    }
+                }
+            });
+        }
+
+        assertThat(offenders)
+                .describedAs("""
+                        These tags carry both a class and a th:replace. th:replace swaps the tag out for \
+                        the fragment, so the class is dropped and the page renders without it. Pass the \
+                        class to the fragment as a parameter instead — see fragments/visit.html.""")
+                .isEmpty();
+    }
+
+    /** Guards the guard: a test that reads no files would pass forever. */
+    @Test
+    @DisplayName("the templates are actually being read")
+    void theTemplatesAreThere() throws IOException {
+        try (Stream<Path> files = Files.walk(TEMPLATES)) {
+            assertThat(files.filter(p -> p.toString().endsWith(".html")).count())
+                    .describedAs("templates directory at %s", TEMPLATES.toAbsolutePath())
+                    .isGreaterThanOrEqualTo(10);
+        }
+    }
+
+    private static String read(Path file) {
+        try {
+            return Files.readString(file);
+        } catch (IOException e) {
+            throw new IllegalStateException("could not read " + file, e);
+        }
+    }
+}