docs(35): capture phase context
This commit is contained in:
133
.planning/phases/35-bug-fixes/35-CONTEXT.md
Normal file
133
.planning/phases/35-bug-fixes/35-CONTEXT.md
Normal file
@@ -0,0 +1,133 @@
|
||||
# Phase 35: Bug Fixes - Context
|
||||
|
||||
**Gathered:** 2026-04-19
|
||||
**Status:** Ready for planning
|
||||
|
||||
<domain>
|
||||
## 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.
|
||||
|
||||
</domain>
|
||||
|
||||
<decisions>
|
||||
## 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 `<img>` 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
|
||||
|
||||
</decisions>
|
||||
|
||||
<canonical_refs>
|
||||
## 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`
|
||||
|
||||
</canonical_refs>
|
||||
|
||||
<code_context>
|
||||
## 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
|
||||
|
||||
</code_context>
|
||||
|
||||
<specifics>
|
||||
## 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 `<img>` 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 `<Redirect>`) — the full page UI with a button is unnecessary.
|
||||
|
||||
</specifics>
|
||||
|
||||
<deferred>
|
||||
## 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.
|
||||
|
||||
</deferred>
|
||||
|
||||
---
|
||||
|
||||
*Phase: 35-bug-fixes*
|
||||
*Context gathered: 2026-04-19*
|
||||
47
.planning/phases/35-bug-fixes/35-DISCUSSION-LOG.md
Normal file
47
.planning/phases/35-bug-fixes/35-DISCUSSION-LOG.md
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user