mirror of
https://github.com/olehomelchenko/astrolabe-nvc.git
synced 2025-12-21 13:12:23 +00:00
refactor: enhance generic list rendering and simplify dataset/snippet management
This commit is contained in:
@@ -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">
|
||||
|
||||
@@ -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
|
||||
@@ -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'
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user