diff --git a/e2e/project-cards.spec.ts b/e2e/project-cards.spec.ts index fe83f39d..7fc84b31 100644 --- a/e2e/project-cards.spec.ts +++ b/e2e/project-cards.spec.ts @@ -221,7 +221,7 @@ test.describe('Project Cards', () => { .locator('[data-component="secondary-project-card"]') .first(); await expect(card).toHaveClass(/shadow-sm/); - await expect(card).toHaveClass(/hover:shadow-md/); + await expect(card).toHaveClass(/hover:shadow-lg/); }); test('cards have smooth transition', async ({page}) => { diff --git a/simplify.md b/simplify.md deleted file mode 100644 index 382014a6..00000000 --- a/simplify.md +++ /dev/null @@ -1,1284 +0,0 @@ -# Code Simplification Implementation Plan - -This document provides a comprehensive, step-by-step plan to address all identified code simplification issues in the cbenge509.github.io portfolio project. - -## Pre-Implementation Setup - -### Create Git Branch - -```bash -git checkout -b simplification -``` - ---- - -## HIGH PRIORITY - -### 1. Remove Unused Types File (`src/types/index.ts`) - -**Scope:** Small (1 file deletion, verification of no imports) -**Estimated Time:** 5 minutes - -**Issue:** The `src/types/index.ts` file defines types (`Theme`, `NavItem`, `SocialLink`) that are not imported anywhere in the codebase. The `SocialLink` type is already defined locally in `src/data/profile.ts` where it's actually used. - -**Current State (`src/types/index.ts` - lines 1-27):** -```typescript -/** - * Shared TypeScript type definitions - */ - -/** - * Theme type for light/dark mode - */ -export type Theme = 'light' | 'dark'; - -/** - * Navigation item type - */ -export interface NavItem { - label: string; - href: string; - isExternal?: boolean; -} - -/** - * Social link type - */ -export interface SocialLink { - platform: string; - url: string; - icon: string; -} -``` - -**Actions:** - -- [x] Verify no imports exist from `src/types/index.ts`: - ```bash - grep -r "from ['\"].*types['\"]" src/ - grep -r "from ['\"].*types/index['\"]" src/ - ``` - -- [x] Delete the unused types file: - ```bash - rm src/types/index.ts - ``` - -- [x] Remove the `src/types/` directory if empty: - ```bash - rmdir src/types 2>/dev/null || true - ``` - -**Verification:** - -- [x] Run TypeScript check: `npx tsc --noEmit --project tsconfig.ci.json` -- [x] Run tests: `npm run test` -- [x] Run build: `npm run build` - ---- - -## MEDIUM PRIORITY - -### 2. Remove Unused `EXTERNAL_LINK_CLASSES` Export - -**Scope:** Small (modify 2 files) -**Estimated Time:** 10 minutes - -**Issue:** `EXTERNAL_LINK_CLASSES` is exported from `src/utils/card-styles.ts` but never imported by any component. The `ExternalLink.astro` component handles all external link styling internally with its own variant system. - -**Current State (`src/utils/card-styles.ts` - lines 69-86):** -```typescript -/** - * External link classes for consistent styling. - * Provides standard accent color, hover underline, and focus ring. - * - * @example - * ```astro - * - * Link text - * - * ``` - */ -export const EXTERNAL_LINK_CLASSES = - 'inline-flex items-center gap-1 text-sm text-accent dark:text-accent-dark hover:underline focus-ring rounded' as const; -``` - -**Actions:** - -- [x] Remove `EXTERNAL_LINK_CLASSES` export from `src/utils/card-styles.ts` (lines 69-86) - -- [x] Update test file `src/utils/card-styles.test.ts`: - - Remove import of `EXTERNAL_LINK_CLASSES` (line 5) - - Remove entire `describe('EXTERNAL_LINK_CLASSES', ...)` block (lines 77-92) - -**After (`src/utils/card-styles.ts`):** -```typescript -import {cn} from './cn'; - -/** - * Card styling utilities for consistent card component appearance. - * Provides reusable class compositions for card containers and common patterns. - */ - -/** - * Standard card container classes used across card components. - * ... (existing cardContainerClasses function) - */ -export function cardContainerClasses(className?: string): string { - // ... existing implementation -} - -/** - * Card container classes for cards using surface colors. - * ... (existing cardSurfaceContainerClasses function) - */ -export function cardSurfaceContainerClasses(className?: string): string { - // ... existing implementation -} - -/** - * Bullet separator for metadata sections. - * ... (existing BULLET_SEPARATOR constant) - */ -export const BULLET_SEPARATOR = '' as const; -``` - -**After (`src/utils/card-styles.test.ts` - imports):** -```typescript -import {describe, it, expect} from 'vitest'; -import { - cardContainerClasses, - cardSurfaceContainerClasses, - BULLET_SEPARATOR, -} from './card-styles'; -``` - -**Verification:** - -- [x] Run unit tests: `npm run test` -- [x] Run lint: `npm run lint` - ---- - -### 3. Consolidate Card Container Functions - -**Scope:** Medium (1 utility file, 4+ component files) -**Estimated Time:** 30 minutes - -**Issue:** Two card container functions exist with subtle differences: -- `cardContainerClasses()` - uses `bg-white`/`dark:bg-gray-900/50`, `hover:-translate-y-1` -- `cardSurfaceContainerClasses()` - uses `bg-surface`/`dark:bg-surface-dark`, no translate - -However, after reviewing component usage, both functions serve intentionally different purposes. The real issue is that neither is consistently used - most card components define their own inline classes. - -**Decision:** Keep both functions but document their distinct purposes more clearly. This is intentional design, not duplication. - -**Actions:** - -- [x] Update JSDoc comments in `src/utils/card-styles.ts` to clarify when to use each: - -**Before (`src/utils/card-styles.ts` - lines 8-26):** -```typescript -/** - * Standard card container classes used across card components. - * Includes rounded corners, background, border, padding, and hover effects. - * Uses white background with gray-900 for dark mode. - * - * @param className - Optional additional classes to merge - * @returns Merged class string with card base styles - * ... - */ -``` - -**After:** -```typescript -/** - * Card container with elevated hover effect (lift + shadow). - * Best for interactive cards that benefit from visual lift feedback. - * Uses white/gray-900 background for maximum contrast. - * - * Use this for: Interactive cards, clickable items, cards in grids - * - * @param className - Optional additional classes to merge - * @returns Merged class string with card base styles - * ... - */ -``` - -**Before (`src/utils/card-styles.ts` - lines 39-56):** -```typescript -/** - * Card container classes for cards using surface colors. - * Used by PatentCard, PublicationCard, and project cards. - * Surface colors provide semantic alternatives to white/gray-900. - * ... - */ -``` - -**After:** -```typescript -/** - * Card container with subtle hover effect (shadow only, no lift). - * Best for content cards where lift animation would be distracting. - * Uses surface tokens for semantic color theming. - * - * Use this for: Content-heavy cards, publication cards, patent cards - * - * @param className - Optional additional classes to merge - * @returns Merged class string with surface-based card styles - * ... - */ -``` - -- [x] No code changes needed - this was intentional design - -**Verification:** - -- [x] Run tests: `npm run test` - ---- - -### 4. Fix Project Card Props Type Mismatch - -**Scope:** Small (2 component files) -**Estimated Time:** 15 minutes - -**Issue:** `FeaturedProjectCard.astro` and `SecondaryProjectCard.astro` define slightly different `Props.project.image` types: -- `FeaturedProjectCard`: `image: ImageMetadata` -- `SecondaryProjectCard`: `image: string` (plus separate `imageImport?: ImageMetadata`) - -This inconsistency requires different prop patterns when using the components. - -**Current State (`src/components/FeaturedProjectCard.astro` - lines 29-44):** -```typescript -export interface Props extends HTMLAttributes<'article'> { - /** Project data from content collection */ - project: { - title: string; - description: string; - image: ImageMetadata; // <-- ImageMetadata - category: 'leader' | 'builder' | 'winner' | 'research'; - skills: string[]; - tools: string[]; - githubUrl?: string; - achievement?: string; - affiliation?: string; - }; - /** Project slug for linking to detail page */ - slug: string; -} -``` - -**Current State (`src/components/SecondaryProjectCard.astro` - lines 32-49):** -```typescript -export interface Props extends HTMLAttributes<'article'> { - /** Project data from content collection */ - project: { - title: string; - description: string; - image: string; // <-- string (different!) - category: 'leader' | 'builder' | 'winner' | 'research'; - skills: string[]; - tools: string[]; - githubUrl?: string; - achievement?: string; - affiliation?: string; - }; - /** Project slug for linking to detail page */ - slug: string; - /** Image import for Astro optimization (if available) */ - imageImport?: ImageMetadata; // <-- separate prop -} -``` - -**Actions:** - -- [x] Create shared type in `src/utils/project-categories.ts` (already exists, good location): - -**Add to `src/utils/project-categories.ts`:** -```typescript -/** - * Project data shape from content collection. - * Used by FeaturedProjectCard and SecondaryProjectCard components. - */ -export interface ProjectData { - title: string; - description: string; - image: ImageMetadata; - category: 'leader' | 'builder' | 'winner' | 'research'; - skills: string[]; - tools: string[]; - githubUrl?: string; - achievement?: string; - affiliation?: string; -} -``` - -- [x] Update `FeaturedProjectCard.astro` Props (lines 29-44): - -**After:** -```typescript -import type {HTMLAttributes} from 'astro/types'; -import {Image} from 'astro:assets'; -import ProjectTag from './ProjectTag.astro'; -import ExternalLink from './ExternalLink.astro'; -import placeholderImage from '../assets/images/projects/placeholder.svg'; -import {CATEGORY_TO_VARIANT, CATEGORY_LABELS, type ProjectData} from '../utils/project-categories'; - -export interface Props extends HTMLAttributes<'article'> { - /** Project data from content collection */ - project: ProjectData; - /** Project slug for linking to detail page */ - slug: string; -} -``` - -- [x] Update `SecondaryProjectCard.astro` Props (lines 32-49): - -**After:** -```typescript -import type {HTMLAttributes} from 'astro/types'; -import {Image} from 'astro:assets'; -import ProjectTag from './ProjectTag.astro'; -import placeholderImage from '../assets/images/projects/placeholder.svg'; -import {CATEGORY_TO_VARIANT, CATEGORY_LABELS, type ProjectData} from '../utils/project-categories'; - -export interface Props extends HTMLAttributes<'article'> { - /** Project data from content collection */ - project: ProjectData; - /** Project slug for linking to detail page */ - slug: string; -} -``` - -- [x] Update `SecondaryProjectCard.astro` to use `project.image` directly (line 51-55): - -**Before:** -```typescript -const {project, slug, imageImport, class: className, ...attrs} = Astro.props; - -// Use imported image if available, otherwise fallback to placeholder -const imageSource = imageImport || placeholderImage; -const hasValidImage = !!imageImport || (project.image && project.image !== ''); -``` - -**After:** -```typescript -const {project, slug, class: className, ...attrs} = Astro.props; - -// Use project image directly (now ImageMetadata from content collection) -const imageSource = project.image || placeholderImage; -const hasValidImage = !!project.image; -``` - -- [x] Update any usages of `SecondaryProjectCard` that pass `imageImport` prop (check `src/pages/projects/index.astro`) - -**Verification:** - -- [x] Run TypeScript check: `npx tsc --noEmit --project tsconfig.ci.json` -- [x] Run tests: `npm run test` -- [x] Run build: `npm run build` -- [x] Visually verify projects page: `npm run dev` and check `/projects` - ---- - -### 5. Extract Breadcrumb Navigation Component - -**Scope:** Medium (create 1 component, update 2 pages) -**Estimated Time:** 25 minutes - -**Issue:** Identical breadcrumb navigation markup is duplicated in: -- `src/pages/about.astro` (lines 54-67) -- `src/pages/publications.astro` (lines 51-68) - -**Current State (both files have nearly identical code):** -```astro - - -``` - -**Actions:** - -- [x] Create new component `src/components/Breadcrumb.astro`: - -```astro ---- -import type {HTMLAttributes} from 'astro/types'; - -/** - * Breadcrumb navigation component for consistent page hierarchy display. - * Renders Home link followed by current page name. - * - * @example - * ```astro - * - * - * ``` - */ -interface Props extends HTMLAttributes<'nav'> { - /** Current page name to display */ - currentPage: string; -} - -const {currentPage, class: className, ...attrs} = Astro.props; ---- - - -``` - -- [x] Create test file `src/components/Breadcrumb.test.ts`: - -```typescript -import {experimental_AstroContainer as AstroContainer} from 'astro/container'; -import {describe, it, expect} from 'vitest'; -import Breadcrumb from './Breadcrumb.astro'; - -describe('Breadcrumb', () => { - it('renders current page name', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'About'}, - }); - expect(result).toContain('About'); - }); - - it('renders Home link', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test'}, - }); - expect(result).toContain('href="/"'); - expect(result).toContain('Home'); - }); - - it('has aria-label for accessibility', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test'}, - }); - expect(result).toContain('aria-label="Breadcrumb"'); - }); - - it('marks current page with aria-current', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Publications'}, - }); - expect(result).toContain('aria-current="page"'); - }); - - it('has focus-ring on Home link', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test'}, - }); - expect(result).toContain('focus-ring'); - }); - - it('has separator with aria-hidden', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test'}, - }); - expect(result).toContain('aria-hidden="true"'); - expect(result).toContain('/'); - }); - - it('applies custom class', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test', class: 'custom-breadcrumb'}, - }); - expect(result).toContain('custom-breadcrumb'); - }); - - it('has minimum touch target on Home link', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Breadcrumb, { - props: {currentPage: 'Test'}, - }); - expect(result).toContain('min-h-11'); - }); -}); -``` - -- [x] Update `src/pages/about.astro` (replace lines 54-67): - -**Before:** -```astro - - -``` - -**After:** -```astro - -``` - -- [x] Add import to `src/pages/about.astro` frontmatter: -```astro -import Breadcrumb from '../components/Breadcrumb.astro'; -``` - -- [x] Update `src/pages/publications.astro` (replace lines 51-68): - -**After:** -```astro - -``` - -- [x] Add import to `src/pages/publications.astro` frontmatter: -```astro -import Breadcrumb from '../components/Breadcrumb.astro'; -``` - -**Verification:** - -- [x] Run tests: `npm run test` -- [x] Run build: `npm run build` -- [x] Visually verify: `npm run dev` and check `/about` and `/publications` - ---- - -## LOW PRIORITY - -### 6. Create Collection Sorting Utilities - -**Scope:** Medium (create 1 utility file, update 4 pages) -**Estimated Time:** 30 minutes - -**Issue:** Similar sorting patterns are repeated across pages: -- `about.astro`: `.sort((a, b) => a.data.order - b.data.order)` (3 times) -- `publications.astro`: order-based and year-based sorting -- `index.astro`: `.sort((a, b) => (a.data.order ?? 999) - (b.data.order ?? 999))` -- `projects/index.astro`: order and date sorting - -**Actions:** - -- [x] Create `src/utils/collection-sort.ts`: - -```typescript -/** - * Collection sorting utilities for consistent ordering across pages. - * Provides reusable comparators for content collection items. - */ - -interface WithOrder { - data: {order?: number}; -} - -interface WithYear { - data: {year: number}; -} - -interface WithDate { - data: {publishDate: Date}; -} - -interface WithGrantOrFilingDate { - data: {grantDate?: Date; filingDate: Date}; -} - -/** - * Sort by order field (ascending). Items without order go last. - * @example sortedItems = items.sort(byOrder); - */ -export function byOrder(a: T, b: T): number { - return (a.data.order ?? 999) - (b.data.order ?? 999); -} - -/** - * Sort by year field (descending - newest first). - * @example sortedItems = items.sort(byYearDesc); - */ -export function byYearDesc(a: T, b: T): number { - return b.data.year - a.data.year; -} - -/** - * Sort by publishDate (descending - newest first). - * @example sortedItems = items.sort(byPublishDateDesc); - */ -export function byPublishDateDesc(a: T, b: T): number { - return b.data.publishDate.getTime() - a.data.publishDate.getTime(); -} - -/** - * Sort by grant date or filing date (descending - newest first). - * Uses grantDate if present, otherwise filingDate. - * @example sortedPatents = patents.sort(byPatentDateDesc); - */ -export function byPatentDateDesc( - a: T, - b: T -): number { - const dateA = a.data.grantDate || a.data.filingDate; - const dateB = b.data.grantDate || b.data.filingDate; - return dateB.getTime() - dateA.getTime(); -} - -/** - * Sort by order if both have it, otherwise by year (descending). - * @example sortedPubs = publications.sort(byOrderOrYearDesc); - */ -export function byOrderOrYearDesc( - a: T, - b: T -): number { - if (a.data.order !== undefined && b.data.order !== undefined) { - return a.data.order - b.data.order; - } - return b.data.year - a.data.year; -} -``` - -- [x] Create `src/utils/collection-sort.test.ts`: - -```typescript -import {describe, it, expect} from 'vitest'; -import { - byOrder, - byYearDesc, - byPublishDateDesc, - byPatentDateDesc, - byOrderOrYearDesc, -} from './collection-sort'; - -describe('collection-sort utilities', () => { - describe('byOrder', () => { - it('sorts by order ascending', () => { - const items = [ - {data: {order: 2}}, - {data: {order: 1}}, - {data: {order: 3}}, - ]; - const sorted = items.sort(byOrder); - expect(sorted.map((i) => i.data.order)).toEqual([1, 2, 3]); - }); - - it('puts items without order last', () => { - const items = [ - {data: {order: 2}}, - {data: {}}, - {data: {order: 1}}, - ]; - const sorted = items.sort(byOrder); - expect(sorted.map((i) => i.data.order)).toEqual([1, 2, undefined]); - }); - }); - - describe('byYearDesc', () => { - it('sorts by year descending', () => { - const items = [ - {data: {year: 2022}}, - {data: {year: 2024}}, - {data: {year: 2023}}, - ]; - const sorted = items.sort(byYearDesc); - expect(sorted.map((i) => i.data.year)).toEqual([2024, 2023, 2022]); - }); - }); - - describe('byPublishDateDesc', () => { - it('sorts by publish date descending', () => { - const items = [ - {data: {publishDate: new Date('2023-01-15')}}, - {data: {publishDate: new Date('2024-06-01')}}, - {data: {publishDate: new Date('2023-12-01')}}, - ]; - const sorted = items.sort(byPublishDateDesc); - expect(sorted[0].data.publishDate.getFullYear()).toBe(2024); - expect(sorted[2].data.publishDate.getFullYear()).toBe(2023); - }); - }); - - describe('byPatentDateDesc', () => { - it('uses grantDate when present', () => { - const items = [ - {data: {filingDate: new Date('2020-01-01'), grantDate: new Date('2023-06-01')}}, - {data: {filingDate: new Date('2022-01-01'), grantDate: new Date('2024-01-01')}}, - ]; - const sorted = items.sort(byPatentDateDesc); - expect(sorted[0].data.grantDate?.getFullYear()).toBe(2024); - }); - - it('falls back to filingDate when grantDate missing', () => { - const items = [ - {data: {filingDate: new Date('2022-01-01')}}, - {data: {filingDate: new Date('2023-06-01')}}, - ]; - const sorted = items.sort(byPatentDateDesc); - expect(sorted[0].data.filingDate.getFullYear()).toBe(2023); - }); - }); - - describe('byOrderOrYearDesc', () => { - it('uses order when both have it', () => { - const items = [ - {data: {order: 2, year: 2024}}, - {data: {order: 1, year: 2022}}, - ]; - const sorted = items.sort(byOrderOrYearDesc); - expect(sorted[0].data.order).toBe(1); - }); - - it('falls back to year when order missing', () => { - const items = [ - {data: {year: 2022}}, - {data: {year: 2024}}, - ]; - const sorted = items.sort(byOrderOrYearDesc); - expect(sorted[0].data.year).toBe(2024); - }); - }); -}); -``` - -- [x] Update `src/pages/about.astro` to use utilities: - -**Before (lines 21-33):** -```typescript -const sortedEducation = educationEntries.sort( - (a, b) => a.data.order - b.data.order -); -// ... -const sortedCertifications = certificationEntries.sort( - (a, b) => a.data.order - b.data.order -); -// ... -const sortedAwards = awardEntries.sort((a, b) => a.data.order - b.data.order); -``` - -**After:** -```typescript -import {byOrder} from '../utils/collection-sort'; -// ... -const sortedEducation = educationEntries.sort(byOrder); -const sortedCertifications = certificationEntries.sort(byOrder); -const sortedAwards = awardEntries.sort(byOrder); -``` - -- [x] Update `src/pages/publications.astro`: - -**Before (lines 20-37):** -```typescript -const sortedPublications = publicationEntries.sort((a, b) => { - if (a.data.order !== undefined && b.data.order !== undefined) { - return a.data.order - b.data.order; - } - return b.data.year - a.data.year; -}); -// ... -const sortedPatents = patentEntries.sort((a, b) => { - const dateA = a.data.grantDate || a.data.filingDate; - const dateB = b.data.grantDate || b.data.filingDate; - return dateB.getTime() - dateA.getTime(); -}); -``` - -**After:** -```typescript -import {byOrderOrYearDesc, byPatentDateDesc} from '../utils/collection-sort'; -// ... -const sortedPublications = publicationEntries.sort(byOrderOrYearDesc); -const sortedPatents = patentEntries.sort(byPatentDateDesc); -``` - -- [x] Update `src/pages/index.astro`: - -**Before (lines 10-13):** -```typescript -const featuredProjects = allProjects - .filter((project) => project.data.isFeatured) - .sort((a, b) => (a.data.order ?? 999) - (b.data.order ?? 999)) - .slice(0, 3); -``` - -**After:** -```typescript -import {byOrder} from '../utils/collection-sort'; -// ... -const featuredProjects = allProjects - .filter((project) => project.data.isFeatured) - .sort(byOrder) - .slice(0, 3); -``` - -- [x] Update `src/pages/projects/index.astro`: - -**Before (lines 23-29):** -```typescript -const featuredProjects = allProjects - .filter((p) => p.data.isFeatured) - .sort((a, b) => (a.data.order ?? 999) - (b.data.order ?? 999)); - -const secondaryProjects = allProjects - .filter((p) => !p.data.isFeatured) - .sort((a, b) => b.data.publishDate.getTime() - a.data.publishDate.getTime()); -``` - -**After:** -```typescript -import {byOrder, byPublishDateDesc} from '../../utils/collection-sort'; -// ... -const featuredProjects = allProjects - .filter((p) => p.data.isFeatured) - .sort(byOrder); - -const secondaryProjects = allProjects - .filter((p) => !p.data.isFeatured) - .sort(byPublishDateDesc); -``` - -**Verification:** - -- [x] Run tests: `npm run test` -- [x] Run build: `npm run build` -- [x] Check all pages load correctly - ---- - -### 7. Simplify Image Fallback Logic in Project Cards - -**Scope:** Small (2 component files) -**Estimated Time:** 15 minutes - -**Issue:** Both project cards have similar but slightly different image fallback logic that can be simplified. - -**Current State (`FeaturedProjectCard.astro` - lines 48-51):** -```typescript -// Use project image directly (now ImageMetadata from content collection) -const imageSource = project.image || placeholderImage; -const hasValidImage = !!project.image; -``` - -**Current State (`SecondaryProjectCard.astro` - lines 53-55):** -```typescript -// Use imported image if available, otherwise fallback to placeholder -const imageSource = imageImport || placeholderImage; -const hasValidImage = !!imageImport || (project.image && project.image !== ''); -``` - -**Actions:** - -After completing item #4 (fixing props type mismatch), both components will have the same pattern. No additional changes needed here - this will be resolved by item #4. - -- [x] Verify both components have identical image handling after item #4 is complete - -**Verification:** - -- [x] Visual check that placeholder images work: `npm run dev` - ---- - -### 8. Create Test Helper for AstroContainer - -**Scope:** Medium (create 1 utility, update 16 test files) -**Estimated Time:** 45 minutes - -**Issue:** Every test file repeats `await AstroContainer.create()` multiple times. A helper would reduce boilerplate and make tests more consistent. - -**Actions:** - -- [x] Create `test/helpers/astro-container.ts`: - -```typescript -import {experimental_AstroContainer as AstroContainer} from 'astro/container'; - -/** - * Shared AstroContainer instance for unit tests. - * Reduces boilerplate and ensures consistent test setup. - */ -let containerInstance: AstroContainer | null = null; - -/** - * Get or create an AstroContainer instance. - * The container is cached for performance within a test run. - * - * @example - * const container = await getTestContainer(); - * const result = await container.renderToString(MyComponent, { props: {...} }); - */ -export async function getTestContainer(): Promise { - if (!containerInstance) { - containerInstance = await AstroContainer.create(); - } - return containerInstance; -} - -/** - * Render a component to string using the shared container. - * Convenience method that combines container creation and rendering. - * - * @example - * const html = await renderComponent(Hero); - * const html = await renderComponent(Card, { props: { title: 'Test' } }); - */ -export async function renderComponent( - Component: T, - options?: {props?: Record; slots?: Record} -): Promise { - const container = await getTestContainer(); - return container.renderToString(Component as Parameters[0], options); -} - -/** - * Reset the container instance (useful for test isolation if needed). - */ -export function resetTestContainer(): void { - containerInstance = null; -} -``` - -- [x] Update `vitest.config.ts` to include the helper path (if not already): - -The helpers should be importable as `test/helpers/astro-container` - verify this works with current config. - -- [x] Refactor one test file as example (`src/components/Hero.test.ts`): - -**Before:** -```typescript -import {experimental_AstroContainer as AstroContainer} from 'astro/container'; -import {describe, it, expect} from 'vitest'; -import Hero from './Hero.astro'; - -describe('Hero', () => { - it('renders name correctly', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Hero); - expect(result).toContain('Cris Benge'); - }); - - it('renders role correctly', async () => { - const container = await AstroContainer.create(); - const result = await container.renderToString(Hero); - expect(result).toContain('Head of Federal Innovation, Google'); - }); - // ... many more tests each creating container -}); -``` - -**After:** -```typescript -import {describe, it, expect} from 'vitest'; -import {renderComponent} from '../../test/helpers/astro-container'; -import Hero from './Hero.astro'; - -describe('Hero', () => { - it('renders name correctly', async () => { - const result = await renderComponent(Hero); - expect(result).toContain('Cris Benge'); - }); - - it('renders role correctly', async () => { - const result = await renderComponent(Hero); - expect(result).toContain('Head of Federal Innovation, Google'); - }); - - it('applies custom class when provided', async () => { - const result = await renderComponent(Hero, { - props: {class: 'custom-class'}, - }); - expect(result).toContain('custom-class'); - }); - // ... cleaner tests -}); -``` - -- [x] Update remaining test files (list of 16 files to update): - 1. `src/components/AwardCard.test.ts` - 2. `src/components/CertificationCard.test.ts` - 3. `src/components/ContactSection.test.ts` - 4. `src/components/EducationCard.test.ts` - 5. `src/components/ExternalLink.test.ts` - 6. `src/components/FeaturedProjectCard.test.ts` - 7. `src/components/Footer.test.ts` - 8. `src/components/Hero.test.ts` - 9. `src/components/Navigation.test.ts` - 10. `src/components/PatentCard.test.ts` - 11. `src/components/ProjectTag.test.ts` - 12. `src/components/PublicationCard.test.ts` - 13. `src/components/SecondaryProjectCard.test.ts` - 14. `src/components/SectionHeading.test.ts` - 15. `src/components/SEO.test.ts` - 16. `src/components/ThemeToggle.test.ts` - -**Note:** This is optional refactoring. The current pattern works fine and is explicit. Consider whether the abstraction adds enough value. - -**Verification:** - -- [x] Run all tests: `npm run test` -- [x] Run tests with coverage: `npm run test:coverage` - ---- - -### 9. Extract Profile Data from Hero Component - -**Scope:** Small (1 component, 1 data file) -**Estimated Time:** 15 minutes - -**Issue:** Profile data (name, role, credentials) is hardcoded in `Hero.astro` lines 34-43. This could be moved to the existing `src/data/profile.ts` for centralization. - -**Current State (`src/components/Hero.astro` - lines 34-43):** -```typescript -// Profile data - could be externalized to content collection later -const profile = { - name: 'Cris Benge', - role: 'Head of Federal Innovation, Google', - credentials: [ - {label: 'Senior Leadership', type: 'education' as const}, - {label: 'Research', type: 'education' as const}, - {label: 'Data Science', type: 'education' as const}, - {label: 'TS/SCI w/ Polygraph', type: 'clearance' as const}, - ], -}; -``` - -**Actions:** - -- [x] Add profile data to `src/data/profile.ts`: - -```typescript -/** - * Professional credentials displayed in Hero component. - */ -export interface Credential { - label: string; - type: 'education' | 'clearance'; -} - -/** - * Hero section profile data. - */ -export const HERO_PROFILE = { - name: 'Cris Benge', - role: 'Head of Federal Innovation, Google', - credentials: [ - {label: 'Senior Leadership', type: 'education'}, - {label: 'Research', type: 'education'}, - {label: 'Data Science', type: 'education'}, - {label: 'TS/SCI w/ Polygraph', type: 'clearance'}, - ] satisfies Credential[], -} as const; -``` - -- [x] Update `src/components/Hero.astro` frontmatter: - -**Before:** -```typescript -import {getSocialLink} from '../data/profile'; - -// Profile data - could be externalized to content collection later -const profile = { - name: 'Cris Benge', - // ... -}; -``` - -**After:** -```typescript -import {getSocialLink, HERO_PROFILE} from '../data/profile'; - -const profile = HERO_PROFILE; -``` - -- [x] Update Hero tests to verify data comes from centralized source (optional - current tests still pass) - -**Verification:** - -- [x] Run tests: `npm run test` -- [x] Visual check: `npm run dev` and verify Hero displays correctly - ---- - -### 10. Audit and Document CSS Utilities in global.css - -**Scope:** Small (documentation only) -**Estimated Time:** 15 minutes - -**Issue:** Some CSS utilities in `global.css` may be unused. However, after review, all utilities appear to be in use: -- Typography utilities (`.text-display-lg`, etc.) - used in components -- Focus ring utilities (`.focus-ring`) - used throughout -- Animation utilities - used for scroll reveal -- Layout utilities (`.container-custom`) - used in pages -- Line clamp utilities - used in cards -- Card hover utilities - available for use -- Link underline gradient - used in Footer - -**Actions:** - -- [x] Verify utility usage with grep: - ```bash - grep -r "text-display-lg" src/ - grep -r "container-custom" src/ - grep -r "line-clamp-" src/ - grep -r "card-hover" src/ - grep -r "link-underline-gradient" src/ - ``` - -- [x] Document any utilities that are defined but not yet used (keep for future use) - -- [x] No removals recommended - all utilities are either in use or designed for future use - -**Verification:** - -- [x] Build completes: `npm run build` - ---- - -### 11. Simplify ExternalLink Variant Logic - -**Scope:** Medium (1 component file) -**Estimated Time:** 20 minutes - -**Issue:** `ExternalLink.astro` has complex variant logic with multiple size class mappings. This is intentional for flexibility but could be slightly simplified. - -**Current complexity (lines 89-173):** -- 5 variant class definitions -- 4 different size class mappings (button, text, badge, default) -- Function to select size classes based on variant - -**Analysis:** After review, this complexity is intentional and provides necessary flexibility for different use cases. The component handles: -- Text links (navigation) -- Buttons (CTAs) -- Outlined buttons (secondary CTAs) -- Card actions (publication links) -- Badges (GitHub badges on project cards) - -**Decision:** Keep current implementation. The complexity maps to real UI requirements. - -**Actions:** - -- [x] Add brief comment explaining the variant system purpose: - -**Add at line 88:** -```typescript -/** - * Variant class definitions. - * Each variant provides a distinct visual style for different contexts: - * - text: Inline text links, navigation items - * - button: Primary CTAs (filled background) - * - button-outlined: Secondary CTAs (border only) - * - card-action: Links within cards (subtle, accent color) - * - badge: Floating badges over images (semi-transparent background) - */ -``` - -- [x] No structural changes needed - -**Verification:** - -- [x] Run tests: `npm run test` - ---- - -## Final Verification Checklist - -After completing all items, run the full verification suite: - -- [x] **Lint:** `npm run lint` -- [x] **Format check:** `npm run check` -- [x] **TypeScript:** `npx tsc --noEmit --project tsconfig.ci.json` -- [x] **Unit tests:** `npm run test` -- [x] **Coverage:** `npm run test:coverage` (ensure 90%+ maintained) -- [x] **Build:** `npm run build` -- [ ] **E2E tests:** `npm run test:e2e` -- [ ] **Visual review:** `npm run preview` and check all pages - -## Commit Strategy - -Commit after each major section to maintain clean history: - -1. `refactor: remove unused types file` (Item 1) -2. `refactor: remove unused EXTERNAL_LINK_CLASSES export` (Item 2) -3. `docs: clarify card container function purposes` (Item 3) -4. `refactor: unify project card props types` (Item 4) -5. `refactor: extract Breadcrumb component` (Item 5) -6. `refactor: create collection sorting utilities` (Item 6) -7. `refactor: add test helper for AstroContainer` (Item 8, if done) -8. `refactor: centralize Hero profile data` (Item 9) -9. `docs: add ExternalLink variant documentation` (Item 11) - -Final commit: -```bash -git add . -git commit -m "chore: complete code simplification pass - -- Remove unused types and exports -- Extract reusable Breadcrumb component -- Create collection sorting utilities -- Unify project card props types -- Improve documentation for utilities - -Co-Authored-By: Claude Opus 4.5 " -``` - -## Summary - -| Priority | Item | Scope | Status | -|----------|------|-------|--------| -| HIGH | Remove unused types file | Small | [x] | -| MEDIUM | Remove EXTERNAL_LINK_CLASSES | Small | [x] | -| MEDIUM | Document card container functions | Small | [x] | -| MEDIUM | Fix project card props mismatch | Small | [x] | -| MEDIUM | Extract Breadcrumb component | Medium | [x] | -| LOW | Create collection sorting utilities | Medium | [x] | -| LOW | Simplify image fallback (via #4) | N/A | [x] | -| LOW | Create test helper | Medium | [x] | -| LOW | Centralize Hero profile data | Small | [x] | -| LOW | Audit CSS utilities | Small | [x] | -| LOW | Document ExternalLink variants | Small | [x] | - -Total estimated time: ~3-4 hours - -**COMPLETED:** All 11 items have been implemented and verified. diff --git a/src/components/CertificationCard.astro b/src/components/CertificationCard.astro index 5c1d823e..3143485c 100644 --- a/src/components/CertificationCard.astro +++ b/src/components/CertificationCard.astro @@ -1,7 +1,7 @@ --- import type {HTMLAttributes} from 'astro/types'; import {cardContainerClasses, BULLET_SEPARATOR} from '../utils/card-styles'; -import {BADGE_BASE_CLASSES, CERTIFICATION_BADGE_COLORS} from '../utils/badge'; +import {BADGE_BASE_CLASSES, CERTIFICATION_BADGE_COLORS, CERTIFICATION_CATEGORY_LABELS} from '../utils/badge'; import {cn} from '../utils/cn'; import ExternalLink from './ExternalLink.astro'; @@ -34,8 +34,6 @@ interface Props extends HTMLAttributes<'article'> { year: number; /** Certification category */ category: 'cloud' | 'data' | 'database' | 'other'; - /** Optional badge/logo URL */ - badgeUrl?: string; /** Optional verification URL */ verificationUrl?: string; } @@ -45,19 +43,10 @@ const { issuer, year, category, - badgeUrl, verificationUrl, class: className, ...attrs } = Astro.props; - -// Category labels for display -const categoryLabels: Record = { - cloud: 'Cloud', - data: 'Data', - database: 'Database', - other: 'Other', -}; ---
= { class={cn(BADGE_BASE_CLASSES, CERTIFICATION_BADGE_COLORS[category])} data-testid="category-badge" > - {categoryLabels[category]} + {CERTIFICATION_CATEGORY_LABELS[category]} diff --git a/src/components/EducationCard.astro b/src/components/EducationCard.astro index 28676898..043ebe5e 100644 --- a/src/components/EducationCard.astro +++ b/src/components/EducationCard.astro @@ -2,7 +2,7 @@ import type {HTMLAttributes} from 'astro/types'; import {Image} from 'astro:assets'; import {cardContainerClasses} from '../utils/card-styles'; -import {BADGE_BASE_CLASSES} from '../utils/badge'; +import {BADGE_BASE_CLASSES, EDUCATION_HONOR_COLORS} from '../utils/badge'; import {cn} from '../utils/cn'; import ExternalLink from './ExternalLink.astro'; @@ -107,7 +107,7 @@ const { honors.length > 0 && (