Merge pull request #362 from the-luap/fix/issue-358-theme-aware-skeleton
fix(theme): kill initial white frame + theme-aware skeleton tiles (#358 follow-up)
This commit is contained in:
+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">
|
||||
@@ -86,12 +98,12 @@ export const SkeletonCard: React.FC<{ className?: string }> = ({ className }) =>
|
||||
</div>
|
||||
);
|
||||
|
||||
export const SkeletonTable: React.FC<{ rows?: number; className?: string }> = ({
|
||||
rows = 5,
|
||||
className
|
||||
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