fix(27): revise plans based on checker feedback

2026-04-10 23:40:11 +02:00
parent 0f3e85f7c4
commit 2286e428a0
5 changed files with 192 additions and 9 deletions
--- a/.planning/ROADMAP.md
+++ b/.planning/ROADMAP.md
@@ -136,9 +136,10 @@ Plans:
  3. On mobile, navigation appears as a fixed bottom tab bar with Home, Collection, Setups, and Search icons
  4. The landing page no longer has a hero section — content starts with Popular Setups
  5. Setups has its own top-level route accessible from the nav bar, not nested in Collection tabs
-**Plans**: 3 plans
+**Plans**: 4 plans

 Plans:
+- [ ] 27-00-PLAN.md — Wave 0: E2E test scaffolding for nav restructure
 - [ ] 27-01-PLAN.md — TopNav and BottomTabBar components
 - [ ] 27-02-PLAN.md — Setups top-level route and Collection tab simplification
 - [ ] 27-03-PLAN.md — Root layout wiring, hero removal, and visual verification
--- a/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-00-PLAN.md
+++ b/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-00-PLAN.md
@@ -0,0 +1,178 @@
+---
+phase: 27-top-nav-restructure-and-search-bar-rethink
+plan: 00
+type: execute
+wave: 0
+depends_on: []
+files_modified:
+  - e2e/dashboard.spec.ts
+  - e2e/collection.spec.ts
+autonomous: true
+requirements:
+  - NAV-01
+  - NAV-04
+  - NAV-05
+
+must_haves:
+  truths:
+    - "E2E tests for dashboard reflect new nav structure (TopNav, no hero heading)"
+    - "E2E tests for collection reflect Setups tab removal and /setups route existence"
+    - "E2E tests cover anonymous auth modal trigger from nav"
+    - "E2E tests cover mobile bottom tab bar presence"
+  artifacts:
+    - path: "e2e/dashboard.spec.ts"
+      provides: "Updated dashboard E2E tests matching new nav layout"
+    - path: "e2e/collection.spec.ts"
+      provides: "Updated collection E2E tests without Setups tab assertions"
+  key_links: []
+---
+
+<objective>
+Update existing E2E tests to match the Phase 27 navigation restructure before implementation begins.
+
+Purpose: Wave 0 test scaffolding ensures the Nyquist sampling contract is met. Existing tests assert behaviors that Phase 27 changes (hero heading, Setups tab in collection, dashboard card layout). These tests must be updated to expect the new structure so they serve as regression guards during implementation.
+
+Output: Updated `e2e/dashboard.spec.ts` and `e2e/collection.spec.ts` with assertions matching the post-Phase-27 UI.
+</objective>
+
+<execution_context>
+@$HOME/.claude/get-shit-done/workflows/execute-plan.md
+@$HOME/.claude/get-shit-done/templates/summary.md
+</execution_context>
+
+<context>
+@.planning/PROJECT.md
+@.planning/ROADMAP.md
+@.planning/STATE.md
+@.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-CONTEXT.md
+@.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-VALIDATION.md
+</context>
+
+<tasks>
+
+<task type="auto">
+  <name>Task 1: Update dashboard E2E tests for new nav structure</name>
+  <files>e2e/dashboard.spec.ts</files>
+  <read_first>
+    e2e/dashboard.spec.ts
+    src/client/routes/index.tsx
+    src/client/components/TotalsBar.tsx
+  </read_first>
+  <action>
+Modify `e2e/dashboard.spec.ts` to reflect the Phase 27 navigation changes. These tests will initially FAIL (red) until Plans 01-03 implement the new UI. That is expected — this is Wave 0 scaffolding.
+
+**Changes to existing tests:**
+
+1. **"shows GearBox heading" test:** Update to check for GearBox text in the top nav bar rather than as a standalone heading. The text "GearBox" will still be visible (it's the logo text in TopNav), so this test likely still passes as-is. Add a comment noting it now refers to the nav logo.
+
+2. **"shows Collection, Planning, and Setups card headings" test:** The landing page hero is removed and the page now starts with Popular Setups, Recently Added Items, and Trending Categories sections. Update this test to check for the section headings that will exist on the landing page post-Phase-27: "Popular Setups", "Recently Added", "Trending Categories". Remove the assertions for "Collection", "Planning", and "Setups" card headings (those cards may still exist as discovery sections, but the test name and assertions should match the new landing page structure).
+
+3. **"Collection card links to /collection" test:** The landing page no longer has explicit "Collection" card links — navigation is via the TopNav. Replace this test with a test that verifies the TopNav contains a link to /collection:
+```typescript
+test("top nav contains Collection link", async ({ page }) => {
+  const nav = page.locator("nav");
+  const collectionLink = nav.getByRole("link", { name: /collection/i });
+  await expect(collectionLink).toBeVisible();
+  await collectionLink.click();
+  await page.waitForLoadState("networkidle");
+  await expect(page).toHaveURL(/\/collection/);
+});
+```
+
+**New tests to add:**
+
+4. **Auth modal on anonymous Collection click:** Add a test that verifies clicking Collection in the nav while not authenticated triggers the auth prompt modal. Note: The E2E seed runs with an authenticated user, so this test should be skipped with `test.skip` and a comment explaining it needs an unauthenticated test fixture. Alternatively, if the E2E environment has no auth (public read mode), the nav links for Collection/Setups should render as buttons that trigger the auth modal — test for the modal appearing.
+
+```typescript
+test("shows top nav with navigation links", async ({ page }) => {
+  // TopNav should contain Home, Collection, Setups links
+  const nav = page.locator("nav");
+  await expect(nav).toBeVisible();
+  await expect(nav.getByText("Home")).toBeVisible();
+  await expect(nav.getByText("Collection")).toBeVisible();
+  await expect(nav.getByText("Setups")).toBeVisible();
+});
+```
+
+5. **Mobile bottom tab bar test:** Add a test with mobile viewport that checks for bottom tab bar:
+```typescript
+test("shows bottom tab bar on mobile viewport", async ({ page }) => {
+  await page.setViewportSize({ width: 375, height: 667 });
+  await page.goto("/");
+  await page.waitForLoadState("networkidle");
+  // Bottom tab bar should be visible with 4 items
+  await expect(page.getByText("Home")).toBeVisible();
+  await expect(page.getByText("Collection")).toBeVisible();
+  await expect(page.getByText("Setups")).toBeVisible();
+  await expect(page.getByText("Search")).toBeVisible();
+});
+```
+
+**Keep unchanged:** "shows collection card with item count of 6", "shows active thread count on Planning card", "shows setup count on Setups card" — update only if they reference elements being removed. If they reference landing page discovery cards that still exist, keep as-is. If they reference elements that won't exist post-Phase-27, mark with `test.fixme()` and a comment.
+
+Review the current landing page structure carefully before deciding which tests to keep, update, or mark as fixme. The goal is: tests that will PASS after Plans 01-03 are implemented, and tests that will FAIL now (before implementation) are acceptable for Wave 0.
+  </action>
+  <verify>
+    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "top nav" e2e/dashboard.spec.ts && grep -c "mobile" e2e/dashboard.spec.ts && grep -c "setViewportSize" e2e/dashboard.spec.ts</automated>
+  </verify>
+  <done>dashboard.spec.ts contains updated tests for TopNav presence, nav link visibility, and mobile bottom tab bar. Tests are expected to fail until Plans 01-03 implement the new UI.</done>
+</task>
+
+<task type="auto">
+  <name>Task 2: Update collection E2E tests for Setups tab removal</name>
+  <files>e2e/collection.spec.ts</files>
+  <read_first>
+    e2e/collection.spec.ts
+  </read_first>
+  <action>
+Modify `e2e/collection.spec.ts` to reflect the Setups tab being removed from Collection and elevated to its own route.
+
+**Changes:**
+
+1. **Remove "navigates to setups tab" test:** The test at line 77-81 navigates to `/collection?tab=setups` and expects "Weekend Overnighter". After Phase 27, `/collection?tab=setups` falls back to the Gear tab (via Zod `.catch("gear")`). Replace this test with a fallback test:
+```typescript
+test("setups tab URL falls back to gear tab", async ({ page }) => {
+  await page.goto("/collection?tab=setups");
+  await page.waitForLoadState("networkidle");
+  // Setups tab no longer exists, should fall back to gear
+  await expect(page.getByText("Zpacks Duplex")).toBeVisible();
+});
+```
+
+2. **Add /setups route test:** Add a new test (can be in a new `test.describe` block or at the end) that verifies the `/setups` route renders correctly:
+```typescript
+test.describe("Setups page", () => {
+  test("navigates to /setups and shows seeded setup", async ({ page }) => {
+    await page.goto("/setups");
+    await page.waitForLoadState("networkidle");
+    await expect(page.getByText("Weekend Overnighter")).toBeVisible();
+  });
+});
+```
+
+**Keep unchanged:** All Gear tab tests (search, filter, category) and the "navigates to planning tab" test and "gear tab is default" test. These are unaffected by Phase 27.
+  </action>
+  <verify>
+    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "/setups" e2e/collection.spec.ts && grep -c "falls back" e2e/collection.spec.ts && ! grep -q 'tab=setups.*Weekend Overnighter' e2e/collection.spec.ts && echo "no-old-setups-tab"</automated>
+  </verify>
+  <done>collection.spec.ts no longer asserts Setups tab content at /collection?tab=setups. New test verifies /setups route renders SetupsView. Fallback test confirms old ?tab=setups URLs show gear tab.</done>
+</task>
+
+</tasks>
+
+<verification>
+- dashboard.spec.ts has tests for TopNav presence, nav links, and mobile bottom tab bar
+- collection.spec.ts has no assertion that /collection?tab=setups shows setup content
+- collection.spec.ts has a test for the standalone /setups route
+- All existing unaffected tests (gear tab filtering, planning tab) remain unchanged
+</verification>
+
+<success_criteria>
+- E2E test files are updated to match the post-Phase-27 expected UI
+- Tests may fail now (pre-implementation) but will pass after Plans 01-03 complete
+- No unrelated test changes — only Phase 27 navigation restructure assertions
+</success_criteria>
+
+<output>
+After completion, create `.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-00-SUMMARY.md`
+</output>
--- a/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-01-PLAN.md
+++ b/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-01-PLAN.md
@@ -186,6 +186,8 @@ Create `src/client/components/BottomTabBar.tsx` for mobile navigation.
 3. Setups — icon: `layers`, label: "Setups", `<Link to="/setups">` if authenticated, `<button onClick={openAuthPrompt}>` if anonymous (per D-02)
 4. Search — icon: `search`, label: "Search", always `<button onClick={() => openCatalogSearch("collection")}>` (per D-14, D-17)

+**Icon availability:** The `layers` icon is confirmed available in the curated icon set (`src/client/lib/iconData.tsx`). If for any reason it's not found at implementation time, use `briefcase` or `grid-2x2` as fallback.
+
 **Active state:**
 Use same `useMatchRoute` pattern as TopNav. Active tab: `text-gray-900`, inactive: `text-gray-400`. Search tab is never "active" (it opens an overlay, not a route).

@@ -212,7 +214,7 @@ Add `pb-[env(safe-area-inset-bottom)]` to the container for iOS devices with hom
 Export: `export function BottomTabBar()`
  </action>
  <verify>
-    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "export function BottomTabBar" src/client/components/BottomTabBar.tsx && grep -c "md:hidden" src/client/components/BottomTabBar.tsx && grep -c "openCatalogSearch" src/client/components/BottomTabBar.tsx && grep -c "openAuthPrompt" src/client/components/BottomTabBar.tsx</automated>
+    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "export function BottomTabBar" src/client/components/BottomTabBar.tsx && grep -c "md:hidden" src/client/components/BottomTabBar.tsx && grep -c "openCatalogSearch" src/client/components/BottomTabBar.tsx && grep -c "openAuthPrompt" src/client/components/BottomTabBar.tsx && grep -c "layers" src/client/lib/iconData.tsx</automated>
  </verify>
  <acceptance_criteria>
    - src/client/components/BottomTabBar.tsx exists and exports `export function BottomTabBar()`
@@ -222,8 +224,9 @@ Export: `export function BottomTabBar()`
    - File contains 4 tab items: Home, Collection, Setups, Search (grep for all 4 labels)
    - File contains `openCatalogSearch("collection")` for Search tab
    - File contains `openAuthPrompt` for anonymous Collection/Setups taps
-    - File uses `LucideIcon` with names: `home`, `package`, `layers`, `search`
+    - File uses `LucideIcon` with names: `home`, `package`, `layers` (or fallback `briefcase`/`grid-2x2`), `search`
    - File imports `motion` from `framer-motion` for entry animation
+    - The `layers` icon exists in `src/client/lib/iconData.tsx` (verified: present)
  </acceptance_criteria>
  <done>BottomTabBar renders 4 tabs with Lucide icons on mobile viewports. Search tab opens CatalogSearchOverlay. Collection/Setups tabs fire AuthPromptModal for anonymous users. Active tab is highlighted. Component hidden on md+ screens.</done>
 </task>
@@ -237,6 +240,7 @@ Export: `export function BottomTabBar()`
 - Both use `openCatalogSearch("collection")` for search trigger
 - TopNav uses `hidden md:flex` for desktop nav; BottomTabBar uses `md:hidden` for mobile only
 - Neither imports directly from `lucide-react` — both use `LucideIcon` wrapper
+- The `layers` icon is available in the curated icon set
 </verification>

 <success_criteria>
--- a/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-02-PLAN.md
+++ b/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-02-PLAN.md
@@ -10,7 +10,6 @@ files_modified:
 autonomous: true
 requirements:
  - NAV-05
-  - NAV-04

 must_haves:
  truths:
--- a/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-03-PLAN.md
+++ b/.planning/phases/27-top-nav-restructure-and-search-bar-rethink/27-03-PLAN.md
@@ -163,7 +163,7 @@ On the root div `<div className="min-h-screen bg-gray-50">`, add mobile-only bot
 **Do NOT change:** CandidateDeleteDialog, ResolveDialog, CatalogSearchOverlay, AddToCollectionModal, AddToThreadModal, Toaster, AuthPromptModal, OnboardingWizard, ConfirmDialog, ExternalLinkDialog, or any other existing modal/dialog code.
  </action>
  <verify>
-    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "TopNav" src/client/routes/__root.tsx && grep -c "BottomTabBar" src/client/routes/__root.tsx && ! grep -q "TotalsBar" src/client/routes/__root.tsx && echo "no-totalsbar" && grep -c 'hidden md:block' src/client/routes/__root.tsx && grep 'isPublicRoute' src/client/routes/__root.tsx | grep -c '/setups"'</automated>
+    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && grep -c "TopNav" src/client/routes/__root.tsx && grep -c "BottomTabBar" src/client/routes/__root.tsx && ! grep -q "TotalsBar" src/client/routes/__root.tsx && echo "no-totalsbar" && grep -c 'hidden md:block' src/client/routes/__root.tsx && grep 'isPublicRoute' src/client/routes/__root.tsx | grep -c '/setups"' && grep -c 'pb-16 md:pb-0' src/client/routes/__root.tsx</automated>
  </verify>
  <acceptance_criteria>
    - __root.tsx imports `TopNav` from `../components/TopNav` (not TotalsBar)
@@ -174,7 +174,7 @@ On the root div `<div className="min-h-screen bg-gray-50">`, add mobile-only bot
    - __root.tsx renders `<BottomTabBar />`
    - FabMenu is wrapped in `<div className="hidden md:block">`
    - isPublicRoute includes `location.pathname === "/setups"`
-    - Root div has `pb-16 md:pb-0` classes
+    - Root div has `pb-16 md:pb-0` classes for mobile bottom padding
    - All existing modals/dialogs remain unchanged
  </acceptance_criteria>
  <done>Root layout uses TopNav instead of TotalsBar, BottomTabBar is rendered for mobile, FAB is hidden on mobile, /setups is a public route, and mobile bottom padding prevents content occlusion.</done>
@@ -215,19 +215,20 @@ function LandingPage() {
 4. **Keep unchanged:** `PopularSetupsSection`, `RecentItemsSection`, `TrendingCategoriesSection`, `SectionSkeleton`, all discovery hooks imports, `GlobalItemCard` import, `PublicSetupCard` import, and the `createFileRoute` route definition.
  </action>
  <verify>
-    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && ! grep -q "HeroSection" src/client/routes/index.tsx && echo "no-hero" && ! grep -q "Discover Gear" src/client/routes/index.tsx && echo "no-heading" && ! grep -q "lucide-react" src/client/routes/index.tsx && echo "no-lucide-direct" && grep -c "PopularSetupsSection" src/client/routes/index.tsx</automated>
+    <automated>cd /home/jean-luc-makiola/Development/projects/GearBox && ! grep -q "HeroSection" src/client/routes/index.tsx && echo "no-hero" && ! grep -q "Discover Gear" src/client/routes/index.tsx && echo "no-heading" && ! grep -q "lucide-react" src/client/routes/index.tsx && echo "no-lucide-direct" && grep -c "PopularSetupsSection" src/client/routes/index.tsx && ! grep -q "useAuth" src/client/routes/index.tsx && echo "no-useAuth" && ! grep -q "useUIStore" src/client/routes/index.tsx && echo "no-useUIStore"</automated>
  </verify>
  <acceptance_criteria>
    - src/client/routes/index.tsx does NOT contain `HeroSection` (function definition or usage)
    - src/client/routes/index.tsx does NOT contain `Discover Gear` heading text
    - src/client/routes/index.tsx does NOT contain `Go to Collection` link text
    - src/client/routes/index.tsx does NOT import from `lucide-react`
-    - src/client/routes/index.tsx does NOT import `useAuth` or `useUIStore`
+    - src/client/routes/index.tsx does NOT import `useAuth`
+    - src/client/routes/index.tsx does NOT import `useUIStore`
    - LandingPage function renders PopularSetupsSection as first child
    - PopularSetupsSection, RecentItemsSection, TrendingCategoriesSection all remain
    - SectionSkeleton helper function remains
  </acceptance_criteria>
-  <done>Landing page starts directly with Popular Setups section. No hero section, no heading, no search bar, no "Go to Collection" link. Search is now exclusively in the TopNav bar.</done>
+  <done>Landing page starts directly with Popular Setups section. No hero section, no heading, no search bar, no "Go to Collection" link. No unused imports (useAuth, useUIStore, lucide-react). Search is now exclusively in the TopNav bar.</done>
 </task>

 <task type="checkpoint:human-verify" gate="blocking">