Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions src/components/HeroImagePair.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
"use client";

import Image from "next/image";
import { useLayoutEffect, useRef, useState, type CSSProperties } from "react";

interface HeroImagePairProps {
lightSrc: string;
darkSrc: string;
lightAlt: string;
darkAlt: string;
sizes: string;
/** objectPosition style for both variants, or undefined for the 50/50 default. */
imgStyle?: CSSProperties;
}

function rand(min: number, max: number) {
return Math.round((min + Math.random() * (max - min)) * 10) / 10;
}

/**
* Returns the per-instance CSS custom properties that drive the shared
* `.glitch-reveal` keyframes (see globals.css). Mirrors FadeReveal so each
* hero reveal jitters slightly differently instead of marching in lockstep.
*/
function glitchVars(): CSSProperties {
return {
"--gr-jitter-1": `${rand(0.5, 1.2)}px`,
"--gr-jitter-2": `${rand(-0.8, -0.3)}px`,
"--gr-jitter-3": `${rand(0.1, 0.5)}px`,
"--gr-mid-opacity": `${rand(0.5, 0.75)}`,
animationDuration: `${rand(280, 360)}ms`,
} as CSSProperties;
}

/**
* Load-aware hero image pair. Renders the two stacked theme variants
* (light visible in light mode, dark visible in dark mode) and manages a
* loading spinner + glitch-in reveal:
*
* - Cached case: a layout effect detects `img.complete` synchronously on
* mount (before the browser paints), so a warm image appears INSTANTLY —
* no spinner flash, no glitch.
* - Real-load case: the spinner shows while the visible variant streams in;
* that variant's `onLoad` removes the spinner and applies `.glitch-reveal`
* for a quick glitch-in.
*
* `prefers-reduced-motion: reduce` is honored by the global media query in
* globals.css, which collapses every animation (including `.glitch-reveal`)
* to ~0ms — the image simply appears, with the spinner still allowed.
*
* The reveal class lives on a theme-agnostic inner wrapper that holds BOTH
* variants, so the glitch reads correctly whichever variant the theme is
* currently displaying. We deliberately keep the variants themselves free of
* any opacity / transition classes so the View Transitions crossfade on the
* site-wide theme toggle is not degraded (see issue #53).
*/
export function HeroImagePair({
lightSrc,
darkSrc,
lightAlt,
darkAlt,
sizes,
imgStyle,
}: HeroImagePairProps) {
const innerRef = useRef<HTMLDivElement>(null);
// `loaded` gates the spinner; `glitch` is true only after a genuine load
// (never for a cached image). Both start false so the SSR/first paint shows
// the spinner; the layout effect flips them before paint for warm images.
const [state, setState] = useState<{
loaded: boolean;
glitch: boolean;
vars: CSSProperties;
}>({ loaded: false, glitch: false, vars: {} });

/** The currently displayed variant (the other is `display: none`). */
function visibleImg(): HTMLImageElement | null {
const imgs = innerRef.current?.querySelectorAll("img");
if (!imgs) return null;
for (const img of imgs) {
if (window.getComputedStyle(img).display !== "none") {
return img as HTMLImageElement;
}
}
return null;
}

useLayoutEffect(() => {
// Synchronous, pre-paint cache check: if the visible variant is already
// decoded, reveal it instantly with no spinner and no glitch.
const img = visibleImg();
if (img?.complete && img.naturalWidth > 0) {
// eslint-disable-next-line react-hooks/set-state-in-effect
setState({ loaded: true, glitch: false, vars: {} });
}
// Empty deps: this is a one-shot mount probe, mirroring FadeReveal.
}, []);

function handleLoad(e: React.SyntheticEvent<HTMLImageElement>) {
// Only the visible variant drives the reveal; the hidden (display:none)
// variant still fires onLoad as it streams, but it must not flip state.
if (window.getComputedStyle(e.currentTarget).display === "none") return;
setState((prev) => {
if (prev.loaded) return prev;
// Reaching here means the spinner was actually shown (not cached at
// mount), so this is a genuine load → glitch-in.
return { loaded: true, glitch: true, vars: glitchVars() };
});
}

return (
<>
{/* Centered loading spinner, shown only while the visible variant loads. */}
{!state.loaded && (
<div
className="hero-spinner pointer-events-none absolute inset-0 z-10 flex items-center justify-center"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION — no-JS / hydration-failure trade-off introduced by this PR.

The previous spinner (base 61510a1) rendered behind both images with no z-index and no JS gate, so a loaded opaque object-cover image painted over it even with zero client JS. This spinner is z-10 (on top) and is removed only by client state ({!state.loaded && ...}), so on a hydration failure or with JS disabled it sits over a fully-loaded hero.

It's pointer-events-none (no interaction block) and the repo declares no progressive-enhancement contract, so this is degraded-state cosmetics, not a blocker — moving loading state client-side is the whole point of a load-aware spinner (reasonable trade-off, R9). Cheap hardening if you ever want the JS-off path: render the spinner behind the images (so the loaded image covers it as a fallback). Flagging so the trade-off is intentional rather than accidental.

aria-hidden="true"
>
<div className="h-8 w-8 animate-spin rounded-full border-4 border-border border-t-accent" />
</div>
)}
{/* `absolute inset-0` makes this the positioning context the two
`fill` images need, while exactly overlaying the parent wrapper so
there is no layout shift. The glitch class is added only on a real
load; otherwise this is an inert full-bleed layer. */}
<div
ref={innerRef}
className={`absolute inset-0${state.glitch ? " glitch-reveal" : ""}`}
style={state.vars}
>
<Image
src={lightSrc}
alt={lightAlt}
fill
fetchPriority="high"
sizes={sizes}
className="object-cover dark:hidden"
style={imgStyle}
onLoad={handleLoad}
/>
<Image
src={darkSrc}
alt={darkAlt}
fill
fetchPriority="high"
sizes={sizes}
className="hidden object-cover dark:block"
style={imgStyle}
onLoad={handleLoad}
/>
</div>
</>
);
}
42 changes: 12 additions & 30 deletions src/components/ThemeAwareHero.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Image from "next/image";
import { HeroImagePair } from "@/components/HeroImagePair";
import { isMediaObject } from "@/lib/types/media";
import type { Media } from "@/payload-types";

Expand Down Expand Up @@ -55,12 +55,11 @@ function toRelativeSrc(src: string): string {
* The parent `relative` wrapper holds an `aspect-ratio` matching the source
* images so swapping which child is `display: none` never causes layout shift.
*
* Loading state: a CSS-only spinner sits in the center of the positioned
* wrapper, rendered BEHIND both images (the images are `object-cover` + `fill`,
* so the opaque loaded image paints over the spinner). This keeps the
* component a zero-JS server component — no `onLoad` handler, no client
* boundary — while still showing a standard centered spinner during the
* brief window before the active variant's bytes arrive.
* This component stays a server component: it does the Media null-guard, URL
* normalization, focal-point math, and aspect-ratio sizing, then hands the
* image pair to {@link HeroImagePair} — a thin `"use client"` child that owns
* the load-aware spinner + glitch-in reveal. Keeping the boundary here means
* the public props and call sites (PostCard, posts/[slug]) are unchanged.
*
* LCP note: both children are `loading="lazy"` by default with
* `fetchPriority="high"`. The browser correctly skips lazy-loading the
Expand Down Expand Up @@ -92,30 +91,13 @@ export function ThemeAwareHero({
className={`relative w-full overflow-hidden ${className}`.trim()}
style={aspectStyle}
>
{/* Centered loading spinner, painted over by the opaque image once loaded. */}
<div
className="hero-spinner pointer-events-none absolute inset-0 flex items-center justify-center"
aria-hidden="true"
>
<div className="h-8 w-8 animate-spin rounded-full border-4 border-border border-t-accent" />
</div>
<Image
src={toRelativeSrc(light.url)}
alt={light.alt || alt}
fill
fetchPriority="high"
<HeroImagePair
lightSrc={toRelativeSrc(light.url)}
darkSrc={toRelativeSrc(dark.url)}
lightAlt={light.alt || alt}
darkAlt={dark.alt || alt}
sizes={resolvedSizes}
className="object-cover dark:hidden"
style={imgStyle}
/>
<Image
src={toRelativeSrc(dark.url)}
alt={dark.alt || alt}
fill
fetchPriority="high"
sizes={resolvedSizes}
className="hidden object-cover dark:block"
style={imgStyle}
imgStyle={imgStyle}
/>
</div>
);
Expand Down
143 changes: 140 additions & 3 deletions tests/unit/components/ThemeAwareHero.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect, vi } from "vitest";
import { render, screen } from "@testing-library/react";
import { describe, it, expect, vi, afterEach } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import React from "react";
import { ThemeAwareHero } from "@/components/ThemeAwareHero";
import type { Media } from "@/payload-types";
Expand All @@ -19,6 +19,7 @@ vi.mock("next/image", () => ({
width,
height,
style,
onLoad,
...rest
}: {
src: string;
Expand All @@ -31,16 +32,19 @@ vi.mock("next/image", () => ({
width?: number;
height?: number;
style?: React.CSSProperties;
onLoad?: React.ReactEventHandler<HTMLImageElement>;
[key: string]: unknown;
}) => {
// Strip out Next.js-only props that would generate React warnings on an
// intrinsic <img> element; preserve the props we actually want to assert.
// onLoad is preserved so tests can simulate the image finishing loading.
void rest;
return React.createElement("img", {
src,
alt,
className,
style,
onLoad,
"data-fill": fill ? "true" : undefined,
"data-fetch-priority": fetchPriority,
"data-priority": priority ? "true" : undefined,
Expand Down Expand Up @@ -106,7 +110,9 @@ describe("ThemeAwareHero", () => {
expect(darkImg?.className).toContain("object-cover");
});

it("renders a centered loading spinner behind the images", () => {
it("renders a centered loading spinner while the image is loading", () => {
// jsdom never actually loads the image, so img.complete stays false and the
// mount layout-effect leaves the component in its loading state.
const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
);
Expand All @@ -119,6 +125,137 @@ describe("ThemeAwareHero", () => {
expect(spinner?.querySelector(".animate-spin")).toBeTruthy();
});

// --- load-aware behavior (spinner / cached-instant / glitch-in) ---

it("shows NO spinner and NO glitch when the visible image is already cached on mount", () => {
// Stub the cached signal the layout effect reads: a warm image reports
// complete=true with a real naturalWidth before paint.
const completeSpy = vi
.spyOn(window.HTMLImageElement.prototype, "complete", "get")
.mockReturnValue(true);
const naturalWidthSpy = vi
.spyOn(window.HTMLImageElement.prototype, "naturalWidth", "get")
.mockReturnValue(1200);

const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
);

// No spinner flash for a cached image, and no glitch-reveal applied.
expect(container.querySelector(".hero-spinner")).toBeNull();
expect(container.querySelector(".glitch-reveal")).toBeNull();

completeSpy.mockRestore();
naturalWidthSpy.mockRestore();
});

it("removes the spinner and applies glitch-reveal when a real load completes", () => {
// Default jsdom: images are not complete on mount, so the spinner shows.
const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
);
expect(container.querySelector(".hero-spinner")).toBeTruthy();
expect(container.querySelector(".glitch-reveal")).toBeNull();

// Simulate the visible variant finishing a genuine load.
const lightImg = container.querySelector(
'img[src="/media/post-light.png"]'
) as HTMLImageElement;
fireEvent.load(lightImg);

// Spinner gone; glitch-in applied to the (theme-agnostic) image wrapper.
expect(container.querySelector(".hero-spinner")).toBeNull();
expect(container.querySelector(".glitch-reveal")).toBeTruthy();
});

// --- visible/hidden onLoad discrimination (the display:"none" guard) ---
//
// The component decides which variant "owns" the reveal by reading
// window.getComputedStyle(img).display: only the variant that is NOT
// display:"none" may remove the spinner and trigger the glitch. jsdom never
// applies the Tailwind stylesheet, so by default BOTH variants report
// display:"inline" and the guard branch (handleLoad early-return for the
// hidden variant) is never exercised — visibleImg() passes only by
// first-element fallthrough. These tests stub getComputedStyle so the dark
// variant reports display:"none" (hidden) and the light variant reports a
// visible display, then drive onLoad on each variant to prove the guard.

let computedStyleSpy: ReturnType<typeof vi.spyOn> | undefined;

afterEach(() => {
computedStyleSpy?.mockRestore();
computedStyleSpy = undefined;
});

/**
* Stub getComputedStyle so the DARK variant (src .../post-dark.png) reports
* display:"none" and everything else delegates to jsdom's real
* implementation (so the light variant stays visible: display:"inline").
* The component only reads `.display`, so a minimal override is sufficient.
*/
function stubDarkHidden() {
const real = window.getComputedStyle.bind(window);
computedStyleSpy = vi
.spyOn(window, "getComputedStyle")
.mockImplementation((el: Element, pseudo?: string | null) => {
if (
el instanceof window.HTMLImageElement &&
(el.getAttribute("src") ?? "").endsWith("post-dark.png")
) {
return { display: "none" } as unknown as CSSStyleDeclaration;
}
return real(el, pseudo ?? undefined);
});
}

it("ignores onLoad from the HIDDEN (display:none) variant — no spinner removal, no glitch", () => {
stubDarkHidden();

const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
);
// Spinner shows (images not complete at mount); no glitch yet.
expect(container.querySelector(".hero-spinner")).toBeTruthy();
expect(container.querySelector(".glitch-reveal")).toBeNull();

// Fire load on the HIDDEN variant. Its onLoad must early-return on the
// display:"none" guard and leave state untouched.
const darkImg = container.querySelector(
'img[src="/media/post-dark.png"]'
) as HTMLImageElement;
fireEvent.load(darkImg);

// No state change: spinner still present, still no glitch.
expect(container.querySelector(".hero-spinner")).toBeTruthy();
expect(container.querySelector(".glitch-reveal")).toBeNull();
});

it("honors onLoad from the VISIBLE variant — removes spinner and applies glitch (with dark variant stubbed hidden)", () => {
stubDarkHidden();

const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
);
expect(container.querySelector(".hero-spinner")).toBeTruthy();
expect(container.querySelector(".glitch-reveal")).toBeNull();

// Loading the hidden variant first must NOT consume the reveal (guard).
const darkImg = container.querySelector(
'img[src="/media/post-dark.png"]'
) as HTMLImageElement;
fireEvent.load(darkImg);
expect(container.querySelector(".hero-spinner")).toBeTruthy();
expect(container.querySelector(".glitch-reveal")).toBeNull();

// Now the VISIBLE (light) variant finishes: spinner removed + glitch-in.
const lightImg = container.querySelector(
'img[src="/media/post-light.png"]'
) as HTMLImageElement;
fireEvent.load(lightImg);
expect(container.querySelector(".hero-spinner")).toBeNull();
expect(container.querySelector(".glitch-reveal")).toBeTruthy();
});

it("does NOT apply transition-opacity or fade animation CSS (View Transitions compliance)", () => {
const { container } = render(
<ThemeAwareHero light={lightMedia} dark={darkMedia} alt="Hero" />
Expand Down
Loading