fix(book-form): make contributor/author/publisher autocomplete race-proof#272
Merged
Conversation
…roof 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).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIl form aggiorna la ricerca server-side dei picker Choices.js per autori, contributori ed editori, disabilitando il filtro locale, sostituendo anche risultati vuoti e preservando l’input. È stato centralizzato il layout dell’input ed esteso il test PHP di regressione. ChangesAutocomplete server-side
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Views/libri/partials/book_form.php`:
- Around line 1425-1430: Remove the newChoices.length > 0 guards in
app/Views/libri/partials/book_form.php at lines 1425-1430, 2446-2448, and
2504-2508. Invoke authorsChoice.setChoices, choice.setChoices, and
publishersChoice.setChoices respectively with the existing arguments
unconditionally, including when newChoices is empty, so stale options are
cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1abe6e1b-f4d7-42b3-9044-40bffe49e415
📒 Files selected for processing (2)
app/Views/libri/partials/book_form.phptests/contributor-autocomplete-config.unit.php
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.
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.
…regression) The stackChoicesInput refactor replaced the authors-only forceInputWidth block, which also declared `const internalInput`. The authors picker's #74 _onEnterKey monkey-patch and its ensureAuthorChoice helpers still reference `internalInput`, so on the first keystroke the book form threw "ReferenceError: internalInput is not defined" — breaking add-author-via- Enter and the rest of the form's inline JS on both create and edit. Restore the declaration right after `const wrapper = element.closest`. Extend the config guard test (section E) to assert the authors picker declares internalInput before its _onEnterKey patch, so a future refactor that drops it fails the gate instead of shipping a broken form. Verified live: create (author via Enter + illustrator autocomplete), edit (add illustrator), delete (remove illustrator) all persist; console clean.
Himura2la
pushed a commit
to hackerembassy/Pinakes
that referenced
this pull request
Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The book form's server-search Choices.js pickers — illustrator/translator/curator/colorist contributors, plus the author and publisher pickers — fetch matches from
/api/search/autorion keystroke and feed them in viasetChoices(). But Choices.js's own client-side Fuse filter (searchChoices, defaulttrue) 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 stay hidden until the next keystroke. The autocomplete looked broken for partial queries (e.g. typingrodshowed nothing even thoughRodariexists).The backend was fine (
/api/search/autorireturns the matches); the results just weren't reaching the dropdown reliably.Fix
On the three server-search pickers:
searchChoices: false— disable Choices' client Fuse filter so it can't hide server results.setChoices(newChoices, 'value', 'label', true, false)—replaceChoices=truedrops the previous query's options (no stale results lingering now that the client filter is off) andclearSearchFlag=falseleaves the user's in-progress search input untouched.Verified on the live form (Playwright)
manz,rod,ein) reliably shows the matching existing authors/publishers.setChoicesthe typed text is NOT cleared and the earlier chip is preserved; selecting the second result yields both chips.Enter-to-add-a-new contributor (adding new author #74) still works.Test
tests/contributor-autocomplete-config.unit.php(9 assertions) locks in the structural fix —searchChoices: falseon all three pickers and thereplace=true, clearSearchFlag=falsesetChoices form, and guards against a revert to the old racy append form. The race itself is timing-dependent and can't be asserted deterministically, so behaviour is covered by the manual E2E above.Pre-existing issue, independent of the 0.7.38-rc.1 bundle.
Summary by CodeRabbit