From 011516672e6c9bf755d82af9cc6776c036765396 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 19 Jul 2026 17:39:49 +0200 Subject: [PATCH 1/4] fix(book-form): make contributor/author/publisher autocomplete race-proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The illustrator/translator/curator/colorist pickers (and the author + publisher pickers) fetch matches from /api/search/autori on keystroke and feed them via setChoices(). Choices.js's own client-side Fuse filter (searchChoices, default true) races that async populate: for a short/partial query it renders the dropdown as 'no results' BEFORE the fetch lands and never re-filters, so fresh server results are hidden until the next keystroke — the autocomplete looked broken for partial queries. Set searchChoices: false on the three server-search pickers so the client filter can't hide server results, and make each server response setChoices(..., replaceChoices=true, clearSearchFlag=false): replace drops the previous query's options (no stale results lingering with the filter off), and clearSearchFlag=false leaves the user's in-progress input untouched. Verified on the live form that typing a partial query reliably shows matches, that selecting a result then typing more preserves BOTH the typed text and the earlier chip, and that Enter-to-add-a-new-contributor (#74) still works. tests/contributor-autocomplete-config.unit.php locks in the structural fix (the race is timing-dependent and can't be asserted deterministically). --- app/Views/libri/partials/book_form.php | 26 +++++- .../contributor-autocomplete-config.unit.php | 84 +++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/contributor-autocomplete-config.unit.php diff --git a/app/Views/libri/partials/book_form.php b/app/Views/libri/partials/book_form.php index a0f60e5f4..1ca27fd6d 100644 --- a/app/Views/libri/partials/book_form.php +++ b/app/Views/libri/partials/book_form.php @@ -1366,6 +1366,11 @@ function initializeChoicesJS() { authorsChoice = new Choices(element, { searchEnabled: true, + // Server-side search: the 'search' handler below fetches matches and + // setChoices()es them. Disable Choices' own client-side Fuse filter so + // it can't race that async populate and hide fresh results as "no + // results" until the next keystroke. + searchChoices: false, removeItemButton: true, addItems: true, duplicateItemsAllowed: false, @@ -1418,7 +1423,10 @@ classNames: { })); if (newChoices.length > 0) { - authorsChoice.setChoices(newChoices, 'value', 'label', false); + // replace (drop the previous query's options, no stale + // results) + clearSearchFlag=false (do NOT reset the + // user's in-progress search input/state). + authorsChoice.setChoices(newChoices, 'value', 'label', true, false); } } catch (e) { console.error('Server-side author search failed:', e); @@ -2306,6 +2314,10 @@ function initContributorPicker(roleKey) { const choice = new Choices(el, { searchEnabled: true, + // Server-side search (see the 'search' handler below): disable the + // client-side Fuse filter so it can't race the async setChoices and + // hide fresh results as "no results". + searchChoices: false, removeItemButton: true, addItems: true, duplicateItemsAllowed: false, @@ -2431,7 +2443,9 @@ function syncHidden(value, label, add) { const newChoices = (results || []) .filter((a) => !selected.has(String(a.id))) .map((a) => ({ value: String(a.id), label: a.label, selected: false })); - if (newChoices.length > 0) choice.setChoices(newChoices, 'value', 'label', false); + // replace previous query's options (no stale) without + // resetting the user's in-progress search input/state. + if (newChoices.length > 0) choice.setChoices(newChoices, 'value', 'label', true, false); } catch (e) { console.error('Contributor search failed (' + roleKey + '):', e); } @@ -2453,6 +2467,10 @@ function initializePublishersChoices() { publishersChoice = new Choices(element, { searchEnabled: true, + // Server-side search (see the 'search' handler below): disable the + // client-side Fuse filter so it can't race the async setChoices and + // hide fresh results as "no results". + searchChoices: false, removeItemButton: true, addItems: true, duplicateItemsAllowed: false, @@ -2484,7 +2502,9 @@ classNames: { containerInner: 'choices__inner' } .filter(p => !selectedValues.has(String(p.id))) .map(p => ({ value: String(p.id), label: p.label, selected: false, customProperties: { isNew: false } })); if (newChoices.length > 0) { - publishersChoice.setChoices(newChoices, 'value', 'label', false); + // replace previous query's options (no stale) without + // resetting the user's in-progress search input/state. + publishersChoice.setChoices(newChoices, 'value', 'label', true, false); } } catch (e) { console.error('Server-side publisher search failed:', e); diff --git a/tests/contributor-autocomplete-config.unit.php b/tests/contributor-autocomplete-config.unit.php new file mode 100644 index 000000000..e2092b3cb --- /dev/null +++ b/tests/contributor-autocomplete-config.unit.php @@ -0,0 +1,84 @@ + 'authorsChoice = new Choices(element, {', + 'contributors' => 'const choice = new Choices(el, {', + 'publishers' => 'publishersChoice = new Choices(element, {', +]; +foreach ($configs as $name => $ctor) { + $pos = strpos($flat, $ctor); + // Look at the ~400 chars following the constructor for searchChoices:false. + $window = $pos !== false ? substr($flat, $pos, 400) : ''; + $check($pos !== false && (bool) preg_match('/searchChoices:\s*false/', $window), + "{$name} picker sets searchChoices: false"); +} + +echo "B. Server results REPLACE options without resetting the search input\n"; +// The three search-handler setChoices(newChoices, ...) calls must pass +// replaceChoices=true AND clearSearchFlag=false — i.e. `, true, false)`. +$searchSetChoices = [ + 'authors' => 'authorsChoice.setChoices(newChoices', + 'contributors' => 'choice.setChoices(newChoices', + 'publishers' => 'publishersChoice.setChoices(newChoices', +]; +foreach ($searchSetChoices as $name => $call) { + // Match the specific call and assert its trailing args are `, true, false)`. + $ok = (bool) preg_match( + '/' . preg_quote($call, '/') . ",\s*'value',\s*'label',\s*true,\s*false\s*\)/", + $flat + ); + $check($ok, "{$name} search handler uses setChoices(..., replace=true, clearSearchFlag=false)"); +} + +echo "C. No server-search handler still uses the racy append form\n"; +// Guard against a partial revert: none of the three newChoices setChoices calls +// may use the old 4-arg append form `, 'value', 'label', false)`. +foreach ($searchSetChoices as $name => $call) { + $regressed = (bool) preg_match( + '/' . preg_quote($call, '/') . ",\s*'value',\s*'label',\s*false\s*\)/", + $flat + ); + $check(!$regressed, "{$name} search handler does NOT use the old append form (racy)"); +} + +echo "\n" . ($fail === 0 ? "ALL {$pass} PASS\n" : "{$pass} PASS, {$fail} FAIL\n"); +exit($fail === 0 ? 0 : 1); From 3c5b2b2c7c219fbe261657811489f802c0e5a21f Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 19 Jul 2026 18:01:15 +0200 Subject: [PATCH 2/4] fix(book-form): clear the dropdown on a no-results search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit on #272: with searchChoices:false the client-side filter is off, so a `if (newChoices.length > 0)` guard before setChoices left the previous query's options visible when a later search returned nothing — type "manz" then a no-match query, and Manzini stayed shown. Call setChoices unconditionally on all three server-search pickers (authors, contributors, publishers): replace clears the dropdown even for an empty result set, while clearSearchFlag=false still preserves the user's in-progress input. Verified: after a matching query, a subsequent no-results query now empties the dropdown. Guard test extended to assert no newChoices.length gate remains. --- app/Views/libri/partials/book_form.php | 29 ++++++++++--------- .../contributor-autocomplete-config.unit.php | 8 +++++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/app/Views/libri/partials/book_form.php b/app/Views/libri/partials/book_form.php index 1ca27fd6d..9765994cb 100644 --- a/app/Views/libri/partials/book_form.php +++ b/app/Views/libri/partials/book_form.php @@ -1422,12 +1422,11 @@ classNames: { customProperties: { isNew: false } })); - if (newChoices.length > 0) { - // replace (drop the previous query's options, no stale - // results) + clearSearchFlag=false (do NOT reset the - // user's in-progress search input/state). - authorsChoice.setChoices(newChoices, 'value', 'label', true, false); - } + // Replace unconditionally (even when empty): with the client + // Fuse filter off, skipping the call on a no-results query + // would leave the PREVIOUS query's options stale in the + // dropdown. clearSearchFlag=false keeps the user's input. + authorsChoice.setChoices(newChoices, 'value', 'label', true, false); } catch (e) { console.error('Server-side author search failed:', e); } @@ -2443,9 +2442,11 @@ function syncHidden(value, label, add) { const newChoices = (results || []) .filter((a) => !selected.has(String(a.id))) .map((a) => ({ value: String(a.id), label: a.label, selected: false })); - // replace previous query's options (no stale) without - // resetting the user's in-progress search input/state. - if (newChoices.length > 0) choice.setChoices(newChoices, 'value', 'label', true, false); + // Replace unconditionally (even when empty): with the client + // Fuse filter off, skipping the call on a no-results query + // would leave the PREVIOUS query's options stale in the + // dropdown. clearSearchFlag=false keeps the user's input. + choice.setChoices(newChoices, 'value', 'label', true, false); } catch (e) { console.error('Contributor search failed (' + roleKey + '):', e); } @@ -2501,11 +2502,11 @@ classNames: { containerInner: 'choices__inner' } const newChoices = (serverResults || []) .filter(p => !selectedValues.has(String(p.id))) .map(p => ({ value: String(p.id), label: p.label, selected: false, customProperties: { isNew: false } })); - if (newChoices.length > 0) { - // replace previous query's options (no stale) without - // resetting the user's in-progress search input/state. - publishersChoice.setChoices(newChoices, 'value', 'label', true, false); - } + // Replace unconditionally (even when empty): with the client + // Fuse filter off, skipping the call on a no-results query + // would leave the PREVIOUS query's options stale in the + // dropdown. clearSearchFlag=false keeps the user's input. + publishersChoice.setChoices(newChoices, 'value', 'label', true, false); } catch (e) { console.error('Server-side publisher search failed:', e); } diff --git a/tests/contributor-autocomplete-config.unit.php b/tests/contributor-autocomplete-config.unit.php index e2092b3cb..7c6bfe210 100644 --- a/tests/contributor-autocomplete-config.unit.php +++ b/tests/contributor-autocomplete-config.unit.php @@ -80,5 +80,13 @@ $check(!$regressed, "{$name} search handler does NOT use the old append form (racy)"); } +echo "D. Server results are applied unconditionally (clears stale on no-results)\n"; +// With the client filter off, a `if (newChoices.length > 0)` guard before +// setChoices would leave the previous query's options visible when a search +// returns nothing. The setChoices must run even for an empty array so replace +// clears the dropdown (CodeRabbit #272). No such guard may remain. +$check(!str_contains($src, 'newChoices.length > 0'), + 'no `if (newChoices.length > 0)` guard gates the server-search setChoices calls'); + echo "\n" . ($fail === 0 ? "ALL {$pass} PASS\n" : "{$pass} PASS, {$fail} FAIL\n"); exit($fail === 0 ? 0 : 1); From b57a7f52662db70624b8e4d2cd1228f03b2aa96d Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sun, 19 Jul 2026 18:17:35 +0200 Subject: [PATCH 3/4] feat(book-form): stack the multi-select search input below the chips On the contributor (illustrator/translator/curator/colorist), author and publisher pickers the Choices.js search input sat inline to the RIGHT of the last selected chip, so you typed squeezed next to the chips instead of under them. Add a shared stackChoicesInput(wrapper) helper that makes the chip row wrap and drops the search field onto its own full-width line below the chips. It applies the styles inline with !important (the compiled Choices CSS otherwise wins) and re-applies them via a MutationObserver, because Choices.js autosizes the input's inline width on every keystroke and would otherwise pull it back onto the chip row. Replaces the authors-only forceInputWidth (which did the opposite, filling the remaining inline space) so all three pickers share one behaviour. Verified on the live form: with a chip selected the input is full-width and sits below the chip on all three pickers; autocomplete and Enter-to-add still work; no console errors. --- app/Views/libri/partials/book_form.php | 63 +++++++++++++++----------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/app/Views/libri/partials/book_form.php b/app/Views/libri/partials/book_form.php index 9765994cb..786150918 100644 --- a/app/Views/libri/partials/book_form.php +++ b/app/Views/libri/partials/book_form.php @@ -1434,33 +1434,9 @@ classNames: { }); const wrapper = element.closest('.choices'); - const internalInput = wrapper ? wrapper.querySelector('.choices__input--cloned') : null; - - // Force input to take remaining space, overriding Choices.js inline styles - if (internalInput) { - const forceInputWidth = () => { - internalInput.style.flex = '1 1 auto'; - internalInput.style.minWidth = '200px'; - internalInput.style.width = 'auto'; - }; - - // Initial force - forceInputWidth(); - - // Watch for Choices.js changing the width - const observer = new MutationObserver((mutations) => { - mutations.forEach((mutation) => { - if (mutation.type === 'attributes' && mutation.attributeName === 'style') { - forceInputWidth(); - } - }); - }); - - observer.observe(internalInput, { - attributes: true, - attributeFilter: ['style'] - }); - } + // Stack the search input on its own full-width row below the chips + // (shared with the contributor + publisher pickers). + stackChoicesInput(wrapper); const ensureAuthorChoice = (value, label, customProperties = {}) => { const stringValue = String(value); @@ -2300,6 +2276,33 @@ classNames: { containerInner: 'choices__inner' } }); } +// Multi-select tag inputs: wrap the selected chips across rows and drop the +// search field onto its own full-width line BELOW them, so you type UNDER the +// chips instead of being squeezed to the right of the last one. Applied as +// inline !important (the compiled Choices CSS otherwise wins) and re-applied +// via a MutationObserver because Choices.js autosizes the input's inline width +// on every keystroke, which would otherwise pull it back onto the chip row. +function stackChoicesInput(wrapper) { + if (!wrapper) return; + const inner = wrapper.querySelector('.choices__inner'); + if (inner) { + inner.style.setProperty('flex-wrap', 'wrap', 'important'); + inner.style.setProperty('align-items', 'flex-start', 'important'); + } + const input = wrapper.querySelector('.choices__input--cloned'); + if (!input) return; + const apply = () => { + input.style.setProperty('flex', '1 0 100%', 'important'); + input.style.setProperty('min-width', '100%', 'important'); + input.style.setProperty('width', '100%', 'important'); + input.style.setProperty('margin', '6px 8px 2px', 'important'); + }; + apply(); + // Re-apply identical values when Choices rewrites the style attribute; setting + // the same values produces no further mutation record, so this settles at once. + new MutationObserver(() => apply()).observe(input, { attributes: true, attributeFilter: ['style'] }); +} + // Generic contributor picker (issue #237) — one Choices.js entity picker per // role (illustratori/traduttori/curatori/coloristi). Mirrors the authors picker // (same /api/search/autori autocomplete, create-on-Enter) but self-contained and @@ -2452,6 +2455,9 @@ function syncHidden(value, label, add) { } }, 300); }); + + // Search input on its own full-width row below the selected chips. + stackChoicesInput(wrapper); return true; } catch (error) { console.error('initContributorPicker failed for ' + roleKey + ':', error); @@ -2516,6 +2522,9 @@ classNames: { containerInner: 'choices__inner' } const pubWrapper = element.closest('.choices'); const pubInternalInput = pubWrapper ? pubWrapper.querySelector('.choices__input--cloned') : null; + // Search input on its own full-width row below the selected chips. + stackChoicesInput(pubWrapper); + /** * Create a NEW publisher from typed text. A