fix(forms): one bad select no longer kills every dropdown after it
Reported: dropdowns dead on the app config page only, in a private window (so not cache), with the served frontend confirmed identical to source. The enhancer ran in a bare forEach with no try/catch anywhere, and build() inserts its wrapper via `select.parentNode.insertBefore(...)`. A detached select makes that a null deref, and one throw abandoned the rest of the pass — every select AFTER it silently stayed native. The MutationObserver callback had the same exposure for the remaining mutation records in a batch. The app config page is the one that can produce a detached select: it builds its category panels in an async loop, so the observer can see a node a later render already replaced. That matches "only app config". Worse than losing the theme: if the throw landed after build() added .custom-select-native, the select was left opacity:0 / pointer-events: none behind a button with no listeners — a dropdown that looks right and does nothing. Now: detached/unconnected selects are skipped (they get enhanced when their subtree is attached and the observer fires again), every enhancement is individually guarded, a failed one is rolled back so it can never be left invisible, and failures console.warn with the field name instead of vanishing. Verified with jsdom against the real file: detached nodes skipped without throwing, a failure mid-pass leaves later selects working, and the survivors open and set their value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
4ee25292d5
commit
5e06e77d7f
@ -314,12 +314,46 @@
|
|||||||
if (el.closest('.custom-select')) return false;
|
if (el.closest('.custom-select')) return false;
|
||||||
if (el.multiple) return false; // multi-selects need different UX
|
if (el.multiple) return false; // multi-selects need different UX
|
||||||
if (el.hasAttribute('data-no-enhance')) return false;
|
if (el.hasAttribute('data-no-enhance')) return false;
|
||||||
|
// Detached nodes can't be enhanced: build() inserts the wrapper via
|
||||||
|
// select.parentNode, which is null here and would throw. This happens for
|
||||||
|
// real — the app config form builds its category panels in an async loop,
|
||||||
|
// so the observer can see a select that a later render already replaced.
|
||||||
|
// Skipping is safe: attaching the subtree fires the observer again, and the
|
||||||
|
// select is enhanced then, connected.
|
||||||
|
if (!el.isConnected || !el.parentNode) return false;
|
||||||
return ENHANCE_CLASSES.some(c => el.classList.contains(c));
|
return ENHANCE_CLASSES.some(c => el.classList.contains(c));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// One bad select must never cost the others their dropdown. Before this,
|
||||||
|
// enhancement ran in a bare forEach: a single throw aborted the whole pass,
|
||||||
|
// so every select AFTER it silently stayed native — page-wide breakage from
|
||||||
|
// one edge case, with nothing in the console to say so.
|
||||||
|
function enhanceOne(select) {
|
||||||
|
try {
|
||||||
|
new CustomSelect(select);
|
||||||
|
} catch (err) {
|
||||||
|
// Roll back a half-built widget: a select left with .custom-select-native
|
||||||
|
// is invisible (opacity 0, pointer-events none) — worse than un-enhanced.
|
||||||
|
try {
|
||||||
|
select.classList.remove('custom-select-native');
|
||||||
|
select[ENHANCED] = false;
|
||||||
|
const wrapper = select.closest('.custom-select');
|
||||||
|
if (wrapper && wrapper.parentNode) wrapper.parentNode.insertBefore(select, wrapper);
|
||||||
|
if (wrapper) wrapper.remove();
|
||||||
|
} catch (_) { /* best effort — the native select still works */ }
|
||||||
|
console.warn('[custom-select] could not enhance', select.name || select.id || select, err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function enhanceAll(root = document) {
|
function enhanceAll(root = document) {
|
||||||
|
// shouldEnhance is inside the guard too: it walks the DOM (closest), which
|
||||||
|
// can itself throw on a node being torn down mid-pass.
|
||||||
root.querySelectorAll(ENHANCE_SELECTOR).forEach(s => {
|
root.querySelectorAll(ENHANCE_SELECTOR).forEach(s => {
|
||||||
if (shouldEnhance(s)) new CustomSelect(s);
|
try {
|
||||||
|
if (shouldEnhance(s)) enhanceOne(s);
|
||||||
|
} catch (err) {
|
||||||
|
console.warn('[custom-select] skipped a select', err);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -330,10 +364,14 @@
|
|||||||
for (const m of mutations) {
|
for (const m of mutations) {
|
||||||
for (const node of m.addedNodes) {
|
for (const node of m.addedNodes) {
|
||||||
if (node.nodeType !== 1) continue;
|
if (node.nodeType !== 1) continue;
|
||||||
if (shouldEnhance(node)) {
|
// Guarded for the same reason as enhanceAll: a throw here would
|
||||||
new CustomSelect(node);
|
// abandon the remaining mutation records in this batch, so one
|
||||||
} else {
|
// awkward node could leave a whole freshly-rendered form native.
|
||||||
enhanceAll(node);
|
try {
|
||||||
|
if (shouldEnhance(node)) enhanceOne(node);
|
||||||
|
else enhanceAll(node);
|
||||||
|
} catch (err) {
|
||||||
|
console.warn('[custom-select] observer pass failed', err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user