initial commit
This commit is contained in:
+161
@@ -0,0 +1,161 @@
|
||||
# Event Propagation: IIFE Capture-Phase Handler vs Save Buttons
|
||||
|
||||
## The Bug
|
||||
|
||||
On pages with the [Universal Modal Manager](#) IIFE (Section 14), clicking a Save button in the site-settings-modal closes the modal without saving. The form appears to work (checkboxes toggle, dropdowns change) but clicking Save does nothing.
|
||||
|
||||
## Root Cause
|
||||
|
||||
The IIFE adds a capture-phase listener on `document`:
|
||||
|
||||
```javascript
|
||||
document.addEventListener('click', function(e) {
|
||||
var closeBtn = e.target.closest('[aria-label="Close"],[data-close-modal]');
|
||||
if (closeBtn) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation(); // ← KILLS all subsequent handlers
|
||||
// ... closes modal
|
||||
}
|
||||
}, true); // capture phase
|
||||
```
|
||||
|
||||
`ensureSiteSettingsCloseAttributes()` in `settings.js` sets `data-close-modal` on **all three** buttons: close, cancel, AND save. When the user clicks Save:
|
||||
|
||||
1. Capture phase: IIFE matches `[data-close-modal]` on save button
|
||||
2. Calls `e.stopPropagation()` — prevents target phase (inline `onclick`) and bubble phase (`addEventListener` handlers) from ever firing
|
||||
3. Calls `window.closeModal('site-settings-modal')` — modal closes
|
||||
4. The save handler (`window.saveSiteSettings` or `setupUnifiedSiteSettingsModalHandlers.saveSettings`) **never runs**
|
||||
|
||||
## Diagnosis Signal
|
||||
|
||||
The save works on pages WITHOUT the IIFE (e.g., index.html, appointments.html, customers.html) but silently fails on pages WITH it (repair-orders.html).
|
||||
|
||||
To confirm: check if `ensureSiteSettingsCloseAttributes()` sets `data-close-modal` on the save button, and whether the page has the capture-phase IIFE:
|
||||
|
||||
```bash
|
||||
# Check for data-close-modal on save button
|
||||
grep -n 'save-site-settings.*data-close-modal' page.html
|
||||
|
||||
# Check for capture-phase IIFE
|
||||
grep -n "addEventListener('click'" page.html | grep 'true)'
|
||||
```
|
||||
|
||||
## The Fix (Three Parts)
|
||||
|
||||
### Part 1: Don't tag Save buttons with `data-close-modal`
|
||||
|
||||
In `settings.js`, `ensureSiteSettingsCloseAttributes()`:
|
||||
|
||||
```javascript
|
||||
// BEFORE (broken): sets data-close-modal on save button too
|
||||
const map = [{
|
||||
modalId: 'site-settings-modal',
|
||||
closeId: 'close-site-settings',
|
||||
cancelId: 'cancel-site-settings',
|
||||
saveId: 'save-site-settings' // ← REMOVE THIS
|
||||
}];
|
||||
|
||||
// AFTER (fixed): only close and cancel
|
||||
const map = [{
|
||||
modalId: 'site-settings-modal',
|
||||
closeId: 'close-site-settings',
|
||||
cancelId: 'cancel-site-settings'
|
||||
}];
|
||||
```
|
||||
|
||||
Same fix in `dashboard.js` if it also sets `data-close-modal` on save.
|
||||
|
||||
### Part 2: Remove inline `onclick` from Save button
|
||||
|
||||
Remove `onclick="saveSiteSettings()"` from the HTML. This prevents a double-fire when both the inline handler and the module-level `addEventListener` handler run.
|
||||
|
||||
### Part 3: Set `onclick` property in the IIFE with `stopImmediatePropagation()`
|
||||
|
||||
At the end of the IIFE (after the capture-phase handler), set the save button's `onclick` property directly:
|
||||
|
||||
```javascript
|
||||
// In the IIFE, after the capture-phase addEventListener
|
||||
var saveSettingsBtn = document.getElementById('save-site-settings');
|
||||
if (saveSettingsBtn) {
|
||||
saveSettingsBtn.onclick = function(e) {
|
||||
e.stopImmediatePropagation(); // prevents module-level addEventListener
|
||||
if (typeof window.saveSiteSettings === 'function') {
|
||||
window.saveSiteSettings();
|
||||
} else {
|
||||
// Fallback: direct localStorage save
|
||||
var s = {};
|
||||
try { var ex = localStorage.getItem('quoteGenSettings_v5');
|
||||
if (ex) s = JSON.parse(ex); } catch(ign) {}
|
||||
var gv = function(id, ck) {
|
||||
var el = document.getElementById(id);
|
||||
if (!el) return ck ? false : '';
|
||||
return ck ? !!el.checked : el.value;
|
||||
};
|
||||
s.darkMode = false;
|
||||
s.refreshRate = parseInt(gv('refresh-rate')) || 60;
|
||||
s.itemsPerPage = parseInt(gv('items-per-page')) || 25;
|
||||
s.desktopNotifications = gv('desktop-notifications', true);
|
||||
s.soundAlerts = gv('sound-alerts', true);
|
||||
s.criticalAlerts = gv('critical-alerts', true);
|
||||
s.notifyBeforePromised = parseInt(gv('notify-before-promised')) || 0;
|
||||
s.timezone = gv('timezone-select') || 'America/New_York';
|
||||
s.autoSaveForms = gv('auto-save-forms', true);
|
||||
localStorage.setItem('quoteGenSettings_v5', JSON.stringify(s));
|
||||
}
|
||||
window.closeModal('site-settings-modal');
|
||||
return false;
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
### Why `onclick` property instead of `addEventListener`?
|
||||
|
||||
Event phases execute in this order:
|
||||
1. **Capture phase** — IIFE on `document` (ignores save button now, no `data-close-modal`)
|
||||
2. **Target phase** — `onclick` property fires → saves → calls `stopImmediatePropagation()`
|
||||
3. **Bubble phase** — blocked by `stopImmediatePropagation()`; module-level `addEventListener` never fires
|
||||
|
||||
The `onclick` property fires in the target phase, AFTER the IIFE capture handler has already decided to ignore it, but BEFORE any bubbling-phase handlers. `stopImmediatePropagation()` then prevents duplicate saves from module-level listeners.
|
||||
|
||||
## Alternative: Add save handling to the capture-phase IIFE
|
||||
|
||||
Instead of Part 3 above, you can also handle the save in the capture-phase handler itself (before the close/cancel checks):
|
||||
|
||||
```javascript
|
||||
document.addEventListener('click', function(e) {
|
||||
// Check save button FIRST
|
||||
var saveBtn = e.target.closest('#save-site-settings');
|
||||
if (saveBtn) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation(); // prevents everything
|
||||
if (typeof window.saveSiteSettings === 'function') {
|
||||
window.saveSiteSettings();
|
||||
} else {
|
||||
// ... fallback save ...
|
||||
}
|
||||
window.closeModal('site-settings-modal');
|
||||
return;
|
||||
}
|
||||
// Then close/cancel checks...
|
||||
}, true);
|
||||
```
|
||||
|
||||
This is simpler but re-adding `data-close-modal` would be required, and the capture handler must be kept in sync with the save logic.
|
||||
|
||||
## Full Event Trace (After Fix)
|
||||
|
||||
For the Save button after all three fixes:
|
||||
|
||||
| Phase | Handler | Action |
|
||||
|-------|---------|--------|
|
||||
| Capture (document) | IIFE `addEventListener(..., true)` | No `data-close-modal` match → ignored |
|
||||
| Target | `saveSettingsBtn.onclick` | Reads form, saves to localStorage, calls `closeModal()` |
|
||||
| Bubble | Module `addEventListener` | Blocked by `stopImmediatePropagation()` |
|
||||
|
||||
For Close/Cancel (unchanged):
|
||||
|
||||
| Phase | Handler | Action |
|
||||
|-------|---------|--------|
|
||||
| Capture (document) | IIFE | Matches `[aria-label="Close"]` or `[data-close-modal]` → closes |
|
||||
| Target | `onclick` attribute | Blocked by `stopPropagation()` |
|
||||
| Bubble | Module `addEventListener` | Blocked by `stopPropagation()` |
|
||||
Reference in New Issue
Block a user