From 690d1c59534e64bb0994548f49113abc24ea8783 Mon Sep 17 00:00:00 2001
From: Oleh Omelchenko <ptrvtch@protonmail.com>
Date: Sun, 19 Oct 2025 02:49:20 +0300
Subject: [PATCH] refactor: enhance generic list rendering and simplify
 dataset/snippet management

---
 index.html                       |   2 +-
 project-docs/refactoring-plan.md | 272 -------------------------------
 src/js/dataset-manager.js        |  17 +-
 src/js/generic-storage-ui.js     |  45 ++++-
 src/js/snippet-manager.js        |  69 +++-----
 5 files changed, 65 insertions(+), 340 deletions(-)
 delete mode 100644 project-docs/refactoring-plan.md

diff --git a/index.html b/index.html
index 9afeba3..15e9cf7 100644
--- a/index.html
+++ b/index.html
@@ -83,7 +83,7 @@
                     <button class="btn btn-icon" id="search-clear" title="Clear search">×</button>
                 </div>
                 <div class="panel-content">
-                    <ul class="snippet-list">
+                    <ul class="snippet-list" id="snippet-list">
                         <!-- Dynamically populated by renderSnippetList() -->
                     </ul>
                     <div class="placeholder">
diff --git a/project-docs/refactoring-plan.md b/project-docs/refactoring-plan.md
deleted file mode 100644
index dbca4fa..0000000
--- a/project-docs/refactoring-plan.md
+++ /dev/null
@@ -1,272 +0,0 @@
-# Astrolabe Refactoring Plan
-
-**Date:** 2025-10-19
-**Goal:** Reduce code duplication and improve maintainability while preserving simplicity
-**Expected Impact:** Reduce codebase by ~400-500 lines (~7-10%)
-
----
-
-## High Impact, Low Effort Refactorings
-
-### 1. Consolidate Event Handlers in app.js
-**Current:** 795 lines with ~200 lines of repetitive addEventListener calls
-**Target:** ~650-700 lines
-**Savings:** ~100-150 lines
-
-#### Issues:
-- Lines 118-434 are primarily boilerplate event registration
-- Repetitive patterns for modal open/close
-- Each modal has dedicated open/close functions that just toggle display
-- Event listeners attached individually to each button
-
-#### Implementation:
-```javascript
-// Generic modal manager
-const ModalManager = {
-  open(modalId) {
-    const modal = document.getElementById(modalId);
-    if (modal) {
-      modal.style.display = 'flex';
-      this.trackOpen(modalId);
-    }
-  },
-  close(modalId) {
-    const modal = document.getElementById(modalId);
-    if (modal) modal.style.display = 'none';
-  },
-  trackOpen(modalId) {
-    const trackingMap = {
-      'help-modal': 'modal-help',
-      'donate-modal': 'modal-donate',
-      'settings-modal': 'modal-settings',
-      'dataset-modal': 'modal-dataset'
-    };
-    if (trackingMap[modalId]) {
-      Analytics.track(trackingMap[modalId], `Open ${modalId}`);
-    }
-  }
-};
-
-// Event delegation for modal closes
-document.addEventListener('click', (e) => {
-  // Close buttons
-  if (e.target.matches('[id$="-modal-close"]')) {
-    const modalId = e.target.id.replace('-close', '');
-    ModalManager.close(modalId);
-  }
-  // Overlay clicks
-  if (e.target.classList.contains('modal')) {
-    ModalManager.close(e.target.id);
-  }
-});
-```
-
-#### Files to modify:
-- `src/js/app.js` - Consolidate modal handlers
-
----
-
-### 2. Better Utilize generic-storage-ui.js
-**Current:** generic-storage-ui.js exists but is underused
-**Target:** Apply consistently across snippet-manager.js and dataset-manager.js
-**Savings:** ~100-150 lines
-
-#### Issues:
-- Both managers have duplicate patterns for:
-  - List rendering with item selection
-  - Linked item display
-  - Modal management
-  - Form validation
-  - Delete confirmations
-- generic-storage-ui.js has the abstractions but they're not fully applied
-
-#### Duplicated Functions to Consolidate:
-
-**Snippet Manager:**
-- `renderSnippetList()` → Use `renderGenericList()`
-- `attachSnippetEventListeners()` → Integrated into generic renderer
-- `selectSnippet()` → Use `selectGenericItem()`
-- `updateLinkedDatasets()` → Already uses `updateGenericLinkedItems()` ✓
-- `deleteSnippet()` → Already uses `confirmGenericDeletion()` ✓
-
-**Dataset Manager:**
-- `renderDatasetList()` → Use `renderGenericList()`
-- `attachDatasetEventListeners()` → Integrated into generic renderer
-- `selectDataset()` → Use `selectGenericItem()`
-- `updateLinkedSnippets()` → Use `updateGenericLinkedItems()`
-- `deleteCurrentDataset()` → Use `confirmGenericDeletion()`
-
-#### Implementation Strategy:
-1. Enhance `renderGenericList()` to handle ghost cards (e.g., "Create New Snippet")
-2. Add optional transformer for list items (format functions)
-3. Ensure `selectGenericItem()` handles both numeric and string IDs
-4. Update both managers to use generic functions consistently
-
-#### Files to modify:
-- `src/js/generic-storage-ui.js` - Enhance existing functions
-- `src/js/snippet-manager.js` - Apply generic list/select functions
-- `src/js/dataset-manager.js` - Apply generic list/select functions
-
----
-
-### 3. Simplify user-settings.js
-**Current:** 300 lines with complex dot-path API
-**Target:** ~220-250 lines
-**Savings:** ~50-80 lines
-
-#### Issues:
-- Dot-path API (`getSetting('editor.fontSize')`) is over-engineered for ~10 settings
-- `validateSetting()` has 40 lines for simple validation
-- Import/export functions (30 lines) appear unused
-- Path parsing logic (20 lines) in getter/setter
-
-#### Implementation:
-```javascript
-// Simpler API - direct property access
-const settings = getSettings();
-settings.editor.fontSize = 14;
-saveSettings();
-
-// Or keep helper but simplify:
-function getSetting(path) {
-  const [section, key] = path.split('.');
-  return userSettings?.[section]?.[key];
-}
-
-function updateSetting(path, value) {
-  const [section, key] = path.split('.');
-  if (userSettings[section]) {
-    userSettings[section][key] = value;
-    return saveSettings();
-  }
-  return false;
-}
-```
-
-#### Changes:
-1. Simplify getter/setter - assume 2-level structure (section.key)
-2. Inline validation - check ranges directly in `updateSettings()`
-3. Remove import/export if unused (check usage first)
-4. Keep `formatDate()` logic - it's used throughout UI
-
-#### Files to modify:
-- `src/js/user-settings.js` - Simplify API and validation
-
----
-
-## Medium Impact, Medium Effort (Future)
-
-### 4. Refactor config.js into Focused Modules
-**Current:** 257 lines of mixed concerns
-**Target:** Split into 3-4 focused files
-**Savings:** 0 LOC, but improves organization
-
-#### Proposed Structure:
-```
-src/js/
-  state.js          - URLState, global variables (editor, currentSnippetId, etc.)
-  ui-utils.js       - Toast, formatBytes, ModalManager
-  analytics.js      - Analytics wrapper
-  defaults.js       - sampleSpec, constants (STORAGE_LIMIT_BYTES, etc.)
-```
-
-#### Benefits:
-- Easier to find utilities
-- Clearer dependencies
-- Better mental model of what's "global"
-
-#### Files to create/modify:
-- Create: `src/js/state.js`, `src/js/ui-utils.js`, `src/js/analytics.js`, `src/js/defaults.js`
-- Modify: `src/js/config.js` (delete after migration)
-- Update: `index.html` script tags
-
----
-
-### 5. Extract snippet-drafts.js from snippet-manager.js
-**Current:** snippet-manager.js at 1393 lines handles many concerns
-**Target:** Split draft workflow into separate module
-**Savings:** 0 LOC, but improves maintainability
-
-#### Functions to Extract:
-- `loadSnippetIntoEditor()`
-- `updateViewModeUI()`
-- `switchViewMode()`
-- `publishDraft()`
-- `revertDraft()`
-- `autoSaveDraft()`
-- `debouncedAutoSave()`
-- `initializeAutoSave()`
-
-**Lines:** ~200-250 lines → `snippet-drafts.js`
-
-#### Benefits:
-- Easier to work on CRUD without draft complexity
-- Draft workflow becomes a clear, separate concern
-- Reduces cognitive load when reading snippet-manager.js
-
-#### Files to create/modify:
-- Create: `src/js/snippet-drafts.js`
-- Modify: `src/js/snippet-manager.js` (remove draft functions)
-- Update: `index.html` script tag order
-
----
-
-## Implementation Order
-
-### Phase 1: High Impact, Low Effort (This Session)
-1. ✅ Create this refactoring plan
-2. ✅ Consolidate event handlers in app.js (795 → 672 lines, -123)
-3. ✅ Simplify user-settings.js (300 → 224 lines, -76)
-4. ✅ Added ModalManager to config.js (257 → 303 lines, +46)
-5. ⏸️ Enhance and apply generic-storage-ui.js (IN PROGRESS)
-
-**Net Savings So Far: 153 lines (2.5% reduction)**
-
-### Phase 2: Medium Effort (Future Session)
-5. Refactor config.js into focused modules
-6. Extract snippet-drafts.js
-
----
-
-## Testing Strategy
-
-After each refactoring step:
-1. Open `index.html` in browser
-2. Test core workflows:
-   - Create/edit/delete snippet
-   - Create/edit/delete dataset
-   - Draft/publish workflow
-   - Search and sort snippets
-   - Import/export snippets
-   - Settings modal
-   - All keyboard shortcuts
-3. Check browser console for errors
-4. Verify localStorage and IndexedDB persistence
-
----
-
-## Rollback Strategy
-
-- Each refactoring committed separately
-- Git tags for each major step
-- Can revert individual changes via `git revert`
-
----
-
-## Success Metrics
-
-- **LOC Reduction:** 400-500 lines (~7-10%)
-- **Duplication:** Near-zero between snippet/dataset managers
-- **Complexity:** event handlers reduced from ~30 to ~10
-- **Maintainability:** Adding dataset search becomes trivial (reuse snippet search pattern)
-- **Philosophy:** Code size better matches feature importance
-
----
-
-## Notes
-
-- Keep changes incremental and testable
-- Preserve all existing functionality
-- Don't introduce new dependencies
-- Maintain vanilla JS approach
-- Update comments for clarity
diff --git a/src/js/dataset-manager.js b/src/js/dataset-manager.js
index 2310947..5b35fc9 100644
--- a/src/js/dataset-manager.js
+++ b/src/js/dataset-manager.js
@@ -278,16 +278,16 @@ async function fetchURLMetadata(url, format) {
 // Render dataset list in modal
 async function renderDatasetList() {
     const datasets = await DatasetStorage.listDatasets();
-    const listContainer = document.getElementById('dataset-list');
 
     if (datasets.length === 0) {
-        listContainer.innerHTML = '<div class="dataset-empty">No datasets yet. Click "New Dataset" to create one.</div>';
+        document.getElementById('dataset-list').innerHTML = '<div class="dataset-empty">No datasets yet. Click "New Dataset" to create one.</div>';
         return;
     }
 
     // Sort by modified date (most recent first)
     datasets.sort((a, b) => new Date(b.modified) - new Date(a.modified));
 
+    // Format individual dataset items
     const formatDatasetItem = (dataset) => {
         let metaText;
         if (dataset.source === 'url') {
@@ -318,15 +318,10 @@ async function renderDatasetList() {
         `;
     };
 
-    const html = datasets.map(formatDatasetItem).join('');
-    listContainer.innerHTML = html;
-
-    // Attach click handlers
-    document.querySelectorAll('.dataset-item').forEach(item => {
-        item.addEventListener('click', function() {
-            const datasetId = parseFloat(this.dataset.itemId);
-            selectDataset(datasetId);
-        });
+    // Use generic list renderer
+    renderGenericList('dataset-list', datasets, formatDatasetItem, selectDataset, {
+        emptyMessage: 'No datasets yet. Click "New Dataset" to create one.',
+        itemSelector: '.dataset-item'
     });
 }
 
diff --git a/src/js/generic-storage-ui.js b/src/js/generic-storage-ui.js
index 656b499..5fa2fbf 100644
--- a/src/js/generic-storage-ui.js
+++ b/src/js/generic-storage-ui.js
@@ -3,29 +3,56 @@
 // Used by both snippet-manager.js and dataset-manager.js
 
 /**
- * Generic list rendering function
+ * Generic list rendering function with support for ghost cards and custom selectors
  * @param {string} containerId - ID of container to render list into
  * @param {Array} items - Array of items to render
  * @param {Function} formatItem - Function that takes item and returns HTML string
  * @param {Function} onSelectItem - Callback when item is selected, receives item ID
- * @param {string} emptyMessage - Message to show when list is empty
+ * @param {Object} options - Optional configuration
+ *   - emptyMessage: Message when list is empty
+ *   - ghostCard: HTML string for "create new" card (prepended to list)
+ *   - onGhostCardClick: Callback for ghost card click
+ *   - itemSelector: CSS selector for clickable items (default: '[data-item-id]')
+ *   - ghostCardSelector: CSS selector for ghost card (default: '.ghost-card')
+ *   - parseId: Function to parse ID from string (default: parseFloat)
  */
-function renderGenericList(containerId, items, formatItem, onSelectItem, emptyMessage = 'No items found') {
+function renderGenericList(containerId, items, formatItem, onSelectItem, options = {}) {
+    const {
+        emptyMessage = 'No items found',
+        ghostCard = null,
+        onGhostCardClick = null,
+        itemSelector = '[data-item-id]',
+        ghostCardSelector = '.ghost-card',
+        parseId = parseFloat
+    } = options;
+
     const container = document.getElementById(containerId);
     if (!container) return;
 
-    if (items.length === 0) {
+    if (items.length === 0 && !ghostCard) {
         container.innerHTML = `<div class="list-empty">${emptyMessage}</div>`;
         return;
     }
 
-    const html = items.map(formatItem).join('');
-    container.innerHTML = html;
+    // Render ghost card + items
+    const itemsHtml = items.map(formatItem).join('');
+    container.innerHTML = (ghostCard || '') + itemsHtml;
+
+    // Attach click handler to ghost card
+    if (ghostCard && onGhostCardClick) {
+        const ghostElement = container.querySelector(ghostCardSelector);
+        if (ghostElement) {
+            ghostElement.addEventListener('click', onGhostCardClick);
+        }
+    }
+
+    // Attach click handlers to regular items
+    container.querySelectorAll(itemSelector).forEach(item => {
+        // Skip ghost cards
+        if (item.matches(ghostCardSelector)) return;
 
-    // Attach click handlers to items
-    container.querySelectorAll('[data-item-id]').forEach(item => {
         item.addEventListener('click', function() {
-            const itemId = this.dataset.itemId;
+            const itemId = parseId(this.dataset.itemId);
             onSelectItem(itemId);
         });
     });
diff --git a/src/js/snippet-manager.js b/src/js/snippet-manager.js
index 715195c..301dda9 100644
--- a/src/js/snippet-manager.js
+++ b/src/js/snippet-manager.js
@@ -315,42 +315,28 @@ function renderSnippetList(searchQuery = null) {
     }
 
     const snippets = SnippetStorage.listSnippets(null, null, searchQuery);
-    const snippetList = document.querySelector('.snippet-list');
     const placeholder = document.querySelector('.placeholder');
 
+    // Handle empty state with placeholder
     if (snippets.length === 0) {
-        snippetList.innerHTML = '';
+        document.querySelector('.snippet-list').innerHTML = '';
         placeholder.style.display = 'block';
-
-        // Show different message for search vs empty state
-        if (searchQuery && searchQuery.trim()) {
-            placeholder.textContent = 'No snippets match your search';
-        } else {
-            placeholder.textContent = 'No snippets found';
-        }
+        placeholder.textContent = searchQuery && searchQuery.trim()
+            ? 'No snippets match your search'
+            : 'No snippets found';
         return;
     }
 
     placeholder.style.display = 'none';
 
-    const ghostCard = `
-        <li class="snippet-item ghost-card" id="new-snippet-card">
-            <div class="snippet-name">+ Create New Snippet</div>
-            <div class="snippet-date">Click to create</div>
-        </li>
-    `;
-
     const currentSort = AppSettings.get('sortBy');
 
-    // Use generic formatter
+    // Format individual snippet items
     const formatSnippetItem = (snippet) => {
         // Show appropriate date based on current sort
-        let dateText;
-        if (currentSort === 'created') {
-            dateText = formatSnippetDate(snippet.created);
-        } else {
-            dateText = formatSnippetDate(snippet.modified);
-        }
+        const dateText = currentSort === 'created'
+            ? formatSnippetDate(snippet.created)
+            : formatSnippetDate(snippet.modified);
 
         // Calculate snippet size
         const snippetSize = new Blob([JSON.stringify(snippet)]).size;
@@ -377,11 +363,20 @@ function renderSnippetList(searchQuery = null) {
         `;
     };
 
-    const snippetItems = snippets.map(formatSnippetItem).join('');
-    snippetList.innerHTML = ghostCard + snippetItems;
+    // Ghost card for creating new snippets
+    const ghostCard = `
+        <li class="snippet-item ghost-card" id="new-snippet-card">
+            <div class="snippet-name">+ Create New Snippet</div>
+            <div class="snippet-date">Click to create</div>
+        </li>
+    `;
 
-    // Re-attach event listeners for snippet selection
-    attachSnippetEventListeners();
+    // Use generic list renderer
+    renderGenericList('snippet-list', snippets, formatSnippetItem, selectSnippet, {
+        ghostCard: ghostCard,
+        onGhostCardClick: createNewSnippet,
+        itemSelector: '.snippet-item'
+    });
 }
 
 // Initialize sort controls
@@ -558,26 +553,6 @@ function clearSelection() {
     }
 }
 
-// Attach event listeners to snippet items
-function attachSnippetEventListeners() {
-    const snippetItems = document.querySelectorAll('.snippet-item');
-    snippetItems.forEach(item => {
-        // Handle ghost card for new snippet creation
-        if (item.id === 'new-snippet-card') {
-            item.addEventListener('click', function () {
-                createNewSnippet();
-            });
-            return;
-        }
-
-        // Left click to select
-        item.addEventListener('click', function () {
-            const snippetId = parseFloat(this.dataset.itemId);
-            selectSnippet(snippetId);
-        });
-    });
-}
-
 // Select and load a snippet into the editor
 function selectSnippet(snippetId, updateURL = true) {
     const snippet = SnippetStorage.getSnippet(snippetId);