diff --git a/.planning/phases/35-bug-fixes/35-CONTEXT.md b/.planning/phases/35-bug-fixes/35-CONTEXT.md new file mode 100644 index 0000000..5d4b245 --- /dev/null +++ b/.planning/phases/35-bug-fixes/35-CONTEXT.md @@ -0,0 +1,133 @@ +# Phase 35: Bug Fixes - Context + +**Gathered:** 2026-04-19 +**Status:** Ready for planning + + +## Phase Boundary + +Resolve 5 known v2.3 regressions and UX polish gaps before starting admin work. All fixes are self-contained — no new capabilities, no schema changes. The phase is complete when all 5 success criteria in ROADMAP.md are verifiably true. + + + + +## Implementation Decisions + +### FIX-01: Add Candidate modal (thread page) + +- **D-01:** Wire the "Add Candidate" toolbar button on the thread detail page to open `CatalogSearchOverlay` (same as the FAB), not the inline `AddCandidateModal`. +- **D-02:** Remove the inline `AddCandidateModal` component from the thread detail page entirely. Manual-add is already accessible via `CatalogSearchOverlay` → "Can't find it? Add manually" — no separate thread-page entry point needed. This deletes dead code. +- **D-03:** The `CatalogSearchOverlay` must be opened in thread-candidate mode (pre-scoped to add a candidate to the current thread), same as the FAB already does it. + +### FIX-02: Item images missing on collection overview + +- **D-04:** Add `imageUrl: string | null`, `dominantColor: string | null`, `cropZoom: number | null`, `cropX: number | null`, `cropY: number | null`, and `priceCurrency: string | null` to the `ItemWithCategory` interface in `src/client/hooks/useItems.ts`. The server already returns these fields via `withImageUrls()` and the DB query — the TypeScript type just hasn't been updated to include them. +- **D-05:** No server-side changes needed — `GET /api/items` already enriches with `imageUrl` via `withImageUrls()`. + +### FIX-03: Slow image loading + +- **D-06:** Scope is **UX-only** — do not touch presigned URL generation or caching. Presigned URL performance is deferred to a future phase. +- **D-07:** Add `loading="lazy"` to all `` tags across the app. +- **D-08:** Add image skeleton/loading states (gray animated placeholder in the image area) to **all** card types that display images: `ItemCard`, `CandidateCard`, and catalog item cards (`GlobalItemCard`, discovery cards). Use the existing `animate-pulse` pattern already present on the collection overview skeleton loader. + +### FIX-04: Auth prompt sign-in redirect + +- **D-09:** The `/login` React route currently renders an intermediate "Sign in to GearBox" page that makes the user click a button before hitting Logto. Change it to auto-redirect immediately via `useEffect` → `window.location.href = "/login"` (the server-side Logto redirect). The intermediate React UI page is not needed. +- **D-10:** `AuthPromptModal` already links to `/login` (TanStack Router navigate) — no changes needed there once the React login route auto-redirects. + +### FIX-05: Cursor pointer on clickable elements + +- **D-11:** Audit all interactive elements and add `cursor-pointer` where missing. Known gaps: `ItemCard` outer button uses conditional cursor logic (`cursor-default` when `linkTo === null`) — ensure all cases are covered. Check all cards, badges, action buttons, and links. +- **D-12:** Add a global CSS rule `[role="button"] { cursor: pointer; }` or use Tailwind's `cursor-pointer` consistently on all clickable elements. Prefer Tailwind utility over global CSS to stay consistent with the codebase style. + +### Testing + +- **D-13:** No new regression tests required for Phase 35. These are UI/type fixes — manual verification in the browser and existing passing tests are sufficient. The codebase already has 20+ test files; adding tests for cursor behavior and loading states has low ROI. + +### Claude's Discretion + +- Which specific catalog card component file implements the loading skeleton for `GlobalItemCard` / discovery cards — researcher should identify the right component(s). +- Whether `priceCurrency` is already in the items list API response or needs to be added server-side (likely already there given the currency system built in v2.3). + +### Folded Todos + +All 5 pending todos are directly mapped to Phase 35 requirements and are folded into scope: + +- `fix-add-candidate-button-shows-wrong-modal-on-thread-page` → FIX-01 +- `fix-item-image-not-showing-on-collection-overview` → FIX-02 +- `investigate-slow-image-loading` → FIX-03 +- `auth-prompt-sign-in-button-should-redirect-directly-to-logto` → FIX-04 +- `add-cursor-pointer-to-all-clickable-links` → FIX-05 + + + + +## Canonical References + +**Downstream agents MUST read these before planning or implementing.** + +### Phase Requirements + +- `.planning/REQUIREMENTS.md` — FIX-01 through FIX-05 acceptance criteria (the definition of done for each fix) +- `.planning/ROADMAP.md` §Phase 35 — Success criteria and phase goal + +### Key Source Files + +- `src/client/routes/threads/$threadId/index.tsx` — Thread detail page with the broken "Add Candidate" button and the inline `AddCandidateModal` to be removed +- `src/client/hooks/useItems.ts` — `ItemWithCategory` interface missing `imageUrl` and related fields (FIX-02) +- `src/client/components/GearImage.tsx` — Core image component; `loading="lazy"` should be added here +- `src/client/components/ItemCard.tsx` — Image skeleton state target +- `src/client/components/CandidateCard.tsx` — Image skeleton state target +- `src/client/components/GlobalItemCard.tsx` — Image skeleton state target (catalog cards) +- `src/client/routes/login.tsx` — Intermediate login page that needs to auto-redirect (FIX-04) +- `src/server/services/storage.service.ts` — `withImageUrls()` / `getImageUrl()` — confirms server already returns `imageUrl` + + + + +## Existing Code Insights + +### Reusable Assets + +- `CatalogSearchOverlay` (`src/client/components/CatalogSearchOverlay.tsx`) — already opens in thread-candidate mode via the FAB; FIX-01 wires the top button to the same `openCatalogSearch` Zustand action +- `useUIStore` (`src/client/stores/uiStore.ts`) — has `openCatalogSearch` action the FAB uses; FIX-01 calls this same action +- `GearImage` component — all image rendering goes through this; `loading="lazy"` added here propagates everywhere +- `animate-pulse` skeleton pattern — already used on collection overview load state; reuse for image placeholders + +### Established Patterns + +- Images: `imageFilename` stored on records → server enriches with `imageUrl` via `withImageUrls()` → client receives full presigned URL and passes to `GearImage` +- Zustand UI state: modals and overlays opened via `useUIStore` actions, not local component state +- Tailwind `cursor-pointer` on interactive elements (not global CSS) +- `useEffect` + `window.location.href` for hard navigation (already used in login page for the button) + +### Integration Points + +- `GET /api/items` → already returns `imageUrl` — only the TypeScript type needs updating +- `openCatalogSearch` in `useUIStore` — FIX-01 calls this from the thread page button +- `AddCandidateModal` inline component in thread route — delete this component and its state + + + + +## Specific Ideas + +- FIX-01: Delete the entire `AddCandidateModal` function and `addCandidateOpen` state from `src/client/routes/threads/$threadId/index.tsx`. Clean code removal, not a hide. +- FIX-03: `loading="lazy"` belongs on the `` elements inside `GearImage.tsx` — one change, affects all usages. +- FIX-04: The `LoginPage` component can be simplified to just a `useEffect` redirect (or even a minimal ``) — the full page UI with a button is unnecessary. + + + + +## Deferred Ideas + +- Presigned URL server-side caching (TTL-based in-memory or Redis cache) — mentioned during FIX-03 discussion, intentionally out of scope for Phase 35. Consider for a future performance phase. +- Image resizing/thumbnails on upload — separate concern, deferred. +- Cache-Control headers on S3 objects — deferred. + + + +--- + +*Phase: 35-bug-fixes* +*Context gathered: 2026-04-19* diff --git a/.planning/phases/35-bug-fixes/35-DISCUSSION-LOG.md b/.planning/phases/35-bug-fixes/35-DISCUSSION-LOG.md new file mode 100644 index 0000000..9ef3a9c --- /dev/null +++ b/.planning/phases/35-bug-fixes/35-DISCUSSION-LOG.md @@ -0,0 +1,47 @@ +# Phase 35: Bug Fixes - Discussion Log + +> **Audit trail only.** Do not use as input to planning, research, or execution agents. +> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered. + +**Date:** 2026-04-19 +**Phase:** 35-bug-fixes +**Areas discussed:** FIX-03 scope, FIX-01 fallback + +--- + +## FIX-03 scope: slow image loading + +| Option | Description | Selected | +|--------|-------------|----------| +| Lazy loading + skeleton states only | Add HTML loading='lazy' and gray animated placeholders while images load. Addresses visible UX symptom without touching presigned URL generation. | ✓ | +| Lazy loading + presigned URL caching | Also cache generated presigned URLs server-side for their TTL duration. More impactful but touches the storage layer. | | +| Full investigation and fix all causes | Audit all three causes (lazy loading, presigned URL generation overhead, image sizing), fix everything found. | | + +**User's choice:** Lazy loading + skeleton states only +**Notes:** Presigned URL caching explicitly deferred to a future phase. Skeletons should appear on all card types that display images (ItemCard, CandidateCard, catalog/discovery cards) — not just ItemCard. + +--- + +## FIX-01 fallback: catalog vs manual flow + +| Option | Description | Selected | +|--------|-------------|----------| +| Inline fallback inside CatalogSearchOverlay | CatalogSearchOverlay already has "Can't find it? Add manually" link. Just wire the button to the overlay, no extra UI needed. | | +| Secondary button on thread page | Keep an explicit "Add manually" secondary button on the thread page toolbar. | | +| Remove the manual-add modal from thread page entirely | Delete the inline AddCandidateModal component and its state. Manual add is already reachable via FAB → overlay → "add manually". | ✓ | + +**User's choice:** Remove the manual-add modal from thread page entirely +**Notes:** The `AddCandidateModal` inline component and `addCandidateOpen` state should be fully deleted from the thread detail route. Clean code removal. + +--- + +## Claude's Discretion + +- Which specific card component file implements catalog/discovery card loading skeletons +- Whether `priceCurrency` is already in the items list API response + +## Deferred Ideas + +- Presigned URL server-side caching (TTL-based) — out of scope for Phase 35 +- Image resizing/thumbnails on upload — deferred +- Cache-Control headers on S3 objects — deferred