refactor: simplify modal handling and improve settings management

src/js/app.js

@@ -146,34 +146,22 @@ document.addEventListener('DOMContentLoaded', function () {
     }
 
     if (helpLink) {
-        helpLink.addEventListener('click', function () {
-            openHelpModal();
-        });
+        helpLink.addEventListener('click', () => ModalManager.open('help-modal'));
     }
 
     const donateLink = document.getElementById('donate-link');
     if (donateLink) {
-        donateLink.addEventListener('click', function () {
-            openDonateModal();
-        });
+        donateLink.addEventListener('click', () => ModalManager.open('donate-modal'));
     }
 
     // Settings Modal
     const settingsLink = document.getElementById('settings-link');
-    const settingsModal = document.getElementById('settings-modal');
-    const settingsModalClose = document.getElementById('settings-modal-close');
     const settingsApplyBtn = document.getElementById('settings-apply-btn');
     const settingsResetBtn = document.getElementById('settings-reset-btn');
     const settingsCancelBtn = document.getElementById('settings-cancel-btn');
 
     if (settingsLink) {
-        settingsLink.addEventListener('click', function () {
-            openSettingsModal();
-        });
-    }
-
-    if (settingsModalClose) {
-        settingsModalClose.addEventListener('click', closeSettingsModal);
+        settingsLink.addEventListener('click', openSettingsModal);
     }
 
     if (settingsCancelBtn) {
@@ -194,14 +182,6 @@ document.addEventListener('DOMContentLoaded', function () {
         });
     }
 
-    // Close on overlay click
-    if (settingsModal) {
-        settingsModal.addEventListener('click', function (e) {
-            if (e.target === settingsModal) {
-                closeSettingsModal();
-            }
-        });
-    }
 
     // Settings UI interactions
     const fontSizeSlider = document.getElementById('setting-font-size');
@@ -235,8 +215,6 @@ document.addEventListener('DOMContentLoaded', function () {
     // Dataset Manager
     const datasetsLink = document.getElementById('datasets-link');
     const toggleDatasetsBtn = document.getElementById('toggle-datasets');
-    const datasetModal = document.getElementById('dataset-modal');
-    const datasetModalClose = document.getElementById('dataset-modal-close');
     const newDatasetBtn = document.getElementById('new-dataset-btn');
     const cancelDatasetBtn = document.getElementById('cancel-dataset-btn');
     const saveDatasetBtn = document.getElementById('save-dataset-btn');
@@ -251,20 +229,6 @@ document.addEventListener('DOMContentLoaded', function () {
         toggleDatasetsBtn.addEventListener('click', openDatasetManager);
     }
 
-    // Close dataset manager
-    if (datasetModalClose) {
-        datasetModalClose.addEventListener('click', closeDatasetManager);
-    }
-
-    // Close on overlay click
-    if (datasetModal) {
-        datasetModal.addEventListener('click', function (e) {
-            if (e.target === datasetModal) {
-                closeDatasetManager();
-            }
-        });
-    }
-
     // New dataset button
     if (newDatasetBtn) {
         newDatasetBtn.addEventListener('click', showNewDatasetForm);
@@ -339,39 +303,21 @@ document.addEventListener('DOMContentLoaded', function () {
         });
     }
 
-    // Help Modal
-    const helpModal = document.getElementById('help-modal');
-    const helpModalClose = document.getElementById('help-modal-close');
-
-    if (helpModalClose) {
-        helpModalClose.addEventListener('click', closeHelpModal);
+    // Global modal event delegation - handles close buttons and overlay clicks
+    document.addEventListener('click', function(e) {
+        // Handle modal close buttons (×)
+        if (e.target.id && e.target.id.endsWith('-modal-close')) {
+            const modalId = e.target.id.replace('-close', '');
+            ModalManager.close(modalId);
+            return;
         }
 
-    // Close on overlay click
-    if (helpModal) {
-        helpModal.addEventListener('click', function (e) {
-            if (e.target === helpModal) {
-                closeHelpModal();
+        // Handle overlay clicks (clicking outside modal content)
+        if (e.target.classList.contains('modal')) {
+            ModalManager.close(e.target.id);
+            return;
         }
     });
-    }
-
-    // Donate Modal
-    const donateModal = document.getElementById('donate-modal');
-    const donateModalClose = document.getElementById('donate-modal-close');
-
-    if (donateModalClose) {
-        donateModalClose.addEventListener('click', closeDonateModal);
-    }
-
-    // Close on overlay click
-    if (donateModal) {
-        donateModal.addEventListener('click', function (e) {
-            if (e.target === donateModal) {
-                closeDonateModal();
-            }
-        });
-    }
 
     // View mode toggle buttons
     document.getElementById('view-draft').addEventListener('click', () => {
@@ -406,14 +352,8 @@ document.addEventListener('DOMContentLoaded', function () {
     }
 
     // Extract modal buttons
-    const extractModalClose = document.getElementById('extract-modal-close');
     const extractCancelBtn = document.getElementById('extract-cancel-btn');
     const extractCreateBtn = document.getElementById('extract-create-btn');
-    const extractModal = document.getElementById('extract-modal');
-
-    if (extractModalClose) {
-        extractModalClose.addEventListener('click', hideExtractModal);
-    }
 
     if (extractCancelBtn) {
         extractCancelBtn.addEventListener('click', hideExtractModal);
@@ -422,15 +362,6 @@ document.addEventListener('DOMContentLoaded', function () {
     if (extractCreateBtn) {
         extractCreateBtn.addEventListener('click', extractToDataset);
     }
-
-    // Close modal on overlay click
-    if (extractModal) {
-        extractModal.addEventListener('click', function (e) {
-            if (e.target === extractModal) {
-                hideExtractModal();
-            }
-        });
-    }
 });
 
 // Handle URL hash changes (browser back/forward)
@@ -495,8 +426,7 @@ const KeyboardActions = {
     },
 
     toggleSettings: function() {
-        const modal = document.getElementById('settings-modal');
-        if (modal && modal.style.display === 'flex') {
+        if (ModalManager.isOpen('settings-modal')) {
             closeSettingsModal();
         } else {
             openSettingsModal();
@@ -504,30 +434,19 @@ const KeyboardActions = {
     },
 
     closeAnyModal: function() {
-        const helpModal = document.getElementById('help-modal');
-        const datasetModal = document.getElementById('dataset-modal');
-        const extractModal = document.getElementById('extract-modal');
-        const donateModal = document.getElementById('donate-modal');
-        const settingsModal = document.getElementById('settings-modal');
-
-        if (helpModal && helpModal.style.display === 'flex') {
-            closeHelpModal();
+        // Try ModalManager first for standard modals
+        if (ModalManager.closeAny()) {
             return true;
         }
-        if (datasetModal && datasetModal.style.display === 'flex') {
-            closeDatasetManager();
-            return true;
-        }
-        if (extractModal && extractModal.style.display === 'flex') {
+        // Handle special modals with custom close logic
+        if (ModalManager.isOpen('extract-modal')) {
             hideExtractModal();
             return true;
         }
-        if (donateModal && donateModal.style.display === 'flex') {
-            closeDonateModal();
-            return true;
-        }
-        if (settingsModal && settingsModal.style.display === 'flex') {
-            closeSettingsModal();
+        // Dataset manager has special close logic (URL state)
+        const datasetModal = document.getElementById('dataset-modal');
+        if (datasetModal && datasetModal.style.display === 'flex') {
+            closeDatasetManager();
             return true;
         }
         return false;
@@ -594,56 +513,14 @@ function registerMonacoKeyboardShortcuts() {
         KeyboardActions.publishDraft);
 }
 
-// Help modal functions
-function openHelpModal() {
-    const modal = document.getElementById('help-modal');
-    if (modal) {
-        modal.style.display = 'flex';
-        // Track event
-        Analytics.track('modal-help', 'Open Help modal');
-    }
-}
-
-function closeHelpModal() {
-    const modal = document.getElementById('help-modal');
-    if (modal) {
-        modal.style.display = 'none';
-    }
-}
-
-// Donate modal functions
-function openDonateModal() {
-    const modal = document.getElementById('donate-modal');
-    if (modal) {
-        modal.style.display = 'flex';
-        // Track event
-        Analytics.track('modal-donate', 'Open Donate modal');
-    }
-}
-
-function closeDonateModal() {
-    const modal = document.getElementById('donate-modal');
-    if (modal) {
-        modal.style.display = 'none';
-    }
-}
-
-// Settings modal functions
+// Settings modal functions (special handling for loading settings into UI)
 function openSettingsModal() {
     loadSettingsIntoUI();
-    const modal = document.getElementById('settings-modal');
-    if (modal) {
-        modal.style.display = 'flex';
-        // Track event
-        Analytics.track('modal-settings', 'Open Settings modal');
-    }
+    ModalManager.open('settings-modal');
 }
 
 function closeSettingsModal() {
-    const modal = document.getElementById('settings-modal');
-    if (modal) {
-        modal.style.display = 'none';
-    }
+    ModalManager.close('settings-modal');
 }
 
 function loadSettingsIntoUI() {

src/js/config.js

@@ -224,6 +224,52 @@ const Analytics = {
     }
 };
 
+// Modal Manager - Generic modal control utility
+const ModalManager = {
+    // Track which events to send to analytics when opening modals
+    trackingMap: {
+        'help-modal': ['modal-help', 'Open Help modal'],
+        'donate-modal': ['modal-donate', 'Open Donate modal'],
+        'settings-modal': ['modal-settings', 'Open Settings modal'],
+        'dataset-modal': ['modal-dataset', 'Open Dataset Manager']
+    },
+
+    open(modalId, shouldTrack = true) {
+        const modal = document.getElementById(modalId);
+        if (modal) {
+            modal.style.display = 'flex';
+            if (shouldTrack && this.trackingMap[modalId]) {
+                const [event, title] = this.trackingMap[modalId];
+                Analytics.track(event, title);
+            }
+        }
+    },
+
+    close(modalId) {
+        const modal = document.getElementById(modalId);
+        if (modal) {
+            modal.style.display = 'none';
+        }
+    },
+
+    isOpen(modalId) {
+        const modal = document.getElementById(modalId);
+        return modal && modal.style.display === 'flex';
+    },
+
+    // Close any open modal (for ESC key handler)
+    closeAny() {
+        const modalIds = ['help-modal', 'donate-modal', 'settings-modal', 'dataset-modal', 'extract-modal'];
+        for (const modalId of modalIds) {
+            if (this.isOpen(modalId)) {
+                this.close(modalId);
+                return true;
+            }
+        }
+        return false;
+    }
+};
+
 // Shared utility: Format bytes for display
 function formatBytes(bytes) {
     if (bytes === null || bytes === undefined) return 'N/A';

src/js/user-settings.js

@@ -87,62 +87,35 @@ function getSettings() {
 }
 
 // Get a specific setting by path (e.g., 'editor.fontSize')
+// Simplified: assumes 2-level structure (section.key)
 function getSetting(path) {
     const settings = getSettings();
-    const parts = path.split('.');
-    let value = settings;
-
-    for (const part of parts) {
-        if (value && typeof value === 'object' && part in value) {
-            value = value[part];
-        } else {
-            return undefined;
-        }
-    }
-
-    return value;
+    const [section, key] = path.split('.');
+    return settings?.[section]?.[key];
 }
 
 // Update a specific setting by path
+// Simplified: assumes 2-level structure (section.key)
 function updateSetting(path, value) {
     const settings = getSettings();
-    const parts = path.split('.');
-    let target = settings;
-
-    // Navigate to the parent object
-    for (let i = 0; i < parts.length - 1; i++) {
-        const part = parts[i];
-        if (!target[part] || typeof target[part] !== 'object') {
-            target[part] = {};
-        }
-        target = target[part];
-    }
-
-    // Set the value
-    const lastPart = parts[parts.length - 1];
-    target[lastPart] = value;
+    const [section, key] = path.split('.');
 
+    if (settings[section]) {
+        settings[section][key] = value;
         return saveSettings();
     }
+    return false;
+}
 
 // Update multiple settings at once
 function updateSettings(updates) {
     const settings = getSettings();
 
     for (const path in updates) {
-        const parts = path.split('.');
-        let target = settings;
-
-        for (let i = 0; i < parts.length - 1; i++) {
-            const part = parts[i];
-            if (!target[part] || typeof target[part] !== 'object') {
-                target[part] = {};
+        const [section, key] = path.split('.');
+        if (settings[section]) {
+            settings[section][key] = updates[path];
         }
-            target = target[part];
-        }
-
-        const lastPart = parts[parts.length - 1];
-        target[lastPart] = updates[path];
     }
 
     return saveSettings();
@@ -154,23 +127,6 @@ function resetSettings() {
     return saveSettings();
 }
 
-// Export settings as JSON
-function exportSettings() {
-    return JSON.stringify(getSettings(), null, 2);
-}
-
-// Import settings from JSON string
-function importSettings(jsonString) {
-    try {
-        const imported = JSON.parse(jsonString);
-        userSettings = mergeWithDefaults(imported);
-        return saveSettings();
-    } catch (error) {
-        console.error('Error importing settings:', error);
-        return false;
-    }
-}
-
 // Format date according to user settings
 function formatDate(isoString, useFullFormat = false) {
     const date = new Date(isoString);
@@ -253,48 +209,16 @@ function formatCustomDate(date, format) {
     return result;
 }
 
-// Validate setting value
+// Validate setting value - simplified with inline validation rules
 function validateSetting(path, value) {
-    const errors = [];
+    const rules = {
+        'editor.fontSize': () => typeof value === 'number' && value >= 10 && value <= 18 ? [] : ['Font size must be between 10 and 18'],
+        'editor.theme': () => ['vs-light', 'vs-dark', 'hc-black'].includes(value) ? [] : ['Invalid editor theme'],
+        'editor.tabSize': () => typeof value === 'number' && value >= 2 && value <= 8 ? [] : ['Tab size must be between 2 and 8'],
+        'performance.renderDebounce': () => typeof value === 'number' && value >= 300 && value <= 3000 ? [] : ['Render debounce must be between 300-3000ms'],
+        'formatting.dateFormat': () => ['smart', 'locale', 'iso', 'custom'].includes(value) ? [] : ['Invalid date format'],
+        'ui.theme': () => ['light', 'experimental'].includes(value) ? [] : ['Invalid UI theme']
+    };
 
-    if (path === 'editor.fontSize') {
-        if (typeof value !== 'number' || value < 10 || value > 18) {
-            errors.push('Font size must be between 10 and 18');
-        }
-    }
-
-    if (path === 'editor.theme') {
-        const validThemes = ['vs-light', 'vs-dark', 'hc-black'];
-        if (!validThemes.includes(value)) {
-            errors.push('Invalid theme value');
-        }
-    }
-
-    if (path === 'editor.tabSize') {
-        if (typeof value !== 'number' || value < 2 || value > 8) {
-            errors.push('Tab size must be between 2 and 8');
-        }
-    }
-
-    if (path === 'performance.renderDebounce') {
-        if (typeof value !== 'number' || value < 300 || value > 3000) {
-            errors.push('Render debounce must be between 300 and 3000ms');
-        }
-    }
-
-    if (path === 'formatting.dateFormat') {
-        const validFormats = ['smart', 'locale', 'iso', 'custom'];
-        if (!validFormats.includes(value)) {
-            errors.push('Invalid date format value');
-        }
-    }
-
-    if (path === 'ui.theme') {
-        const validThemes = ['light', 'experimental'];
-        if (!validThemes.includes(value)) {
-            errors.push('Invalid theme value');
-        }
-    }
-
-    return errors;
+    return rules[path] ? rules[path]() : [];
 }