feat(brand): render the real product glyph SVGs (not inline approximations)
BrandGlyph drew hardcoded monochrome shapes from an inline switch and
ignored the actual /brand/glyphs/*.svg files. Render the real files via a
robust <img> (fail-soft onError, intrinsic width/height so no layout
shift, alt for a11y parity) so every existing placement — PageHeader
(Tracker/About/Release), Command Palette, Release Notes, Onboarding,
Login — shows the full-color brand tiles. Deletes the ~85-line dead
BrandGlyphShape switch and the now-unused useId import.
Senior-review while-here:
- Guard test (client/lib/brand.test.ts): asserts every BrandGlyphName
asset — and every /brand asset in brand.ts — resolves to a real,
non-empty file on disk. Catches the "image added but path wrong /
renamed" regression TS can't see.
- Widen vitest include to {js,jsx,ts,tsx}: TypeScript client tests were
silently never run (client/hooks/useAutoSave.test.ts). Now 6 files / 50
tests run (was 4 / 42).
- Remove dead brand.ts entries: legacyLogo (pointed at the OLD pre-brand
/img/logo.png) and the unused ADMIN_GLYPH const.
- PageHeader glyph class inline-grid -> inline-block for the <img>.
Verified: typecheck + lint clean; test:client 50/50; build (glyphs land
in dist/brand/glyphs, served 200 as image/svg+xml); e2e probe 17/17;
login visual baseline unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
1238ac5b14
commit
df1f61d6cc
|
|
@ -1,4 +1,4 @@
|
|||
import { useId, type ComponentProps } from 'react';
|
||||
import { type ComponentProps } from 'react';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { BRAND, BRAND_GLYPHS, type BrandGlyphName } from '@/lib/brand';
|
||||
import { useTheme } from '@/contexts/ThemeContext';
|
||||
|
|
@ -35,104 +35,18 @@ export function BrandGlyph({
|
|||
className?: string;
|
||||
}) {
|
||||
const glyph = BRAND_GLYPHS[name];
|
||||
const gradientId = `bt-glyph-${useId().replace(/:/g, '')}`;
|
||||
|
||||
return (
|
||||
<span
|
||||
className={cn(
|
||||
'relative inline-grid h-10 w-10 shrink-0 place-items-center overflow-hidden rounded-xl',
|
||||
'border border-primary/20 bg-primary/[0.08] text-primary shadow-sm shadow-primary/10',
|
||||
className,
|
||||
)}
|
||||
aria-label={glyph.label}
|
||||
role="img"
|
||||
>
|
||||
<svg viewBox="0 0 48 48" className="h-7 w-7" aria-hidden="true">
|
||||
<defs>
|
||||
<linearGradient id={gradientId} x1="8" x2="40" y1="40" y2="8" gradientUnits="userSpaceOnUse">
|
||||
<stop offset="0" stopColor="currentColor" stopOpacity="0.68" />
|
||||
<stop offset="1" stopColor="currentColor" />
|
||||
</linearGradient>
|
||||
</defs>
|
||||
<BrandGlyphShape name={name} gradientId={gradientId} />
|
||||
</svg>
|
||||
</span>
|
||||
<img
|
||||
src={glyph.asset}
|
||||
alt={glyph.label}
|
||||
width={40}
|
||||
height={40}
|
||||
draggable={false}
|
||||
// Fail soft: if the asset can't load, hide rather than show a broken image —
|
||||
// the adjacent title/label carries the meaning.
|
||||
onError={(e) => { e.currentTarget.style.display = 'none'; }}
|
||||
className={cn('inline-block h-10 w-10 shrink-0 select-none rounded-xl object-contain shadow-sm', className)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
function BrandGlyphShape({ name, gradientId }: { name: BrandGlyphName; gradientId: string }) {
|
||||
const fill = `url(#${gradientId})`;
|
||||
const commonStroke = 'currentColor';
|
||||
|
||||
switch (name) {
|
||||
case 'bills':
|
||||
return (
|
||||
<>
|
||||
<path d="M13 8h22v31l-4-3-4 3-4-3-4 3-4-3-2 1.5V8Z" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinejoin="round" opacity="0.88" />
|
||||
<path d="M18 17h12M18 24h10M18 31h7" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" opacity="0.72" />
|
||||
<path d="m27 17 3 3 7-8" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'calendar':
|
||||
return (
|
||||
<>
|
||||
<rect x="10" y="12" width="28" height="27" rx="5" fill="none" stroke={commonStroke} strokeWidth="3" />
|
||||
<path d="M16 8v8M32 8v8M11 20h26" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" />
|
||||
<rect x="17" y="25" width="6" height="6" rx="2" fill={fill} />
|
||||
<path d="m25 31 3 3 7-8" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'analytics':
|
||||
return (
|
||||
<>
|
||||
<path d="M10 36h30" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" opacity="0.35" />
|
||||
<path d="M12 31 20 23l7 5 10-14" fill="none" stroke={commonStroke} strokeWidth="4" strokeLinecap="round" strokeLinejoin="round" />
|
||||
<circle cx="20" cy="23" r="3" fill={fill} />
|
||||
<circle cx="37" cy="14" r="3" fill={fill} />
|
||||
</>
|
||||
);
|
||||
case 'data':
|
||||
return (
|
||||
<>
|
||||
<ellipse cx="24" cy="13" rx="13" ry="5" fill="none" stroke={commonStroke} strokeWidth="3" />
|
||||
<path d="M11 13v19c0 2.8 5.8 5 13 5s13-2.2 13-5V13" fill="none" stroke={commonStroke} strokeWidth="3" />
|
||||
<path d="M11 22c0 2.8 5.8 5 13 5s13-2.2 13-5M11 31c0 2.8 5.8 5 13 5s13-2.2 13-5" stroke={commonStroke} strokeWidth="3" opacity="0.58" />
|
||||
<path d="m29 23 3 3 7-8" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'banking':
|
||||
return (
|
||||
<>
|
||||
<path d="M8 18 24 9l16 9H8Z" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinejoin="round" />
|
||||
<path d="M13 21v13M21 21v13M29 21v13M37 21v13M10 38h28" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" />
|
||||
<path d="m28 29 3 3 7-8" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'snowball':
|
||||
return (
|
||||
<>
|
||||
<circle cx="17" cy="18" r="5" fill="none" stroke={commonStroke} strokeWidth="3" opacity="0.74" />
|
||||
<circle cx="29" cy="28" r="9" fill="none" stroke={commonStroke} strokeWidth="3" />
|
||||
<path d="M11 37c8-1 18-7 27-23" fill="none" stroke={commonStroke} strokeWidth="4" strokeLinecap="round" />
|
||||
<path d="m31 14 7 0 0 7" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'security':
|
||||
return (
|
||||
<>
|
||||
<path d="M24 8 37 13v10c0 8-5 14-13 17-8-3-13-9-13-17V13l13-5Z" fill="none" stroke={commonStroke} strokeWidth="3" strokeLinejoin="round" />
|
||||
<path d="m18 24 4 4 9-10" fill="none" stroke={commonStroke} strokeWidth="4" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
case 'tracker':
|
||||
default:
|
||||
return (
|
||||
<>
|
||||
<rect x="10" y="25" width="7" height="13" rx="2.5" fill={fill} opacity="0.78" />
|
||||
<rect x="20.5" y="18" width="7" height="20" rx="2.5" fill={fill} opacity="0.88" />
|
||||
<rect x="31" y="11" width="7" height="27" rx="2.5" fill={fill} />
|
||||
<path d="m10 15 8 8 20-16" fill="none" stroke={commonStroke} strokeWidth="4" strokeLinecap="round" strokeLinejoin="round" />
|
||||
</>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,7 +46,7 @@ export function PageHeader({
|
|||
return (
|
||||
<section className={cn('flex flex-col gap-4 sm:flex-row sm:items-end sm:justify-between', className)}>
|
||||
<div className="flex min-w-0 items-start gap-3">
|
||||
{glyph && <BrandGlyph name={glyph} className="mt-0.5 hidden sm:inline-grid" />}
|
||||
{glyph && <BrandGlyph name={glyph} className="mt-0.5 hidden sm:inline-block" />}
|
||||
{Icon && !glyph && (
|
||||
<span className="mt-0.5 hidden h-10 w-10 shrink-0 place-items-center rounded-xl border border-primary/20 bg-primary/[0.08] text-primary shadow-sm shadow-primary/10 sm:grid">
|
||||
<Icon className="h-5 w-5" />
|
||||
|
|
|
|||
|
|
@ -0,0 +1,29 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync, existsSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { dirname, resolve } from 'node:path';
|
||||
import { BRAND, BRAND_GLYPHS } from './brand';
|
||||
|
||||
// Assets are referenced by absolute URL (/brand/...) and served from client/public.
|
||||
const publicDir = resolve(dirname(fileURLToPath(import.meta.url)), '../public');
|
||||
const toFile = (assetUrl: string) => resolve(publicDir, assetUrl.replace(/^\//, ''));
|
||||
|
||||
describe('brand glyph assets', () => {
|
||||
// TS proves each BrandGlyphName has an asset path; it can't prove the FILE exists.
|
||||
// This is the exact failure mode of "images added but the path is wrong / renamed".
|
||||
it('every glyph resolves to a real, non-empty SVG file on disk', () => {
|
||||
for (const [name, glyph] of Object.entries(BRAND_GLYPHS)) {
|
||||
expect(glyph.asset, `${name} asset path shape`).toMatch(/^\/brand\/glyphs\/.+\.svg$/);
|
||||
const file = toFile(glyph.asset);
|
||||
expect(existsSync(file), `${name} → ${glyph.asset} missing on disk`).toBe(true);
|
||||
expect(readFileSync(file, 'utf8'), `${name} is a valid <svg>`).toMatch(/<svg[\s>]/);
|
||||
}
|
||||
});
|
||||
|
||||
it('every /brand asset referenced in brand.ts exists on disk', () => {
|
||||
for (const [key, url] of Object.entries(BRAND.assets)) {
|
||||
if (!url.startsWith('/brand/')) continue; // skip third-party (/img/) icons
|
||||
expect(existsSync(toFile(url)), `${key} → ${url} missing on disk`).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -19,7 +19,6 @@ export const BRAND = {
|
|||
socialPreview: '/brand/social-preview.svg',
|
||||
socialPreviewPng: '/brand/social-preview.png',
|
||||
emptyState: '/brand/empty-state.svg',
|
||||
legacyLogo: '/img/logo.png',
|
||||
},
|
||||
colors: {
|
||||
ink: '#101417',
|
||||
|
|
@ -51,5 +50,3 @@ export const BRAND_GLYPHS: Record<BrandGlyphName, { label: string; asset: string
|
|||
snowball: { label: 'Snowball', asset: '/brand/glyphs/snowball-payoff.svg' },
|
||||
security: { label: 'Security', asset: '/brand/glyphs/admin-security.svg' },
|
||||
};
|
||||
|
||||
export const ADMIN_GLYPH = { label: 'Admin' } as const;
|
||||
|
|
|
|||
|
|
@ -115,6 +115,6 @@ export default defineConfig({
|
|||
// `npm run test:client`; `npm run test:all` runs both.
|
||||
test: {
|
||||
environment: 'node', // hook/component tests opt into jsdom via @vitest-environment
|
||||
include: ['client/**/*.test.{js,jsx}'],
|
||||
include: ['client/**/*.test.{js,jsx,ts,tsx}'],
|
||||
},
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue