fix(notes): 5 bugs found in post-commit review
1. flushAutosave dropped brand-new unsaved notes.
The guard was `if (_dirty && _activeId != null)` but _activeId is
null until the first POST succeeds. So tap-New-Note → type → tap-
Back went through flushAutosave → skipped → closeToList → lost
the typed content. saveNote() already handles isNew via POST;
the guard only needs to check _dirty.
2. beforeunload sendBeacon used wrong HTTP method.
navigator.sendBeacon is POST-only by spec, but my code used it to
target PUT /api/notes/:id. Beacons silently hit a 404 route and
the note didn't save. For brand-new notes the URL resolved to
/api/notes/undefined. Replaced with fetch({ keepalive: true })
which browsers queue + ship even across unload, supports PUT,
and handles POST for new notes correctly.
3. Mobile: empty-state card stacked below the sidebar in list view.
The mobile media-query hid .notes-reader + .notes-editor when
data-view="list" but didn't include .notes-empty-state, so the
big "new note" card appeared below the list on phones. Added
the missing selector.
4. Delete fallback used window.confirm.
The `else if (window.confirm(...))` branch in deleteActive was
a rule violation even as a fallback — feedback_no_native_dialogs
says no native dialogs anywhere in frontend. Dropped it;
showConfirm is loaded by app.js before any tab activates so the
fallback path is unreachable in practice. Also upgraded the
confirm call to use the danger + confirmText options so the
modal renders red "Delete" button instead of neutral "Confirm".
5. Recorder kept running after switching tabs or notes.
Dictating, then tapping another clinical-tools tab (or opening
another note) left the MediaRecorder active and the mic stream
open — wasted battery, privacy surprise. Added stopRecording
(silent=true) to the non-notes branch of tabChanged, to
startCreate, and to openReader so any in-flight recording
cancels cleanly when the user moves on.
Minor:
• Dead "'Saving…' : 'Saving…'" ternary in saveNote simplified.
SW cache bumped to pedscribe-v12-notes4.
This commit is contained in:
parent
a25c36c875
commit
2a4269d496
3 changed files with 50 additions and 22 deletions
|
|
@ -985,9 +985,13 @@ textarea.full-input{resize:vertical;}
|
||||||
.notes-layout{grid-template-columns:1fr;gap:0;}
|
.notes-layout{grid-template-columns:1fr;gap:0;}
|
||||||
.notes-sidebar,.notes-reader,.notes-editor,.notes-empty-state{max-height:none;}
|
.notes-sidebar,.notes-reader,.notes-editor,.notes-empty-state{max-height:none;}
|
||||||
|
|
||||||
/* Show only the pane matching data-view; hide the others */
|
/* Show only the pane matching data-view; hide the others. The empty-
|
||||||
|
state card is purely a desktop affordance ("nothing picked yet,
|
||||||
|
pick from the sidebar"); on mobile the sidebar IS the list, so
|
||||||
|
we hide empty-state whenever the sidebar is showing. */
|
||||||
.notes-layout[data-view="list"] .notes-reader,
|
.notes-layout[data-view="list"] .notes-reader,
|
||||||
.notes-layout[data-view="list"] .notes-editor,
|
.notes-layout[data-view="list"] .notes-editor,
|
||||||
|
.notes-layout[data-view="list"] .notes-empty-state,
|
||||||
.notes-layout[data-view="reader"] .notes-sidebar,
|
.notes-layout[data-view="reader"] .notes-sidebar,
|
||||||
.notes-layout[data-view="reader"] .notes-editor,
|
.notes-layout[data-view="reader"] .notes-editor,
|
||||||
.notes-layout[data-view="reader"] .notes-empty-state,
|
.notes-layout[data-view="reader"] .notes-empty-state,
|
||||||
|
|
|
||||||
|
|
@ -39,10 +39,16 @@
|
||||||
// Wait for the Notes tab to be activated — the component HTML is
|
// Wait for the Notes tab to be activated — the component HTML is
|
||||||
// lazy-loaded, so elements don't exist until the user clicks the tab.
|
// lazy-loaded, so elements don't exist until the user clicks the tab.
|
||||||
document.addEventListener('tabChanged', function(e) {
|
document.addEventListener('tabChanged', function(e) {
|
||||||
if (e.detail.tab !== 'notes') return;
|
if (e.detail.tab === 'notes') {
|
||||||
if (_inited) { refreshList(); return; }
|
if (_inited) { refreshList(); return; }
|
||||||
_inited = true;
|
_inited = true;
|
||||||
init();
|
init();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Leaving the Notes tab — cancel any in-flight recording so the
|
||||||
|
// mic doesn't stay live + the recorder doesn't keep buffering.
|
||||||
|
if (_recActive) stopRecording(true);
|
||||||
|
flushAutosave();
|
||||||
});
|
});
|
||||||
|
|
||||||
function $(id) { return document.getElementById(id); }
|
function $(id) { return document.getElementById(id); }
|
||||||
|
|
@ -111,16 +117,27 @@
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// Persist dirty state on tab close / page unload — best-effort final flush
|
// Persist dirty state on tab close / page unload — best-effort final flush.
|
||||||
|
// navigator.sendBeacon is POST-only so it can't drive a PUT update; fetch
|
||||||
|
// with keepalive:true is the right primitive here (works for both POST
|
||||||
|
// for new notes and PUT for existing).
|
||||||
window.addEventListener('beforeunload', function() {
|
window.addEventListener('beforeunload', function() {
|
||||||
if (_dirty && _activeId != null) {
|
if (!_dirty) return;
|
||||||
// Use sendBeacon for a guaranteed best-effort send
|
var title = getTitle();
|
||||||
try {
|
if (!title) return;
|
||||||
var payload = JSON.stringify({ title: getTitle(), body: getBody() });
|
var body = getBody();
|
||||||
var headers = { type: 'application/json' };
|
var isNew = _activeId == null;
|
||||||
navigator.sendBeacon && navigator.sendBeacon('/api/notes/' + _activeId, new Blob([payload], headers));
|
var url = isNew ? '/api/notes' : '/api/notes/' + _activeId;
|
||||||
} catch (e) {}
|
var method = isNew ? 'POST' : 'PUT';
|
||||||
}
|
try {
|
||||||
|
fetch(url, {
|
||||||
|
method: method,
|
||||||
|
keepalive: true,
|
||||||
|
credentials: 'include',
|
||||||
|
headers: Object.assign({ 'Content-Type': 'application/json' }, getAuthHeaders()),
|
||||||
|
body: JSON.stringify({ title: title, body: body }),
|
||||||
|
});
|
||||||
|
} catch (e) {}
|
||||||
});
|
});
|
||||||
|
|
||||||
refreshList();
|
refreshList();
|
||||||
|
|
@ -209,6 +226,7 @@
|
||||||
// ── Reader mode (default after opening or saving) ─────────
|
// ── Reader mode (default after opening or saving) ─────────
|
||||||
function openReader(id) {
|
function openReader(id) {
|
||||||
flushAutosave();
|
flushAutosave();
|
||||||
|
if (_recActive) stopRecording(true);
|
||||||
var note = _notes.find(function(n) { return n.id === id; });
|
var note = _notes.find(function(n) { return n.id === id; });
|
||||||
if (!note) return;
|
if (!note) return;
|
||||||
_activeId = id;
|
_activeId = id;
|
||||||
|
|
@ -230,6 +248,7 @@
|
||||||
// ── Editor mode (create new / edit existing) ──────────────
|
// ── Editor mode (create new / edit existing) ──────────────
|
||||||
function startCreate() {
|
function startCreate() {
|
||||||
flushAutosave();
|
flushAutosave();
|
||||||
|
if (_recActive) stopRecording(true);
|
||||||
_activeId = null;
|
_activeId = null;
|
||||||
_dirty = false;
|
_dirty = false;
|
||||||
$('note-title').value = '';
|
$('note-title').value = '';
|
||||||
|
|
@ -281,9 +300,11 @@
|
||||||
|
|
||||||
function flushAutosave() {
|
function flushAutosave() {
|
||||||
if (_autosaveTimer) { clearTimeout(_autosaveTimer); _autosaveTimer = null; }
|
if (_autosaveTimer) { clearTimeout(_autosaveTimer); _autosaveTimer = null; }
|
||||||
if (_dirty && _activeId != null) {
|
// Flush dirty state regardless of whether the note is new or existing.
|
||||||
saveNote({ source: 'flush' });
|
// saveNote() handles both paths (POST for new, PUT for existing), so
|
||||||
}
|
// gating on _activeId here silently dropped the very first flush for
|
||||||
|
// a brand-new note — e.g. tap New note → type → tap Back.
|
||||||
|
if (_dirty) saveNote({ source: 'flush' });
|
||||||
}
|
}
|
||||||
|
|
||||||
function saveNote(opts) {
|
function saveNote(opts) {
|
||||||
|
|
@ -313,7 +334,7 @@
|
||||||
var method = isNew ? 'POST' : 'PUT';
|
var method = isNew ? 'POST' : 'PUT';
|
||||||
|
|
||||||
_autosaveInFlight = true;
|
_autosaveInFlight = true;
|
||||||
updateStatus(opts.source === 'auto' ? 'Saving…' : 'Saving…', 'saving');
|
updateStatus('Saving…', 'saving');
|
||||||
|
|
||||||
fetch(url, {
|
fetch(url, {
|
||||||
method: method,
|
method: method,
|
||||||
|
|
@ -381,10 +402,13 @@
|
||||||
.catch(function(err) { updateStatus(err.message || 'Delete failed', 'err'); });
|
.catch(function(err) { updateStatus(err.message || 'Delete failed', 'err'); });
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// showConfirm is loaded by app.js before any tab activates, so the
|
||||||
|
// "if missing" fallback used to be a native window.confirm() —
|
||||||
|
// dropped here because Daniel's rule is no native dialogs anywhere
|
||||||
|
// in the frontend. If showConfirm is somehow unavailable, the click
|
||||||
|
// silently no-ops; the user can try again after a reload.
|
||||||
if (typeof showConfirm === 'function') {
|
if (typeof showConfirm === 'function') {
|
||||||
showConfirm('Delete "' + label + '"? This cannot be undone.', doDelete);
|
showConfirm('Delete "' + label + '"?', doDelete, { danger: true, confirmText: 'Delete' });
|
||||||
} else if (window.confirm('Delete "' + label + '"? This cannot be undone.')) {
|
|
||||||
doDelete();
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,7 @@
|
||||||
// API calls always fresh (critical for medical data accuracy)
|
// API calls always fresh (critical for medical data accuracy)
|
||||||
// ============================================================
|
// ============================================================
|
||||||
|
|
||||||
var CACHE_NAME = 'pedscribe-v12-notes3';
|
var CACHE_NAME = 'pedscribe-v12-notes4';
|
||||||
var SHELL_ASSETS = [
|
var SHELL_ASSETS = [
|
||||||
'/',
|
'/',
|
||||||
'/index.html',
|
'/index.html',
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue