From 3e25339c867759c0edc704ed2f035070a05de2c7 Mon Sep 17 00:00:00 2001 From: Ulas Date: Mon, 30 Mar 2026 22:26:49 +0200 Subject: [PATCH] fix: resolve event-listener leaks and CSS gaps found in code quality audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - notes.js (Critical): move grid click listener from renderGrid() to render() — was re-registered on every save/pin/delete, causing multiple API calls per user action after several interactions - dashboard.js (Major): introduce AbortController (_fabController) so the anonymous document click listener from initFab() is cancelled on each new render() cycle; also remove the redundant initFab() call on the skeleton render - layout.css (Major): extend .label selector to include .form-label, covering usage in notes.js and settings.js without a mass-rename - test-modal-utils.js (Major): 12 unit tests for wireBlurValidation, btnSuccess, btnError; registered as test:modal-utils in package.json - notes.js (Minor): add btnError() shake feedback to save error handler - calendar.js (Minor): add popup.isConnected guard to closePopup so the listener self-removes correctly after navigation without a click Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 10 +++ package.json | 3 +- public/pages/calendar.js | 2 +- public/pages/dashboard.js | 13 ++- public/pages/notes.js | 35 ++++---- public/styles/layout.css | 2 +- test-modal-utils.js | 181 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 221 insertions(+), 25 deletions(-) create mode 100644 test-modal-utils.js diff --git a/CHANGELOG.md b/CHANGELOG.md index a971509..9e83fa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Accumulating click listeners on `#notes-grid` (Critical): listener is now registered once in `render()` via event delegation instead of re-registered in every `renderGrid()` call +- Accumulating anonymous `document` click listener in dashboard FAB: `initFab()` now accepts an AbortSignal; `render()` aborts the previous signal before creating a new one, eliminating listener leaks across navigation cycles +- Add `btnError()` shake feedback to notes.js save error handler for consistency with other modules +- Calendar event popup `closePopup` listener now checks `popup.isConnected` to self-remove correctly after navigation without a click + +### Added +- CSS alias `.form-label` alongside `.label` to cover usage in `notes.js` and `settings.js` without requiring a mass-rename +- Tests for `wireBlurValidation`, `btnSuccess`, and `btnError` (12 cases) in `test-modal-utils.js` + ## [0.2.0] - 2026-03-30 ### Changed diff --git a/package.json b/package.json index 23a4f00..5c33b86 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "test:calendar": "node --experimental-sqlite test-calendar.js", "test:ncb": "node --experimental-sqlite test-notes-contacts-budget.js", "test:ux-utils": "node test-ux-utils.js", - "test": "node --experimental-sqlite test-db.js && node --experimental-sqlite test-dashboard.js && node --experimental-sqlite test-tasks.js && node --experimental-sqlite test-shopping.js && node --experimental-sqlite test-meals.js && node --experimental-sqlite test-calendar.js && node --experimental-sqlite test-notes-contacts-budget.js && npm run test:ux-utils" + "test:modal-utils": "node test-modal-utils.js", + "test": "node --experimental-sqlite test-db.js && node --experimental-sqlite test-dashboard.js && node --experimental-sqlite test-tasks.js && node --experimental-sqlite test-shopping.js && node --experimental-sqlite test-meals.js && node --experimental-sqlite test-calendar.js && node --experimental-sqlite test-notes-contacts-budget.js && npm run test:ux-utils && npm run test:modal-utils" }, "dependencies": { "bcrypt": "^5.1.1", diff --git a/public/pages/calendar.js b/public/pages/calendar.js index dc64735..687430e 100644 --- a/public/pages/calendar.js +++ b/public/pages/calendar.js @@ -695,7 +695,7 @@ function showEventPopup(ev, anchor) { // Schließen bei Klick außerhalb setTimeout(() => { document.addEventListener('click', function closePopup(e) { - if (!popup.contains(e.target)) { + if (!popup.isConnected || !popup.contains(e.target)) { popup.remove(); document.removeEventListener('click', closePopup); } diff --git a/public/pages/dashboard.js b/public/pages/dashboard.js index 4ffabfb..b16e0c3 100644 --- a/public/pages/dashboard.js +++ b/public/pages/dashboard.js @@ -6,6 +6,9 @@ import { api } from '/api.js'; +// Hält den AbortController des aktuellen FAB-Listeners — wird bei jedem render() erneuert. +let _fabController = null; + // -------------------------------------------------------- // Hilfsfunktionen // -------------------------------------------------------- @@ -339,7 +342,7 @@ function renderFab() { `; } -function initFab(container) { +function initFab(container, signal) { const fabMain = container.querySelector('#fab-main'); const fabActions = container.querySelector('#fab-actions'); if (!fabMain) return; @@ -368,7 +371,7 @@ function initFab(container) { }); }); - document.addEventListener('click', () => { if (open) toggleFab(false); }); + document.addEventListener('click', () => { if (open) toggleFab(false); }, { signal }); } // -------------------------------------------------------- @@ -395,6 +398,9 @@ function wireLinks(container) { // -------------------------------------------------------- export async function render(container, { user }) { + _fabController?.abort(); + _fabController = new AbortController(); + container.innerHTML = `
@@ -412,7 +418,6 @@ export async function render(container, { user }) {
${renderFab()} `; - initFab(container); let data = { upcomingEvents: [], urgentTasks: [], todayMeals: [], pinnedNotes: [] }; let weather = null; @@ -455,6 +460,6 @@ export async function render(container, { user }) { `; wireLinks(container); - initFab(container); + initFab(container, _fabController.signal); if (window.lucide) window.lucide.createIcons(); } diff --git a/public/pages/notes.js b/public/pages/notes.js index e466404..6614abe 100644 --- a/public/pages/notes.js +++ b/public/pages/notes.js @@ -5,7 +5,7 @@ */ import { api } from '/api.js'; -import { openModal as openSharedModal, closeModal } from '/components/modal.js'; +import { openModal as openSharedModal, closeModal, btnError } from '/components/modal.js'; import { stagger, vibrate } from '/utils/ux.js'; // -------------------------------------------------------- @@ -71,6 +71,21 @@ export async function render(container, { user }) { state.notes = []; window.oikos?.showToast('Notizen konnten nicht geladen werden.', 'danger'); } + const grid = container.querySelector('#notes-grid'); + grid.addEventListener('click', async (e) => { + const pinBtn = e.target.closest('[data-action="pin"]'); + if (pinBtn) { e.stopPropagation(); await togglePin(parseInt(pinBtn.dataset.id, 10)); return; } + + const delBtn = e.target.closest('[data-action="delete"]'); + if (delBtn) { e.stopPropagation(); await deleteNote(parseInt(delBtn.dataset.id, 10)); return; } + + const card = e.target.closest('.note-card[data-id]'); + if (card) { + const note = state.notes.find((n) => n.id === parseInt(card.dataset.id, 10)); + if (note) openNoteModal({ mode: 'edit', note }); + } + }); + renderGrid(); const addHandler = () => openNoteModal({ mode: 'create' }); @@ -107,23 +122,6 @@ function renderGrid() { grid.innerHTML = state.notes.map((n) => renderNoteCard(n)).join(''); if (window.lucide) lucide.createIcons(); stagger(grid.querySelectorAll('.note-card')); - - grid.addEventListener('click', async (e) => { - // Pin - const pinBtn = e.target.closest('[data-action="pin"]'); - if (pinBtn) { e.stopPropagation(); await togglePin(parseInt(pinBtn.dataset.id, 10)); return; } - - // Delete - const delBtn = e.target.closest('[data-action="delete"]'); - if (delBtn) { e.stopPropagation(); await deleteNote(parseInt(delBtn.dataset.id, 10)); return; } - - // Edit - const card = e.target.closest('.note-card[data-id]'); - if (card) { - const note = state.notes.find((n) => n.id === parseInt(card.dataset.id, 10)); - if (note) openNoteModal({ mode: 'edit', note }); - } - }); } function renderNoteCard(note) { @@ -429,6 +427,7 @@ function openNoteModal({ mode, note = null }) { window.oikos?.showToast(mode === 'create' ? 'Notiz erstellt' : 'Notiz gespeichert', 'success'); } catch (err) { window.oikos?.showToast(err.data?.error ?? 'Fehler', 'error'); + btnError(saveBtn); saveBtn.disabled = false; saveBtn.textContent = isEdit ? 'Speichern' : 'Erstellen'; } diff --git a/public/styles/layout.css b/public/styles/layout.css index 8faa76a..5f0bba3 100644 --- a/public/styles/layout.css +++ b/public/styles/layout.css @@ -908,7 +908,7 @@ color: var(--color-text-disabled); } -.label { +.label, .form-label { display: block; font-size: var(--text-sm); font-weight: var(--font-weight-medium); diff --git a/test-modal-utils.js b/test-modal-utils.js new file mode 100644 index 0000000..898c86c --- /dev/null +++ b/test-modal-utils.js @@ -0,0 +1,181 @@ +/** + * Tests: Modal Utilities (wireBlurValidation, btnSuccess, btnError) + * Modul: /public/components/modal.js + * Läuft im Node-Kontext — die Utility-Funktionen greifen ausschließlich + * über ihre Parameter auf DOM-Objekte zu, daher kein DOM-Polyfill nötig. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +const { wireBlurValidation, btnSuccess, btnError } = await import('./public/components/modal.js'); + +const _origSetTimeout = setTimeout; + +// -------------------------------------------------------- +// DOM-Mocks +// -------------------------------------------------------- + +function makeField() { + const classes = new Set(); + return { + classList: { + toggle(cls, force) { force ? classes.add(cls) : classes.delete(cls); }, + contains(cls) { return classes.has(cls); }, + }, + _classes: classes, + }; +} + +function makeInput({ value = '', required = true } = {}) { + const listeners = {}; + const field = makeField(); + return { + value, + required, + _field: field, + _listeners: listeners, + addEventListener(event, fn) { listeners[event] = fn; }, + closest() { return field; }, + parentElement: field, + }; +} + +function makeContainer(inputs = []) { + return { + querySelectorAll(selector) { + if (selector.includes('required')) return inputs; + return []; + }, + }; +} + +function makeBtn({ textContent = 'Speichern' } = {}) { + const classes = new Set(); + const listeners = {}; + return { + textContent, + innerHTML: '', + offsetWidth: 0, + classList: { + add(cls) { classes.add(cls); }, + remove(cls) { classes.delete(cls); }, + contains(cls) { return classes.has(cls); }, + }, + addEventListener(event, fn) { listeners[event] = fn; }, + _classes: classes, + _listeners: listeners, + }; +} + +// -------------------------------------------------------- +// wireBlurValidation +// -------------------------------------------------------- + +test('wireBlurValidation: registriert blur-Listener auf required inputs', () => { + const input = makeInput(); + wireBlurValidation(makeContainer([input])); + assert.equal(typeof input._listeners['blur'], 'function'); +}); + +test('wireBlurValidation: blur mit leerem Wert setzt form-field--error', () => { + const input = makeInput({ value: '' }); + wireBlurValidation(makeContainer([input])); + input._listeners['blur'](); + assert.ok(input._field._classes.has('form-field--error')); + assert.ok(!input._field._classes.has('form-field--valid')); +}); + +test('wireBlurValidation: blur mit gültigem Wert setzt form-field--valid', () => { + const input = makeInput({ value: 'Hallo' }); + wireBlurValidation(makeContainer([input])); + input._listeners['blur'](); + assert.ok(input._field._classes.has('form-field--valid')); + assert.ok(!input._field._classes.has('form-field--error')); +}); + +test('wireBlurValidation: Whitespace-only gilt als leer → form-field--error', () => { + const input = makeInput({ value: ' ' }); + wireBlurValidation(makeContainer([input])); + input._listeners['blur'](); + assert.ok(input._field._classes.has('form-field--error')); +}); + +test('wireBlurValidation: kein Fehler wenn closest() null zurückgibt', () => { + const input = makeInput({ value: '' }); + input.closest = () => null; + input.parentElement = null; + wireBlurValidation(makeContainer([input])); + assert.doesNotThrow(() => input._listeners['blur']()); +}); + +// -------------------------------------------------------- +// btnSuccess +// -------------------------------------------------------- + +test('btnSuccess: fügt btn--success-Klasse hinzu', () => { + global.setTimeout = () => {}; + const btn = makeBtn(); + btnSuccess(btn, 'Test'); + assert.ok(btn._classes.has('btn--success')); + global.setTimeout = _origSetTimeout; +}); + +test('btnSuccess: setzt SVG-Checkmark als innerHTML', () => { + global.setTimeout = () => {}; + const btn = makeBtn(); + btnSuccess(btn, 'Test'); + assert.ok(btn.innerHTML.includes(' { + let capturedFn, capturedMs; + global.setTimeout = (fn, ms) => { capturedFn = fn; capturedMs = ms; }; + const btn = makeBtn({ textContent: 'Speichern' }); + btnSuccess(btn, 'Speichern'); + assert.equal(capturedMs, 700); + capturedFn(); + assert.ok(!btn._classes.has('btn--success')); + assert.equal(btn.textContent, 'Speichern'); + global.setTimeout = _origSetTimeout; +}); + +test('btnSuccess: nutzt btn.textContent als Fallback wenn kein Label übergeben', () => { + let capturedFn; + global.setTimeout = (fn) => { capturedFn = fn; }; + const btn = makeBtn({ textContent: 'Automatisch' }); + btnSuccess(btn); + capturedFn(); + assert.equal(btn.textContent, 'Automatisch'); + global.setTimeout = _origSetTimeout; +}); + +// -------------------------------------------------------- +// btnError +// -------------------------------------------------------- + +test('btnError: fügt btn--shaking-Klasse hinzu', () => { + const btn = makeBtn(); + btnError(btn); + assert.ok(btn._classes.has('btn--shaking')); +}); + +test('btnError: entfernt btn--shaking nach animationend', () => { + const btn = makeBtn(); + btnError(btn); + btn._listeners['animationend'](); + assert.ok(!btn._classes.has('btn--shaking')); +}); + +test('btnError: entfernt btn--shaking zuerst um Animation-Restart zu erzwingen', () => { + const order = []; + const btn = makeBtn(); + const origAdd = btn.classList.add.bind(btn); + const origRemove = btn.classList.remove.bind(btn); + btn.classList.remove = (cls) => { order.push(`remove:${cls}`); origRemove(cls); }; + btn.classList.add = (cls) => { order.push(`add:${cls}`); origAdd(cls); }; + btnError(btn); + assert.equal(order[0], 'remove:btn--shaking'); + assert.equal(order[1], 'add:btn--shaking'); +});