docs(35): create phase 35 bug-fix plans (3 plans, wave 1 parallel)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
373
.planning/phases/35-bug-fixes/35-02-PLAN.md
Normal file
373
.planning/phases/35-bug-fixes/35-02-PLAN.md
Normal file
@@ -0,0 +1,373 @@
|
||||
---
|
||||
phase: 35-bug-fixes
|
||||
plan: 02
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- src/client/components/GearImage.tsx
|
||||
- src/client/components/ItemCard.tsx
|
||||
- src/client/components/CandidateCard.tsx
|
||||
- src/client/components/GlobalItemCard.tsx
|
||||
autonomous: true
|
||||
requirements:
|
||||
- FIX-03
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "All img elements in GearImage have loading='lazy'"
|
||||
- "ItemCard shows a bg-gray-100 animate-pulse skeleton while the image loads"
|
||||
- "CandidateCard shows a bg-gray-100 animate-pulse skeleton while the image loads"
|
||||
- "GlobalItemCard shows a bg-gray-100 animate-pulse skeleton while the image loads"
|
||||
- "Once loaded, the img fades in via opacity-0 to opacity-100 transition-opacity duration-200"
|
||||
- "When imageUrl is null, the no-image placeholder (category icon on bg-gray-50) is unchanged"
|
||||
artifacts:
|
||||
- path: "src/client/components/GearImage.tsx"
|
||||
provides: "Lazy-loading image component"
|
||||
contains: "loading=\"lazy\""
|
||||
- path: "src/client/components/ItemCard.tsx"
|
||||
provides: "ItemCard with image skeleton"
|
||||
contains: "animate-pulse"
|
||||
- path: "src/client/components/CandidateCard.tsx"
|
||||
provides: "CandidateCard with image skeleton"
|
||||
contains: "animate-pulse"
|
||||
- path: "src/client/components/GlobalItemCard.tsx"
|
||||
provides: "GlobalItemCard with image skeleton"
|
||||
contains: "animate-pulse"
|
||||
key_links:
|
||||
- from: "ItemCard image area"
|
||||
to: "GearImage onLoad callback"
|
||||
via: "loaded useState"
|
||||
pattern: "onLoad.*setLoaded"
|
||||
- from: "skeleton div"
|
||||
to: "loaded state"
|
||||
via: "conditional rendering"
|
||||
pattern: "loaded.*opacity-0.*opacity-100"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Add lazy loading and image skeleton loading states to all card types that display images. The skeleton prevents layout shift and gives users immediate feedback while presigned S3 URLs resolve.
|
||||
|
||||
Purpose: Resolve FIX-03 — slow image loading UX. Images load lazily (browser-native) and show an animated pulse placeholder until loaded.
|
||||
Output: Updated GearImage, ItemCard, CandidateCard, GlobalItemCard.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/35-bug-fixes/35-CONTEXT.md
|
||||
@.planning/phases/35-bug-fixes/35-UI-SPEC.md
|
||||
</context>
|
||||
|
||||
<interfaces>
|
||||
<!-- Key contracts the executor needs. Extracted from codebase. -->
|
||||
|
||||
From src/client/components/GearImage.tsx (current state):
|
||||
```typescript
|
||||
// Three render paths — each has an <img> element that needs loading="lazy":
|
||||
// 1. cover mode: <img src={src} alt={alt} className={`w-full h-full object-cover ${className}`} />
|
||||
// 2. hasCrop mode: <img src={src} alt={alt} className={`w-full h-full object-cover ${className}`} style={{transform...}} />
|
||||
// 3. default mode: <img src={src} alt={alt} className={`w-full h-full object-contain ${className}`} />
|
||||
```
|
||||
|
||||
Image skeleton pattern (from UI-SPEC, matching existing SkeletonGrid in codebase):
|
||||
```tsx
|
||||
// In each card, inside the aspect-[4/3] container when imageUrl is truthy:
|
||||
const [loaded, setLoaded] = useState(false);
|
||||
|
||||
// Render when imageUrl is truthy:
|
||||
<div className="relative w-full h-full">
|
||||
{!loaded && (
|
||||
<div className="absolute inset-0 bg-gray-100 animate-pulse" />
|
||||
)}
|
||||
<GearImage
|
||||
src={imageUrl}
|
||||
alt={name}
|
||||
// ... other props
|
||||
onLoad={() => setLoaded(true)}
|
||||
className={`transition-opacity duration-200 ${loaded ? "opacity-100" : "opacity-0"}`}
|
||||
/>
|
||||
</div>
|
||||
```
|
||||
|
||||
NOTE: GearImage needs to accept and forward an `onLoad` prop and a `className` to the underlying `<img>` element. The `className` prop already exists on GearImage — it is forwarded to `<img>`. The `onLoad` prop does NOT currently exist — it must be added to `GearImageProps` and forwarded to each `<img>` element.
|
||||
|
||||
From src/client/components/ItemCard.tsx (current state, line 188-213):
|
||||
```tsx
|
||||
<div
|
||||
className="aspect-[4/3] overflow-hidden"
|
||||
style={{ backgroundColor: imageUrl ? imageContainerBg(dominantColor) : undefined }}
|
||||
>
|
||||
{imageUrl ? (
|
||||
<GearImage src={imageUrl} alt={name} dominantColor={dominantColor} cropZoom={cropZoom} cropX={cropX} cropY={cropY} />
|
||||
) : (
|
||||
<div className="w-full h-full bg-gray-50 flex flex-col items-center justify-center">
|
||||
<LucideIcon name={categoryIcon} size={36} className="text-gray-400" />
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
```
|
||||
|
||||
From src/client/components/CandidateCard.tsx (current state, line 163-188):
|
||||
Same pattern as ItemCard above — imageUrl conditional with GearImage or icon placeholder.
|
||||
|
||||
From src/client/components/GlobalItemCard.tsx (current state, line 40-73):
|
||||
Same pattern — imageUrl conditional with GearImage or SVG icon placeholder.
|
||||
</interfaces>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Add loading="lazy" and onLoad prop to GearImage (FIX-03 — part 1)</name>
|
||||
<files>src/client/components/GearImage.tsx</files>
|
||||
<read_first>
|
||||
- src/client/components/GearImage.tsx (read fully — understand all three render paths)
|
||||
</read_first>
|
||||
<action>
|
||||
Make two changes to src/client/components/GearImage.tsx (per D-07):
|
||||
|
||||
**1. Add `onLoad` to the props interface:**
|
||||
```typescript
|
||||
interface GearImageProps {
|
||||
src: string;
|
||||
alt: string;
|
||||
dominantColor?: string | null;
|
||||
cropZoom?: number | null;
|
||||
cropX?: number | null;
|
||||
cropY?: number | null;
|
||||
className?: string;
|
||||
cover?: boolean;
|
||||
onLoad?: () => void; // ADD THIS
|
||||
}
|
||||
```
|
||||
|
||||
**2. Destructure `onLoad` in the function signature:**
|
||||
```typescript
|
||||
export function GearImage({
|
||||
src,
|
||||
alt,
|
||||
dominantColor,
|
||||
cropZoom,
|
||||
cropX,
|
||||
cropY,
|
||||
className = "",
|
||||
cover = false,
|
||||
onLoad, // ADD THIS
|
||||
}: GearImageProps) {
|
||||
```
|
||||
|
||||
**3. Add `loading="lazy"` and `onLoad={onLoad}` to ALL THREE `<img>` elements:**
|
||||
|
||||
Cover path (currently line ~29):
|
||||
```tsx
|
||||
<img
|
||||
src={src}
|
||||
alt={alt}
|
||||
loading="lazy"
|
||||
onLoad={onLoad}
|
||||
className={`w-full h-full object-cover ${className}`}
|
||||
/>
|
||||
```
|
||||
|
||||
hasCrop path (currently line ~43):
|
||||
```tsx
|
||||
<img
|
||||
src={src}
|
||||
alt={alt}
|
||||
loading="lazy"
|
||||
onLoad={onLoad}
|
||||
className={`w-full h-full object-cover ${className}`}
|
||||
style={{
|
||||
transform: `scale(${cropZoom}) translate(${cropX ?? 0}%, ${cropY ?? 0}%)`,
|
||||
transformOrigin: "center center",
|
||||
}}
|
||||
/>
|
||||
```
|
||||
|
||||
Default path (currently line ~58):
|
||||
```tsx
|
||||
<img
|
||||
src={src}
|
||||
alt={alt}
|
||||
loading="lazy"
|
||||
onLoad={onLoad}
|
||||
className={`w-full h-full object-contain ${className}`}
|
||||
/>
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jlmak/Projects/jlmak/GearBox && grep -n 'loading="lazy"' src/client/components/GearImage.tsx | wc -l</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -c 'loading="lazy"' src/client/components/GearImage.tsx` outputs `3` (all three img elements)
|
||||
- `grep -n "onLoad" src/client/components/GearImage.tsx` shows the prop in interface, destructuring, and all three img elements (at least 5 matches)
|
||||
- `bun run lint` passes with no errors on the modified file
|
||||
</acceptance_criteria>
|
||||
<done>GearImage has loading="lazy" on all three img elements and forwards an optional onLoad callback prop. Existing callers pass no onLoad and are unaffected.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Add image skeleton to ItemCard, CandidateCard, and GlobalItemCard (FIX-03 — part 2)</name>
|
||||
<files>
|
||||
src/client/components/ItemCard.tsx,
|
||||
src/client/components/CandidateCard.tsx,
|
||||
src/client/components/GlobalItemCard.tsx
|
||||
</files>
|
||||
<read_first>
|
||||
- src/client/components/ItemCard.tsx (read fully — locate image area at lines 188-213)
|
||||
- src/client/components/CandidateCard.tsx (read fully — locate image area at lines 163-188)
|
||||
- src/client/components/GlobalItemCard.tsx (read fully — locate image area at lines 40-73)
|
||||
</read_first>
|
||||
<action>
|
||||
Apply identical skeleton pattern to all three cards (per D-08, UI-SPEC Image Skeleton Contract):
|
||||
|
||||
**For each card that has an imageUrl prop:**
|
||||
|
||||
**Step 1:** Add `useState` import if not already present (all three cards already import from react via other hooks — check existing imports and add `useState` to the destructured import if missing).
|
||||
|
||||
**Step 2:** Add the loaded state at the top of each component function:
|
||||
```typescript
|
||||
const [loaded, setLoaded] = useState(false);
|
||||
```
|
||||
|
||||
**Step 3:** Replace the imageUrl branch of the image area container.
|
||||
|
||||
**ItemCard** — replace the current `{imageUrl ? ... : ...}` inside `<div className="aspect-[4/3] overflow-hidden" ...>`:
|
||||
```tsx
|
||||
{imageUrl ? (
|
||||
<div className="relative w-full h-full">
|
||||
{!loaded && (
|
||||
<div className="absolute inset-0 bg-gray-100 animate-pulse" />
|
||||
)}
|
||||
<GearImage
|
||||
src={imageUrl}
|
||||
alt={name}
|
||||
dominantColor={dominantColor}
|
||||
cropZoom={cropZoom}
|
||||
cropX={cropX}
|
||||
cropY={cropY}
|
||||
onLoad={() => setLoaded(true)}
|
||||
className={`transition-opacity duration-200 ${loaded ? "opacity-100" : "opacity-0"}`}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<div className="w-full h-full bg-gray-50 flex flex-col items-center justify-center">
|
||||
<LucideIcon name={categoryIcon} size={36} className="text-gray-400" />
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
**CandidateCard** — same pattern as ItemCard, using `name` as the alt:
|
||||
```tsx
|
||||
{imageUrl ? (
|
||||
<div className="relative w-full h-full">
|
||||
{!loaded && (
|
||||
<div className="absolute inset-0 bg-gray-100 animate-pulse" />
|
||||
)}
|
||||
<GearImage
|
||||
src={imageUrl}
|
||||
alt={name}
|
||||
dominantColor={dominantColor}
|
||||
cropZoom={cropZoom}
|
||||
cropX={cropX}
|
||||
cropY={cropY}
|
||||
onLoad={() => setLoaded(true)}
|
||||
className={`transition-opacity duration-200 ${loaded ? "opacity-100" : "opacity-0"}`}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<div className="w-full h-full bg-gray-50 flex flex-col items-center justify-center">
|
||||
<LucideIcon name={categoryIcon} size={36} className="text-gray-400" />
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
**GlobalItemCard** — same pattern, alt is `\`${brand} ${model}\``:
|
||||
```tsx
|
||||
{imageUrl ? (
|
||||
<div className="relative w-full h-full">
|
||||
{!loaded && (
|
||||
<div className="absolute inset-0 bg-gray-100 animate-pulse" />
|
||||
)}
|
||||
<GearImage
|
||||
src={imageUrl}
|
||||
alt={`${brand} ${model}`}
|
||||
dominantColor={dominantColor}
|
||||
cropZoom={cropZoom}
|
||||
cropX={cropX}
|
||||
cropY={cropY}
|
||||
onLoad={() => setLoaded(true)}
|
||||
className={`transition-opacity duration-200 ${loaded ? "opacity-100" : "opacity-0"}`}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<div className="w-full h-full bg-gray-50 flex flex-col items-center justify-center">
|
||||
{/* keep existing SVG icon placeholder unchanged */}
|
||||
</div>
|
||||
)}
|
||||
```
|
||||
|
||||
Do NOT change the no-image placeholder (icon on bg-gray-50) in any card — it is correct behavior.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jlmak/Projects/jlmak/GearBox && grep -l "animate-pulse" src/client/components/ItemCard.tsx src/client/components/CandidateCard.tsx src/client/components/GlobalItemCard.tsx | wc -l</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `grep -n "animate-pulse" src/client/components/ItemCard.tsx` returns at least one match
|
||||
- `grep -n "animate-pulse" src/client/components/CandidateCard.tsx` returns at least one match
|
||||
- `grep -n "animate-pulse" src/client/components/GlobalItemCard.tsx` returns at least one match
|
||||
- `grep -n "useState" src/client/components/ItemCard.tsx` returns at least one match (loaded state)
|
||||
- `grep -n "useState" src/client/components/CandidateCard.tsx` returns at least one match
|
||||
- `grep -n "useState" src/client/components/GlobalItemCard.tsx` returns at least one match
|
||||
- `grep -n "transition-opacity duration-200" src/client/components/ItemCard.tsx` returns at least one match
|
||||
- `grep -n "transition-opacity duration-200" src/client/components/CandidateCard.tsx` returns at least one match
|
||||
- `grep -n "transition-opacity duration-200" src/client/components/GlobalItemCard.tsx` returns at least one match
|
||||
- `grep -n "onLoad" src/client/components/ItemCard.tsx` returns at least one match
|
||||
- `bun run lint` passes with no errors across all three files
|
||||
</acceptance_criteria>
|
||||
<done>All three card components show a gray animated skeleton (bg-gray-100 animate-pulse) while the image loads, then fade in the image via transition-opacity duration-200 once onLoad fires. No-image placeholders are unchanged.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser→S3 presigned URL | img src attributes point to S3 presigned URLs; loading="lazy" defers fetch |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|-------------|-----------------|
|
||||
| T-35-03 | Information Disclosure | GearImage lazy load | accept | loading="lazy" is a browser hint; presigned URLs are already time-limited by S3. No new exposure. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
After both tasks complete:
|
||||
|
||||
1. Open collection overview page — cards with images must show a gray pulsing placeholder, then fade in the image
|
||||
2. Open catalog/global-items page — GlobalItemCard items with images must show skeleton then fade in
|
||||
3. Open a thread page with candidates — CandidateCard images must show skeleton then fade in
|
||||
4. Cards without images must still show the category icon placeholder (no skeleton, no blank)
|
||||
5. Network throttle to "Slow 3G" in DevTools — skeleton must be clearly visible before image loads
|
||||
|
||||
Run: `bun run lint` — zero errors
|
||||
Run: `bun test` — all existing tests pass
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- GearImage has loading="lazy" on all 3 img elements and accepts optional onLoad prop
|
||||
- ItemCard, CandidateCard, GlobalItemCard each have a loaded state and show bg-gray-100 animate-pulse skeleton
|
||||
- Fade-in uses transition-opacity duration-200 on the GearImage className
|
||||
- No-image placeholder (icon on bg-gray-50) is unchanged in all three cards
|
||||
- bun run lint passes with zero errors
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/35-bug-fixes/35-02-SUMMARY.md`
|
||||
</output>
|
||||
Reference in New Issue
Block a user