fix(theme): kill initial white frame + theme-aware skeleton tiles (#358 follow-up)
Two further fixes for the gallery loading sequence shown in @Rekoo-PS's frame breakdown on issue #358 — both about colours that didn't track the active theme. 1. Initial white frame (frame f1) The pre-React bootstrap script in #359 sets the cached background on documentElement, but the browser may paint the very first frame *before* that <script> tag runs (synchronous parse-time JS in the <head> is still slightly later than CSS apply-time). On first-visit dark-OS devices that meant a single white frame before the script resolved. Fix: move the OS-preference default into a <style> block that precedes the script. CSS @media (prefers-color-scheme) is applied before paint, so dark-OS devices land on dark from frame zero. The script keeps the per-gallery cache hit on top, and now also stamps the colour onto document.body in case the body element has already mounted by the time the script runs. 2. "Most annoying" skeleton tile frame (frame f4) Skeleton placeholders rendered as bright `bg-neutral-200` light grey regardless of theme. On a dark gallery that's the highest-contrast thing on screen during loading — the exact frame Rekoo-PS labelled "the most annoying" in the issue. Fix: the Skeleton component's background now reads `var(--color-surface-border)`, which ThemeContext already wires up per active theme (`#e5e5e5` light / `#2e2e2e` dark by default; per- event themes can override). The bare `<div>` no longer carries any colour utility class — the inline style supplies the active value. Also dropped the leftover `bg-white` on SkeletonCard / SkeletonTable in favour of `var(--color-surface)` for the same reason. Tests: New src/components/common/__tests__/Skeleton.test.tsx covers - Skeleton uses var(--color-surface-border, ...) - bg-neutral-200 is no longer present - SkeletonGalleryGrid tiles all inherit the theme colour - SkeletonCard surface uses var(--color-surface)
This commit is contained in:
Paul Nothaft
+24
-17
@@ -5,19 +5,33 @@
|
||||
<link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png" />
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
|
||||
<title>PicPeak - Photo Sharing Platform</title>
|
||||
|
||||
<!-- Pre-React theme bootstrap (#358).
|
||||
The browser may paint the very first frame before our inline
|
||||
<script> below runs, so we set OS-preference defaults via CSS
|
||||
here in <head> — that gets applied before any paint. The
|
||||
script then layers a per-gallery cache hit on top when one is
|
||||
available. Without this CSS, the very first frame on first-
|
||||
visit dark-OS devices flashed white briefly (see Rekoo-PS's
|
||||
frame f1 in the issue). -->
|
||||
<style>
|
||||
html, body { background-color: #fafafa; }
|
||||
@media (prefers-color-scheme: dark) {
|
||||
html, body { background-color: #171717; }
|
||||
}
|
||||
/* Smooth out the cache → API theme transition for the rare case
|
||||
where the cached colour drifts from the freshly fetched theme. */
|
||||
html { transition: background-color 200ms ease; }
|
||||
</style>
|
||||
|
||||
<script>
|
||||
/*
|
||||
* Pre-React theme bootstrap (#358).
|
||||
*
|
||||
* Without this, opening a dark-themed gallery flashes a white
|
||||
* background between the HTML paint and React applying the gallery
|
||||
* theme. We resolve a background colour synchronously from the URL
|
||||
* slug (cached on previous visits) or the OS preference, and apply
|
||||
* it to documentElement before any React render runs.
|
||||
*
|
||||
* Per-gallery cache is written by ThemeContext when the gallery
|
||||
* theme actually loads, keyed by slug, so revisits never flash.
|
||||
* First visits with no cache fall back to the OS preference.
|
||||
* The CSS @media block above handles the OS-preference default
|
||||
* before paint. This script then applies a per-gallery cached
|
||||
* background (written by ThemeContext on the previous visit) so
|
||||
* revisits land on the exact theme background from frame one.
|
||||
*/
|
||||
(function () {
|
||||
try {
|
||||
@@ -26,22 +40,15 @@
|
||||
if (m && m[1]) {
|
||||
bg = localStorage.getItem('gallery-theme-bg-' + decodeURIComponent(m[1]));
|
||||
}
|
||||
if (!bg && window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) {
|
||||
bg = '#171717';
|
||||
}
|
||||
if (bg) {
|
||||
var root = document.documentElement;
|
||||
root.style.backgroundColor = bg;
|
||||
document.body && (document.body.style.backgroundColor = bg);
|
||||
root.style.setProperty('--color-background', bg);
|
||||
}
|
||||
} catch (e) { /* never block render on a cache miss */ }
|
||||
})();
|
||||
</script>
|
||||
<style>
|
||||
/* Smooth out the cache→API theme transition for the rare case where
|
||||
the cached colour drifts from the freshly fetched theme (#358). */
|
||||
html { transition: background-color 200ms ease; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div id="root"></div>
|
||||
|
||||
@@ -16,8 +16,6 @@ export const Skeleton: React.FC<SkeletonProps> = ({
|
||||
height,
|
||||
animation = 'pulse'
|
||||
}) => {
|
||||
const baseClasses = 'bg-neutral-200';
|
||||
|
||||
const animationClasses = {
|
||||
pulse: 'animate-pulse',
|
||||
wave: 'animate-shimmer',
|
||||
@@ -30,14 +28,21 @@ export const Skeleton: React.FC<SkeletonProps> = ({
|
||||
rectangular: 'rounded-lg'
|
||||
};
|
||||
|
||||
const style: React.CSSProperties = {};
|
||||
// Theme-aware placeholder colour. Without this the skeleton tiles
|
||||
// rendered as bright bg-neutral-200 light grey on dark gallery
|
||||
// themes — the "most annoying" frame in #358's screenshots. Using
|
||||
// var(--color-surface-border) tracks whatever shade ThemeContext
|
||||
// resolves for the current colour mode (light: #e5e5e5, dark:
|
||||
// #2e2e2e by default; per-event themes can override).
|
||||
const style: React.CSSProperties = {
|
||||
backgroundColor: 'var(--color-surface-border, #e5e5e5)',
|
||||
};
|
||||
if (width) style.width = typeof width === 'number' ? `${width}px` : width;
|
||||
if (height) style.height = typeof height === 'number' ? `${height}px` : height;
|
||||
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
baseClasses,
|
||||
animationClasses[animation],
|
||||
variantClasses[variant],
|
||||
className
|
||||
@@ -74,9 +79,16 @@ export const SkeletonGroup: React.FC<SkeletonGroupProps> = ({
|
||||
);
|
||||
};
|
||||
|
||||
// Theme-aware container surface — same reasoning as the Skeleton
|
||||
// itself. Reads var(--color-surface) so the card sits on the right
|
||||
// background regardless of the active theme's colour mode.
|
||||
const SURFACE_STYLE: React.CSSProperties = {
|
||||
backgroundColor: 'var(--color-surface, #ffffff)',
|
||||
};
|
||||
|
||||
// Common skeleton patterns
|
||||
export const SkeletonCard: React.FC<{ className?: string }> = ({ className }) => (
|
||||
<div className={cn('bg-white rounded-lg shadow-sm p-6', className)}>
|
||||
<div className={cn('rounded-lg shadow-sm p-6', className)} style={SURFACE_STYLE}>
|
||||
<Skeleton height={24} width="60%" className="mb-4" />
|
||||
<SkeletonGroup count={3} />
|
||||
<div className="flex gap-3 mt-6">
|
||||
@@ -90,8 +102,8 @@ export const SkeletonTable: React.FC<{ rows?: number; className?: string }> = ({
|
||||
rows = 5,
|
||||
className
|
||||
}) => (
|
||||
<div className={cn('bg-white rounded-lg shadow-sm overflow-hidden', className)}>
|
||||
<div className="border-b border-neutral-200 p-4">
|
||||
<div className={cn('rounded-lg shadow-sm overflow-hidden', className)} style={SURFACE_STYLE}>
|
||||
<div className="border-b border-neutral-200 dark:border-neutral-700 p-4">
|
||||
<div className="flex gap-4">
|
||||
<Skeleton width="30%" height={20} />
|
||||
<Skeleton width="25%" height={20} />
|
||||
@@ -99,7 +111,7 @@ export const SkeletonTable: React.FC<{ rows?: number; className?: string }> = ({
|
||||
<Skeleton width="25%" height={20} />
|
||||
</div>
|
||||
</div>
|
||||
<div className="divide-y divide-neutral-100">
|
||||
<div className="divide-y divide-neutral-100 dark:divide-neutral-800">
|
||||
{Array.from({ length: rows }).map((_, index) => (
|
||||
<div key={index} className="p-4">
|
||||
<div className="flex gap-4">
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import React from 'react';
|
||||
import { render } from '@testing-library/react';
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { Skeleton, SkeletonGalleryGrid, SkeletonCard } from '../Skeleton';
|
||||
|
||||
/**
|
||||
* Regression for #358. The Skeleton placeholders used to hard-code
|
||||
* `bg-neutral-200`, which rendered as bright light grey on dark
|
||||
* gallery themes (Rekoo-PS's "most annoying" frame). They must instead
|
||||
* use the active theme's surface-border colour so the placeholders
|
||||
* track whatever the theme defines for both light and dark modes.
|
||||
*/
|
||||
describe('Skeleton — theme-aware colour', () => {
|
||||
it('uses var(--color-surface-border) for the placeholder background', () => {
|
||||
const { container } = render(<Skeleton />);
|
||||
const div = container.querySelector('div');
|
||||
expect(div).not.toBeNull();
|
||||
expect(div!.style.backgroundColor).toBe('var(--color-surface-border, #e5e5e5)');
|
||||
});
|
||||
|
||||
it('does NOT add the legacy hard-coded bg-neutral-200 class', () => {
|
||||
const { container } = render(<Skeleton />);
|
||||
const div = container.querySelector('div');
|
||||
expect(div!.className).not.toMatch(/bg-neutral-200/);
|
||||
});
|
||||
|
||||
it('SkeletonGalleryGrid tiles inherit the theme colour', () => {
|
||||
const { container } = render(<SkeletonGalleryGrid count={3} />);
|
||||
// Tiles are the Skeleton components — direct children of the
|
||||
// gallery-grid wrapper. They carry aria-busy="true" while the
|
||||
// wrapper does not, which is the cleanest way to select them.
|
||||
const tiles = container.querySelectorAll('[aria-busy="true"]');
|
||||
expect(tiles.length).toBe(3);
|
||||
tiles.forEach((tile) => {
|
||||
expect((tile as HTMLElement).style.backgroundColor).toBe(
|
||||
'var(--color-surface-border, #e5e5e5)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('SkeletonCard surface uses var(--color-surface)', () => {
|
||||
const { container } = render(<SkeletonCard />);
|
||||
const card = container.firstElementChild as HTMLElement;
|
||||
expect(card).not.toBeNull();
|
||||
expect(card.style.backgroundColor).toBe('var(--color-surface, #ffffff)');
|
||||
// Sanity: should not retain the old bg-white class either
|
||||
expect(card.className).not.toMatch(/bg-white/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user