From 5382b9b2a9a0d772ec5c9816b4515fa9e2e34da7 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 12 Mar 2026 18:03:37 -0400 Subject: [PATCH] docs(cleanup): merge Alpine.js reactive pattern into SSR cleanup guide Updated COLLECTIONS_CLEANUP_GUIDE.md to present TWO approaches for each page, giving flexibility for quick fixes vs full migration. Two Approaches Now Available: OPTION A: Minimal Fix (Quick) - Fix SSR bugs by removing data fetch from init functions - Keep x-init for setup only (event listeners, modals) - Keep current event listener patterns - Good for quick fixes OPTION B: Full Alpine.js Reactive Pattern (Recommended) - See ALPINE_COMPLETION_GUIDE.md for complete pattern - Eliminate ALL manual DOM manipulation - Use x-data for state, x-show for visibility - Use @click.outside for closing dropdowns/modals - Use x-transition for smooth animations - No initialization functions needed - Aligns with long-term architecture Updates to Guide Sections: Step 3.2 (docs.ts): - Added Option A: Use x-init (simple) - Added Option B: Event delegation pattern - Recommendation: Option A (simple setup, no data fetch) Step 3.3 (library.ts): - Added Option A: Remove reloadLibraries() from init (quick) - Added Option B: Full Alpine.js reactive pattern - Shows how to eliminate manual DOM manipulation - Recommendation: Option B for cleanest architecture Step 3.4 (dashboard.ts): - Added Option A: Wrap in initDashboard() function - Added Option B: Remove DOMContentLoaded, use delegation - Notes event delegation already exists - Recommendation: Option A (keep current pattern) Updated Summary Section: - Added architecture decision point (Path 1 vs Path 2) - Documented mixed approach as recommended - Clear guidance on which approach to use when - References ALPINE_COMPLETION_GUIDE.md throughout This allows developer to choose approach based on: - Page complexity - Time constraints - Learning progression - Long-term architecture goals The guide is now flexible enough to support both quick fixes and full Alpine.js migration as the developer progresses through the app page-by-page. --- COLLECTIONS_CLEANUP_GUIDE.md | 479 ++++++++++++++++++++++++++++++----- 1 file changed, 421 insertions(+), 58 deletions(-) diff --git a/COLLECTIONS_CLEANUP_GUIDE.md b/COLLECTIONS_CLEANUP_GUIDE.md index 7bc6292..afe8dc2 100644 --- a/COLLECTIONS_CLEANUP_GUIDE.md +++ b/COLLECTIONS_CLEANUP_GUIDE.md @@ -322,10 +322,12 @@ Alpine.data("analytics", () => ({ ``` -### Step 3.2: docs.ts +### Step 3.2: docs.ts - SIMPLE SETUP, NO DATA FETCH **File:** `web/src/docs.ts` +**Good news:** `initializeDocsSearch()` ONLY sets up an event listener - no data fetch! + **Current (around line 95-100):** ```typescript document.addEventListener("DOMContentLoaded", () => { @@ -340,9 +342,15 @@ Alpine.data("docs", () => ({ })); ``` -**Change to:** +**TWO OPTIONS:** + +--- + +### **OPTION A: Use x-init (Simple)** + +**Remove DOMContentLoaded:** ```typescript -// REMOVE this entire block: +// DELETE: // document.addEventListener("DOMContentLoaded", () => { // initializeDocsSearch(); // }); @@ -351,29 +359,64 @@ export { toggleSidebar, initializeDocsSearch }; Alpine.data("docs", () => ({ toggleSidebar, - initializeSearch: initializeDocsSearch, + initializeDocsSearch, // Keep as-is })); ``` -**Template Update:** `templates/docs.templ` - -**Find the `` tag** and add x-init: - +**Update template:** ```html - - - - - + + ``` -### Step 3.3: library.ts - SSR-FIRST FIX REQUIRED +**✅ Simple** +**✅ Clear initialization** -**⚠️ CRITICAL SSR BUG:** The `initializeLibraryAdmin()` function currently calls `reloadLibraries()` which fetches data from the API and replaces the SSR-rendered library list. This defeats SSR for the admin library page! +--- + +### **OPTION B: Event Delegation Pattern (See ALPINE_COMPLETION_GUIDE.md)** + +**If the search input uses data-action attributes, use global event delegation:** + +**Remove DOMContentLoaded and initializeDocsSearch entirely:** +```typescript +// DELETE both: +// document.addEventListener("DOMContentLoaded", () => { +// initializeDocsSearch(); +// }); +// function initializeDocsSearch() { /* ... */ } + +// Export only business logic +export { toggleSidebar }; // Keep if used + +// Search setup via global event delegation in main.ts or separate file +``` + +**Template:** +```html + + + +``` + +**✅ No initialization** +**✅ Works with existing event delegation** + +--- + +### **Recommendation: OPTION A** + +Since `initializeDocsSearch()` doesn't fetch data and only sets up a listener, using x-init is fine and keeps the code clear. + +### Step 3.3: library.ts - TWO APPROACHES + +**⚠️ CRITICAL SSR BUG:** The `initializeLibraryAdmin()` function currently calls `reloadLibraries()` which fetches data from the API and replaces the SSR-rendered library list. + +**Reference:** The full Alpine.js reactive pattern is documented in `ALPINE_COMPLETION_GUIDE.md` **File:** `web/src/library.ts` -**Problem (around line 653-655):** +**Current Problem (around line 653-655):** ```typescript function initializeLibraryAdmin(): void { // Setup event listeners @@ -389,7 +432,13 @@ function initializeLibraryAdmin(): void { } ``` -**Fix - Remove data fetch from init:** +**TWO SOLUTIONS - Choose one:** + +--- + +### **OPTION A: Minimal Fix (Quick) - Remove Data Fetch from Init** + +**Fix library.ts:** ```typescript function initializeLibraryAdmin(): void { // Setup event listeners @@ -406,45 +455,299 @@ function initializeLibraryAdmin(): void { } ``` -**Why this fix matters:** -- SSR renders the library list on the server for fast initial load -- Calling `reloadLibraries()` in x-init replaces this with a slower API call -- `reloadLibraries()` should ONLY be called after CRUD operations (create/delete/update) -- The `initializeLibraryAdmin()` should ONLY setup event listeners and modals - -**Template Update:** `templates/admin_library.templ` - -**Current state (line 11):** +**Template remains:** ```html ``` -**Status:** ✅ Already correct - template already has x-data and x-init setup -**Action:** No template change needed - just fix library.ts to not fetch data in init +**✅ Keeps current architecture** +**✅ Minimal code changes** +**⚠️ Still uses manual event listeners** -### Step 3.4: dashboard.ts +--- + +### **OPTION B: Full Alpine.js Reactive Pattern (Recommended) - See ALPINE_COMPLETION_GUIDE.md** + +**This approach eliminates ALL manual DOM manipulation and event listeners.** + +**Step 1: Update template to use reactive state:** + +**File:** `templates/admin_library.templ` + +**Find modals (around lines 93-155):** + +**Current (create library modal):** +```html + +``` + +**Change to:** +```html + +
+ + + + + +
+``` + +**Update button to open modal (line 22):** +```html + + + + + +``` + +**Step 2: Remove manual DOM manipulation from library.ts:** + +**Delete these functions (no longer needed):** +```typescript +// DELETE ALL: +function showCreateLibraryModal() { /* ... */ } +function hideCreateLibraryModal() { /* ... */ } +function showDeleteModal() { /* ... */ } +function hideDeleteModal() { /* ... */ } +function showFolderBrowser() { /* ... */ } +function hideFolderBrowser() { /* ... */ } + +// DELETE initializeLibraryAdmin entirely: +function initializeLibraryAdmin() { /* ... */ } +``` + +**Keep ONLY business logic functions:** +```typescript +// KEEP - These are pure business logic, no DOM manipulation: +async function handleCreateLibrarySubmit(event: Event) { /* ... */ } +async function deleteLibrary(libraryId: string) { /* ... */ } +async function addLibraryFolder(libraryId: string) { /* ... */ } +async function removeLibraryFolder(libraryId: string, folderPath: string) { /* ... */ } +async function showLibraryFolders(libraryId: string) { /* renders folders via innerHTML - keep for now */ } +async function loadUserVisibility() { /* renders checkboxes via innerHTML - keep for now */ } +async function setLibraryVisibility(userId: string, libraryId: string, isVisible: boolean) { /* ... */ } +``` + +**Step 3: Remove x-init from template:** + +**File:** `templates/admin_library.templ` (line 11) + +```html + + + + + +``` + +**Step 4: Update Alpine.data registration:** + +**File:** `web/src/library.ts` (bottom of file) + +```typescript +// BEFORE: +Alpine.data("library", () => ({ + addLibraryFolder, + confirmDeleteLibrary, + deleteLibrary, + editLibrary, + handleCreateLibrarySubmit, + hideDeleteModal, + hideFolderBrowser, + initializeLibraryAdmin, // ← Remove this + loadUserVisibility, + navigateFolderBrowser, + removeLibraryFolder, + selectBrowseFolder, + setLibraryVisibility, + showDeleteModal, + showFolderBrowser, + showLibraryFolders, +})); + +// AFTER: +Alpine.data("library", () => ({ + // Only business logic functions + addLibraryFolder, + confirmDeleteLibrary, + deleteLibrary, + editLibrary, + handleCreateLibrarySubmit, + loadUserVisibility, + navigateFolderBrowser, + removeLibraryFolder, + selectBrowseFolder, + setLibraryVisibility, + showLibraryFolders, +})); +``` + +**✅ Eliminates ALL manual DOM manipulation** +**✅ No initialization needed** +**✅ Reactive state in template** +**✅ Follows full Alpine.js pattern from ALPINE_COMPLETION_GUIDE.md** + +--- + +### **Which Option to Choose?** + +**Choose Option A (Minimal Fix) if:** +- You want to fix the SSR bug quickly +- You're comfortable with manual event listeners +- You plan to refactor to full Alpine.js later + +**Choose Option B (Full Alpine.js) if:** +- You want the cleanest architecture +- You want to eliminate all manual DOM manipulation +- You're following the pattern in ALPINE_COMPLETION_GUIDE.md +- You want smooth transitions and better UX + +**Recommendation:** **Option B** - Full Alpine.js reactive pattern. It's cleaner, more maintainable, and aligns with the long-term architecture goal. + +### Step 3.4: dashboard.ts - EVENT DELEGATION ALREADY EXISTS **File:** `web/src/dashboard.ts` -**This file needs special handling** since it has a complex DOMContentLoaded block with multiple event listeners. +**Good news:** Dashboard already uses event delegation with `data-action` attributes! **Current (lines 494-521):** ```typescript document.addEventListener("DOMContentLoaded", () => { + initDragAndDrop(); // ← Drag-drop initialization + + 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": /* ... */ break; + // ... more cases ... + } + }); + + // ... library select setup ... +}); +``` + +**TWO OPTIONS:** + +--- + +### **OPTION A: Wrap in initDashboard() function** + +**Create wrapper function at end of dashboard.ts:** +```typescript +function initDashboard() { initDragAndDrop(); document.addEventListener("click", (e: Event) => { - // ... 70+ lines of event handling ... + // ... existing click handler code ... }); document.addEventListener("input", (e: Event) => { - // ... event handling ... + // ... existing input handler code ... }); - // ... more initialization ... + // ... library select and localStorage code ... +} + +export { initDashboard }; + +Alpine.data("dashboard", () => ({ + initDashboard, +})); +``` + +**Update template:** +```html + + +``` + +**✅ Keeps current event delegation pattern** +**✅ Simple wrapper function** +**⚠️ Still runs on page load** + +--- + +### **OPTION B: Remove DOMContentLoaded, Rely on Global Event Delegation** + +**Since dashboard already uses data-action attributes with global listeners, we can remove DOMContentLoaded entirely:** + +**Remove from dashboard.ts (lines 494-521):** +```typescript +// DELETE this entire block: +document.addEventListener("DOMContentLoaded", () => { + // ... all of it ... }); ``` +**Move drag-drop initialization to separate function (can be called later if needed):** +```typescript +// Drag-drop requires setup after content loads +// Since dashboard content is SSR'd, we can initialize immediately +function initDragAndDrop() { + // ... existing drag-drop code ... +} + +// Call it directly (no need to wait for DOMContentLoaded) +initDragAndDrop(); + +// Global event listeners are already set up in main.ts or run immediately +document.addEventListener("click", (e: Event) => { + // ... existing click handler ... +}); + +document.addEventListener("input", (e: Event) => { + // ... existing input handler ... +}); + +// ... library select setup ... +``` + +**No template changes needed:** +```html + + + +``` + +**✅ No initialization on page load** +**✅ Relies on existing event delegation** +**✅ SSR-first compatible** +**⚠️ Drag-drop initializes immediately (might be too early)** + +--- + +### **Recommendation: OPTION A** + +Use the wrapper function approach. The dashboard has complex initialization (drag-drop, localStorage check) that benefits from running after Alpine loads. The event delegation pattern is already correct - just wrap it properly. + **Create a wrapper function at the end of the file:** **Add before the export statement (before line ~490):** @@ -616,11 +919,11 @@ go build ./cmd/server ### Progress So Far -**Pages methodically reviewed:** -- ✅ **Dashboard** - Completed (drag-drop works, SSR maintained) -- 🔄 **Collections** - In progress (dead exports removed, more fixes needed) -- ⏸️ **Admin Library** - SSR bug identified, fix documented in Step 3.3 -- ⏸️ **All other pages** - Not yet reviewed, will be done page-by-page +**Pages methodically reviewed so far:** +- ✅ **Dashboard** - Completed, working correctly with SSR +- 🔄 **Collections** - In progress (fixes in progress) +- ⏸️ **Admin Library** - SSR bug identified, TWO fix approaches documented +- ⏸️ **Other pages** - Not yet reviewed, will be done page-by-page ### Completed Changes @@ -632,12 +935,41 @@ go build ./cmd/server **Template Files (1 file):** - `templates/analytics.templ` - ✅ Added x-init="loadAnalytics" (JS-heavy page, correct) -### Pending Critical Fixes +### Two Approaches Available -**SSR Bug in library.ts (Step 3.3):** -- ❌ `initializeLibraryAdmin()` calls `reloadLibraries()` which fetches data and replaces SSR -- ✅ Fix documented: Remove data fetch from init, only setup event listeners -- ⚠️ Template already correct: `templates/admin_library.templ` has x-data/x-init +**This guide now presents TWO approaches for each page:** + +**OPTION A: Minimal Fix (Quick)** +- Fix SSR bugs by removing data fetch from init functions +- Keep x-init for setup only +- Keep current event listener patterns +- Good for quick fixes + +**OPTION B: Full Alpine.js Reactive Pattern (Recommended - see ALPINE_COMPLETION_GUIDE.md)** +- Eliminate ALL manual DOM manipulation +- Use x-data for state, x-show for visibility +- Use @click.outside for closing dropdowns/modals +- Use x-transition for smooth animations +- No initialization functions needed +- Aligns with long-term architecture + +**Reference:** `ALPINE_COMPLETION_GUIDE.md` contains the complete Alpine.js pattern with examples + +### Pending Fixes + +**Admin Library (Step 3.3) - TWO approaches:** +- **Option A:** Remove `reloadLibraries()` from `initializeLibraryAdmin()` - Quick fix +- **Option B:** Full Alpine.js reactive pattern - Eliminate all manual DOM manipulation +- Template already has `x-data="library" x-init="initializeLibraryAdmin"` + +**Dashboard (Step 3.4):** +- **Option A:** Wrap existing code in `initDashboard()` function +- **Option B:** Remove DOMContentLoaded, rely on event delegation +- Already uses data-action event delegation correctly + +**Docs (Step 3.2):** +- **Option A:** Use x-init for search setup (simple, no data fetch) +- **Option B:** Use event delegation pattern ### What Was NOT Changed @@ -645,30 +977,61 @@ go build ./cmd/server - **WebSocket code** - Already moved to templates with server-side token injection - **API endpoints** - Already exist and work correctly - **HTMX modals** - Already implemented for collection CRUD -- **Search** - Skipped pending user's planned revamp -### SSR-First Principles Applied +### SSR-First Principles ✅ **Analytics (80% JS):** x-init fetches data - intentional for dynamic page ✅ **Dashboard (SSR + JS):** x-init ONLY sets up event listeners, no data fetch -✅ **Admin Library:** Will fix to only setup event listeners, SSR provides data +✅ **Admin Library:** Choose Option A (quick) or Option B (full Alpine.js) ✅ **Collections:** SSR provides data, JS for interactivity (in progress) -### Result So Far +**Key Principle:** +- ✅ DO in x-init: Set up event listeners, initialize modals, setup drag-drop +- ❌ DON'T in x-init: Fetch data via API, replace SSR innerHTML -✅ Collections dead exports removed -✅ Analytics loads correctly with SSR-first approach -✅ Admin WebSocket fixed with server-side token injection -⏳ Admin library SSR bug identified, fix ready to implement -⏳ Other pages will be reviewed methodically, one by one +### Architecture Decision Point + +**You're at a decision point:** + +**Path 1: Quick Fixes (Option A for all pages)** +- Fix SSR bugs by removing data fetch from inits +- Keep current event listener patterns +- Faster to complete +- Can refactor to full Alpine.js later + +**Path 2: Full Alpine.js Migration (Option B for all pages)** +- Follow ALPINE_COMPLETION_GUIDE.md pattern +- Eliminate all manual DOM manipulation +- Cleaner long-term architecture +- More work upfront, but better result + +**Recommendation:** Mix both approaches based on page complexity: +- Simple pages (docs): Use Option A +- Complex pages (admin library): Use Option B (full Alpine.js) +- This lets you learn the pattern progressively ### Next Steps -**Immediate (Critical SSR Bug):** -1. Fix `web/src/library.ts` Step 3.3 - Remove `reloadLibraries()` from `initializeLibraryAdmin()` -2. Test admin library page to verify SSR content is not replaced +**Choose your approach:** + +**If Path 1 (Quick Fixes):** +1. Apply Option A to admin_library, dashboard, docs +2. Continue page-by-page review +3. Plan full Alpine.js migration for later + +**If Path 2 (Full Alpine.js):** +1. Follow ALPINE_COMPLETION_GUIDE.md completely +2. Start with header.templ (reference implementation) +3. Apply Alpine.store pattern to all modals +4. Eliminate all manual DOM manipulation + +**If Mixed Approach (Recommended):** +1. Apply Option B (full Alpine.js) to admin_library +2. Apply Option A (quick fix) to dashboard and docs +3. Learn the pattern as you go +4. Gradually migrate to full Alpine.js **Continue Page-by-Page Review:** -3. Complete collections page fixes -4. Review and fix remaining pages one at a time -5. Update this guide as each page is completed +- Complete collections page fixes +- Review and fix remaining pages one at a time +- Update this guide as each page is completed