diff --git a/ALPINE_COMPLETION_GUIDE.md b/ALPINE_COMPLETION_GUIDE.md index 2666fbb..221ab92 100644 --- a/ALPINE_COMPLETION_GUIDE.md +++ b/ALPINE_COMPLETION_GUIDE.md @@ -11,7 +11,6 @@ This guide completes the migration from **hybrid onclick/@click with manual DOM **References:** - **`SSR_FIRST_ALPINE_GUIDE.md`** - SSR-first architecture principles (READ THIS FIRST) -- **`COLLECTIONS_CLEANUP_GUIDE.md`** - Immediate console error fixes (quick reference) --- @@ -44,7 +43,7 @@ This guide completes the migration from **hybrid onclick/@click with manual DOM ### Documentation Structure -**Three complementary guides:** +**Two complementary guides:** 1. **`SSR_FIRST_ALPINE_GUIDE.md`** - **READ THIS FIRST** - SSR-first architecture principles @@ -53,15 +52,11 @@ This guide completes the migration from **hybrid onclick/@click with manual DOM - Server-side token injection - **Prerequisite for understanding this guide** -2. **`COLLECTIONS_CLEANUP_GUIDE.md`** - Quick reference for immediate fixes - - Dead export removal (causes console errors) - - DOMContentLoaded cleanup (prevents wrong-page execution) - - Step-by-step instructions for common fixes - - **Use as reference during this migration** - -3. **`ALPINE_COMPLETION_GUIDE.md`** - **This document** +2. **`ALPINE_COMPLETION_GUIDE.md`** - **This document** - Full reactive Alpine.js migration path - - Eliminate all manual DOM manipulation + - Dead export removal (Phase 0) + - DOMContentLoaded cleanup (Phase 3) + - Template function call fixes - Complete code examples and patterns - **Long-term architecture goal** @@ -147,55 +142,262 @@ When functions are deleted from TypeScript but remain in `Alpine.data()` exports These must be fixed before attempting full migration. -### Quick Reference +--- -For detailed step-by-step instructions, see **`COLLECTIONS_CLEANUP_GUIDE.md`** - **Step 1** covers this process comprehensively. +### Step 0.1: Verify Current State -### The Process +Before starting, check the current errors: -**For collections.ts (and similar files):** +```bash +cd /home/nymusicman/Code/bookhoard +npm run build:ts + +# Should see errors about: +# - "addbooksToAdd is not defined" +# - "removebooksToAdd is not defined" +# - "toggleBookSelection is not defined" +# - etc. +``` + +--- + +### Step 0.2: Fix collections.ts Alpine.data Export + +**File:** `web/src/collections.ts` + +**Problem:** Alpine.data exports functions that were deleted in commit 93710a1. + +#### Step 0.2.1: Read Current Alpine.data Export + +Check what's currently exported: + +```bash +# Check what's currently exported +tail -50 web/src/collections.ts | grep -A25 "Alpine.data" +``` + +You'll see something like: + +```typescript +Alpine.data("collections", () => ({ + addbooksToAdd, // ❌ Does NOT exist - deleted + backToCollections, + closeCollectionModal, + createRule, + deleteRule, + filterCollectionBooks, + filterIcons, + hideAddBooksModal, + initCollectionDetail, // ❌ Does NOT exist - deleted + initColorSelection, // ❌ Does NOT exist - deleted + initIconSelection, // ❌ Does NOT exist - deleted + loadCollectionRules, + loadCollections, + navigateToCollection, + populateIconGrid, + removeBook, + removebooksToAdd, // ❌ Does NOT exist - deleted + searchBooksForCollections, + selectColor, + selectIcon, + showAddBooksModal, + showAllIcons, // ❌ Does NOT exist - deleted + setupHTMXAuth, + testRule, + toggleBookForRemoval, // ❌ Does NOT exist - deleted + toggleBookSelection, // ❌ Does NOT exist - deleted + updateSelectedCount, +})); +``` + +#### Step 0.2.2: Read Current Export Statement + +Check the export statement at the end of the file: + +```bash +# Check the export statement at the end of the file +grep -A30 "^export {" web/src/collections.ts +``` + +You'll see similar dead exports. + +#### Step 0.2.3: Verify Which Functions Actually Exist + +Search for function definitions: + +```bash +# Search for function definitions +grep -n "^function\|^async function" web/src/collections.ts +``` + +Expected output (actual existing functions): +- `backToCollections` ✓ +- `closeCollectionModal` ✓ +- `createRule` ✓ +- `deleteRule` ✓ +- `filterCollectionBooks` ✓ +- `filterIcons` ✓ +- `hideAddBooksModal` ✓ +- `loadCollectionRules` ✓ +- `loadCollections` ✓ +- `navigateToCollection` ✓ +- `populateIconGrid` ✓ +- `removeBook` ✓ +- `searchBooksForCollections` ✓ +- `selectColor` ✓ +- `selectIcon` ✓ +- `showAddBooksModal` ✓ +- `setupHTMXAuth` ✓ +- `testRule` ✓ +- `updateSelectedCount` ✓ + +#### Step 0.2.4: Update the Export Statement + +**Line ~407:** Change the `export` statement to only include existing functions: + +```typescript +export { + backToCollections, + closeCollectionModal, + createRule, + deleteRule, + filterCollectionBooks, + filterIcons, + hideAddBooksModal, + loadCollectionRules, + loadCollections, + navigateToCollection, + populateIconGrid, + removeBook, + searchBooksForCollections, + selectColor, + selectIcon, + showAddBooksModal, + setupHTMXAuth, + testRule, + updateSelectedCount, +}; +``` + +#### Step 0.2.5: Update Alpine.data Registration + +**Line ~424:** Update Alpine.data to match the export: + +```typescript +Alpine.data("collections", () => ({ + backToCollections, + closeCollectionModal, + createRule, + deleteRule, + filterCollectionBooks, + filterIcons, + hideAddBooksModal, + loadCollectionRules, + loadCollections, + navigateToCollection, + populateIconGrid, + removeBook, + searchBooksForCollections, + selectColor, + selectIcon, + showAddBooksModal, + setupHTMXAuth, + testRule, + updateSelectedCount, +})); +``` + +#### Step 0.2.6: Verify the Fix + +```bash +# Build TypeScript +npm run build:ts + +# Should now succeed with 0 errors +``` + +--- + +### Step 0.3: Fix Template Function Calls + +**File:** `templates/collections.templ` + +**Problem:** Template calls functions that no longer exist. + +#### Step 0.3.1: Remove Dead Function Calls from Collection Detail Page + +**Line ~226:** Remove the `removebooksToAdd` call: + +```html + + + + + +``` + +**Line ~229:** Remove `addbooksToAdd` call: + +```html + + + + + +``` + +#### Step 0.3.2: Regenerate Templates + +```bash +# Generate Go template files +templ generate + +# Should see: Complete [updates=0 duration=~40ms] +``` + +--- + +### Step 0.4: Check Other Files for Similar Issues + +Based on commit 93710a1 and current errors: +- ✅ `web/src/collections.ts` - Fixed above +- Check other files for similar issues as you encounter them + +**General process for any file:** 1. **Identify dead exports:** ```bash # Check what's exported -grep -A25 "Alpine.data" web/src/collections.ts +grep -A25 "Alpine.data" web/src/FILENAME.ts # Find actual function definitions -grep -n "^function\|^async function" web/src/collections.ts +grep -n "^function\|^async function" web/src/FILENAME.ts ``` -2. **Update export statement:** -```typescript -// Remove dead functions from export -export { - // Keep only existing functions - backToCollections, - closeCollectionModal, - // ... etc ... -}; -``` +2. **Update export statement** to remove dead functions -3. **Update Alpine.data registration:** -```typescript -Alpine.data("collections", () => ({ - // Keep only existing functions - backToCollections, - closeCollectionModal, - // ... etc ... -})); -``` +3. **Update Alpine.data registration** to remove dead functions -4. **Verify:** -```bash -npm run build:ts -# Should succeed with 0 errors -``` +4. **Update templates** to remove dead function calls -### Files That Need This Fix - -Based on commit 93710a1 and current errors: -- ✅ `web/src/collections.ts` - Already documented in COLLECTIONS_CLEANUP_GUIDE.md -- Check other files for similar issues as you encounter them +5. **Verify:** `npm run build:ts` and `templ generate` --- @@ -801,68 +1003,356 @@ Delete `showConflictModal()`, `hideConflictModal()` functions. **See `SSR_FIRST_ALPINE_GUIDE.md`** for complete SSR-first architecture principles. -### Quick Reference +--- -For detailed instructions on dashboard, docs, and other pages, see **`COLLECTIONS_CLEANUP_GUIDE.md`** - **Step 3** covers DOMContentLoaded removal. +### The Problem -### The Pattern +**DOMContentLoaded listeners run on EVERY page** due to `main.ts` importing all modules. -**Current (WRONG):** +**Example:** ```typescript -// ❌ Runs on EVERY page (main.ts imports all modules) -document.addEventListener("DOMContentLoaded", initializePage); +// web/src/admin.ts +document.addEventListener("DOMContentLoaded", () => { + initializeAdmin(); // Runs on index page! +}); ``` -**Solution 1: x-init Wrapper (Current Approach):** ```typescript -// ✅ Wrap in named function, call via x-init -function initializePage() { +// web/src/main.ts +import "./admin"; // Imports admin module on ALL pages +import "./dashboard"; // Imports dashboard module on ALL pages +``` + +--- + +### The Solution: Two Approaches + +#### Approach 1: x-init Wrapper (Current Approach) + +Wrap DOMContentLoaded logic in named function, call via x-init: + +```typescript +// web/src/admin.ts +function initializeAdmin() { setupEventListeners(); } -export { initializePage }; - -Alpine.data("page", () => ({ - initializePage, +Alpine.data("admin", () => ({ + initializeAdmin, })); ``` ```html - -
+ + ``` -**Solution 2: Event Delegation Only (Future Goal):** +#### Approach 2: Event Delegation Only (Future) + +Remove x-init entirely, rely on global event delegation: + ```typescript -// ✅ Rely on global event delegation, no init needed -// See ALPINE_COMPLETION_GUIDE.md for full migration path +// web/src/admin.ts +// NO init function - use global event delegation + +// Global event listener checks for data-action attributes +document.addEventListener("click", (e) => { + const action = e.target.closest("[data-action]")?.dataset.action; + if (action === "delete-library") deleteLibrary(); +}); ``` +```html + + + + + +``` + +**Which to Use?** +- **Current state:** Use Approach 1 (x-init wrapper) +- **Future goal:** Use Approach 2 (event delegation only) + +--- + ### Files Requiring Cleanup -**analytics.ts** (Type 3 - 80% JavaScript page): -- ✅ Already correct - uses `x-init="loadAnalytics"` -- ✅ Data fetch is intentional for this dynamic page +#### 3.1: analytics.ts - ALREADY CORRECT ✅ -**docs.ts** (Type 1 - 80% SSR page): -- ✅ Remove DOMContentLoaded -- ✅ Add `x-init="initializeDocsSearch"` to template -- ✅ Simple setup only, no data fetch +**File:** `web/src/analytics.ts` -**dashboard.ts** (Type 2 - SSR + Interactive page): -- ✅ Wrap existing DOMContentLoaded code in `initDashboard()` function -- ✅ Add `x-data="dashboard" x-init="initDashboard"` to template -- ✅ Does NOT fetch data on page load (SSR provides initial dashboard) -- ✅ Event delegation already in place with `data-action` attributes +**Type:** Type 3 (80% JavaScript page) -**library.ts** (Type 2 - SSR + Interactive page): -- ✅ Already fixed (commit 1b9bc64) -- ✅ Removed `reloadLibraries()` from `initializeLibraryAdmin()` -- ✅ SSR provides initial library list +**Status:** Already correct - no changes needed -### Implementation Steps +**Current:** +```typescript +export { loadAnalytics }; -For each file: +document.addEventListener("DOMContentLoaded", loadAnalytics); + +Alpine.data("analytics", () => ({ + loadAnalytics, +})); +``` + +**Change to:** +```typescript +export { loadAnalytics }; + +// REMOVE this line: +// document.addEventListener("DOMContentLoaded", loadAnalytics); + +Alpine.data("analytics", () => ({ + loadAnalytics, +})); +``` + +**Template Update:** `templates/analytics.templ` + +**Line ~14:** Add x-init to body tag: + +```html + + + + + +``` + +**✅ Simple setup only** +**✅ Data fetch is intentional** (analytics is a dynamic dashboard) +**✅ x-init is appropriate here** + +--- + +#### 3.2: docs.ts - SIMPLE SETUP ONLY + +**File:** `web/src/docs.ts` + +**Type:** Type 1 (80% SSR page) + +**Good news:** `initializeDocsSearch()` ONLY sets up an event listener - no data fetch! + +**Current (around line 95-100):** +```typescript +document.addEventListener("DOMContentLoaded", () => { + initializeDocsSearch(); +}); +``` + +**Solution:** Remove DOMContentLoaded, use x-init + +**Remove DOMContentLoaded:** +```typescript +// DELETE: +// document.addEventListener("DOMContentLoaded", () => { +// initializeDocsSearch(); +// }); + +export { toggleSidebar, initializeDocsSearch }; + +Alpine.data("docs", () => ({ + toggleSidebar, + initializeDocsSearch, // Keep as-is +})); +``` + +**Update template:** +```html + + +``` + +**✅ Simple setup only** +**✅ No data fetch** (search is client-side) +**✅ x-init is appropriate here** + +--- + +#### 3.3: library.ts - ALREADY FIXED ✅ + +**File:** `web/src/library.ts` + +**Type:** Type 2 (SSR + Interactive page) + +**Status:** ✅ **COMPLETED** - See commit 1b9bc64 + +**The SSR bug has been fixed:** +- Removed `void reloadLibraries()` from `initializeLibraryAdmin()` +- SSR provides initial library list (no fetch on page load) +- `reloadLibraries()` available for after CRUD operations only +- See `SSR_FIRST_ALPINE_GUIDE.md` for complete SSR-first principles + +**Current state (web/src/library.ts:653-655):** +```typescript +function initializeLibraryAdmin(): void { + // Setup event listeners + const librariesList = document.getElementById("libraries-list"); + if (librariesList) { + librariesList.addEventListener("click", handleLibraryListClick); + } + + // ... setup code ... + + // ✅ FIXED: No data fetch - SSR provides initial library list + // reloadLibraries() is called AFTER create/delete/update operations only +} +``` + +**Template (templates/admin_library.templ:11):** +```html + +``` + +**✅ No changes needed** - SSR bug is already fixed. + +--- + +#### 3.4: dashboard.ts - WRAP EXISTING CODE + +**File:** `web/src/dashboard.ts` + +**Type:** Type 2 (SSR + Interactive page) + +**Good news:** Dashboard already uses event delegation with `data-action` attributes! + +**Current (lines 494-521):** +```typescript +document.addEventListener("DOMContentLoaded", () => { + initDragAndDrop(); + + document.addEventListener("click", (e: Event) => { + // ... event delegation with data-action ... + }); + + // ... library select setup ... +}); +``` + +**Solution:** Wrap existing DOMContentLoaded code in `initDashboard()` function + +**Create wrapper function at end of dashboard.ts:** +```typescript +function initDashboard() { + initDragAndDrop(); // Setup drag-drop + + document.addEventListener("click", (e: Event) => { + const target = e.target as HTMLElement; + const actionElem = target.closest("[data-action]") as HTMLElement; + const action = actionElem?.getAttribute("data-action"); + + switch (action) { + case "scroll-carousel": { + const collectionId = + target.dataset.direction || actionElem?.dataset.direction || "0"; + if (collectionId) scrollCarousel(collectionId, direction); + break; + } + // ... keep all existing cases ... + } + }); + + document.addEventListener("input", (e: Event) => { + const target = e.target as HTMLElement; + const actionElem = target.closest("[data-input-action]") as HTMLElement; + const action = actionElem?.getAttribute("data-input-action"); + + switch (action) { + case "update-items-count": { + const input = target as HTMLInputElement; + const displayTarget = input.getAttribute("target"); + if (displayTarget) updateItemsCount(input, displayTarget); + break; + } + } + }); + + // Load saved library on page load + const librarySelect = document.getElementById( + "library-select", + ) as HTMLSelectElement; + if (librarySelect) { + librarySelect.addEventListener("change", (e) => { + const target = e.target as HTMLSelectElement; + if (target.value) { + switchLibrary(target.value); + } + }); + } + + const savedLibrary = localStorage.getItem("selectedLibrary"); + const currentLibrary = new URLSearchParams(window.location.search).get( + "library_id", + ); + if (savedLibrary && savedLibrary !== currentLibrary) { + window.location.href = `/dashboard?library_id=${savedLibrary}`; + } +} + +export { initDashboard }; + +Alpine.data("dashboard", () => ({ + initDashboard, +})); +``` + +**Remove the old DOMContentLoaded block** (delete lines 494-521): +```typescript +// DELETE this entire block: +// document.addEventListener("DOMContentLoaded", () => { +// initDragAndDrop(); +// ... all 70+ lines ... +// }); +``` + +**Add to export statement:** +```typescript +export { + closeDashboardSettings, + openDashboardSettings, + initDashboard, // ← ADD THIS + saveDashboardSettings, + scrollCarousel, + // ... keep all other exports ... +}; +``` + +**Add to Alpine.data:** +```typescript +Alpine.data("dashboard", () => ({ + closeDashboardSettings, + openDashboardSettings, + initDashboard, // ← ADD THIS + saveDashboardSettings, + scrollCarousel, + // ... keep all other exports ... +})); +``` + +**Template Update:** `templates/dashboard.templ` + +**Line ~19:** Add x-data and x-init to body tag: + +```html + + + + + +``` + +**✅ Keeps event delegation pattern** +**✅ Wraps existing code (minimal changes)** +**✅ x-init does NOT fetch data** (SSR provides initial dashboard) +**✅ localStorage redirect is user preference, not data fetch** + +--- + +### Implementation Steps (For Each File) + +For each file that needs DOMContentLoaded cleanup: 1. **Remove DOMContentLoaded:** ```typescript @@ -898,6 +1388,22 @@ Alpine.data("page", () => ({ --- +### Summary of Changes + +**Files Updated:** +- ✅ `web/src/analytics.ts` - Remove DOMContentLoaded, add x-init +- ✅ `web/src/docs.ts` - Remove DOMContentLoaded, add x-init +- ✅ `web/src/dashboard.ts` - Wrap existing code in initDashboard() +- ✅ `web/src/library.ts` - Already fixed (commit 1b9bc64) + +**Templates Updated:** +- ✅ `templates/analytics.templ` - Add x-init="loadAnalytics" +- ✅ `templates/docs.templ` - Add x-init="initializeDocsSearch" +- ✅ `templates/dashboard.templ` - Add x-data and x-init +- ✅ `templates/admin_library.templ` - Already correct + +--- + ## Phase 4: Verification & Testing ### For Each Migrated Template @@ -1006,9 +1512,9 @@ Add completion note: - 60% less TypeScript code (header.ts: 100 → 40 lines) ``` -### 4.3: How This Guide Relates to Others +### 4.3: Documentation Guide -**Three complementary guides:** +**Two complementary guides:** 1. **`SSR_FIRST_ALPINE_GUIDE.md`** - **READ THIS FIRST** - SSR-first architecture principles @@ -1017,16 +1523,11 @@ Add completion note: - Server-side token injection - **Must read before using this guide** -2. **`COLLECTIONS_CLEANUP_GUIDE.md`** - Quick reference for immediate fixes - - Dead export removal (Phase 0 prerequisites) - - DOMContentLoaded cleanup (Phase 3) - - Template regeneration - - Build verification steps - - **Use as step-by-step reference** - -3. **`ALPINE_COMPLETION_GUIDE.md`** - **This document** +2. **`ALPINE_COMPLETION_GUIDE.md`** - **This document** - Full reactive Alpine.js migration - - Eliminate all manual DOM manipulation + - Dead export removal (Phase 0) + - DOMContentLoaded cleanup (Phase 3) + - Template function call fixes - Header template reference implementation - Modal templates migration - **Long-term architecture goal** @@ -1575,7 +2076,7 @@ export { addSelectedBooks, searchBooksForCollections }; - Know when to fetch data 2. **Fix immediate console errors** (if needed) - - See `COLLECTIONS_CLEANUP_GUIDE.md` Step 1 + - See this guide, Phase 0: Dead Export Removal - Remove dead exports - Clean up DOMContentLoaded listeners - Verify builds work @@ -1602,12 +2103,9 @@ export { addSelectedBooks, searchBooksForCollections }; ### Documentation Strategy -**Goal:** Eventually deprecate `COLLECTIONS_CLEANUP_GUIDE.md` once all patterns are understood. - **Current state:** - `SSR_FIRST_ALPINE_GUIDE.md` - Architecture principles (permanent reference) -- `ALPINE_COMPLETION_GUIDE.md` - Full migration guide (active use) -- `COLLECTIONS_CLEANUP_GUIDE.md` - Step-by-step fixes (quick reference, will be deprecate) +- `ALPINE_COMPLETION_GUIDE.md` - Complete migration guide with all steps (this document) ### Key Success Factors