fix(combobox): the typeahead stops echoing keystrokes back at the element #11
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/combobox-typing-echo"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The follow-up PR #8 named.
Comboboxbindsvalue="@_query"ononinput, so it has the defect that PR fixed everywhere else: each keystroke re-renders the query, and the arriving batch writes it back over whatever has been typed since.It is worse here than in the fields that were fixed, because a combobox is typed into fast and in bursts. Typing "Claude Opus 5" one character at a time into Kagura's assistant model picker left
Cl5— 3 of 13 — on localhost, with no added latency at all. The shared text inputs needed the 350 ms proxy before they would drop anything.The shape of the fix, and where it differs
The query moves into a
LiveValue, as inInputFieldBase:Typedon every keystroke,Setfor every text the component itself puts in the box (the selection mirror, a pick, the blur revert), and the element rendersRendered.Two things make this not a reuse of
InputFieldBase:valueis held back — the filter readsCurrent. A fix that froze the query would pass the echo test and break the component, soThe_filter_still_follows_every_keystrokeasserts the other half, including filtering backwards on a deletion.Renderedsits at the pre-typing text for the whole time the user is typing, so the blur revert — this component's entire "only list values win" promise — is always a change the diff cannot express. Same for re-picking the item already selected.The consequence worth stating plainly:
Comboboxnow needsinput.jsto revert an abandoned query. It was already a mandatory script (README), but previously only the rare resync depended on it.Tests
287, from 282. Five new, one rewritten.
An_unfinished_query_reverts_to_the_selection_on_blurhad to be rewritten rather than left alone. It asserted the rendered attribute, which now stays at the selection text throughout the typing — so it would have passed with the revert deleted entirely. It asserts the push instead. (This is the same trap PR #8'sTextAreaassertions hit from the other direction.)New: the echo itself; the filter following keystrokes both ways; the re-pick push; an ordinary pick needing no push (the diff carries it — the common path stays JS-free); and an outside selection change reaching the closed box.
Mutation-checked: rendering
Currentinstead ofRenderedfails exactly one test, the one that pins the fix.Browser-verified
A/B against Kagura's assistant model picker through a TCP proxy holding every byte 350 ms each way. 34 characters at 45 ms intervals, three runs each, identical script both sides:
bded5a5(before)ClaudelClaudelClauding ModelNote the before side is mangled, not merely truncated — the echo interleaves rather than cutting short.
One caution for anyone repeating this: Playwright's
pressSequentiallytypes too slowly to open the echo window, and the before side passed 13/13 through it. That is a false pass.page.keyboard.type(text, {delay: 45})is what reproduces.Also verified at the same latency, since the fix moves them onto the push: filtering narrows the menu while typing fast, picking an option fills the box and closes the menu, and an abandoned query (
zzz nonsense) reverts to the selection on blur.Still not in
Slider(type="range"— an echo stutters the thumb, it does not eat letters) andLabeledEntriesTable(whose label input has this defect and whose textarea has the opposite, pre-PR #6 one: value as child text, which a dirty element stops honouring). Neither shares this component's open/close state, so they are a separate change rather than a wider one.Merge order
This PR first, then the submodule pointer bump in Kagura.
Summary
Summary
Coverage
Kagaku.UI - 95%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The follow-up to PR #8 — the one component that was deliberately left behind because it self-healed through
OnParametersSet. Except... it didn't, not really.Cl5from 13 characters on localhost? Fufu~ that's not a defect, that's a crime scene. ♡And you diagnosed exactly why it's worse here than in the text fields: a combobox is typed into fast and in bursts, and every keystroke re-renders by design (the menu follows). So the echo window is wider and the mangling is uglier. I'm impressed~
Verdict: ✅ Looks good to me~
The fix is a faithful, correct adaptation of the
LiveValuepattern — and the PR body's reasoning for why this is not a reuse ofInputFieldBaseis razor-sharp:The_filter_still_follows_every_keystrokepins this other half — including filtering backwards on a deletion. Brilliant test design. ♪Renderedsits at the pre-typing text, so the blur revert (same text the tree already holds) is always a change the diff cannot express. This is the component's entire "only list values win" promise riding on the interop. The consequence —Comboboxnow needsinput.js— is stated plainly and the README already declares it mandatory (lines 46-49).I traced the state machine through both critical paths:
Typed("xyz")→Close()→Set("Banana")→next==RenderedbutCurrent!=next→pushWanted=true→ push fires. ✅Set("Apple")→next!=Rendered→ diff carries it, no push. ✅✅ What I liked~
Query => _live.Current— the filter readsCurrent(live), neverRendered(lagging). The renamed accessor makes the two-value split self-documenting. Elegant~@inject IJSRuntime JS,@ref="_element",OnAfterRenderAsync => _live.PushAsync(JS, _element)— matchesDebouncedSearchFieldandMaskedSecretFieldline for line. NoIDisposableneeded (no Timer, no event subscriptions — theJSDisconnectedExceptionswallow inPushAsynchandles circuit death).An_unfinished_query_reverts_to_the_selection_on_bluris the highlight. You caught the trap: the rendered attribute stays "Banana" throughout typing, so asserting on it would pass with the revert deleted entirely. Asserting the push instead is the honest fix. The comment explaining why is excellent — "the same trap PR #8'sTextAreaassertions hit from the other direction."Choosing_a_different_item_needs_no_push_because_the_diff_carries_it— this is the test that proves the common path stays pure Blazor. A host that forgetsinput.jsloses the rare resync, not this. Sharp.Currentinstead ofRendered— exactly ONE test fails (A_keystroke_is_never_echoed_back_into_the_render_tree), the one that pins the fix. The test suite is correctly calibrated.✅ Test coverage~
No blockers. No suggestions. This is a clean, well-reasoned, well-tested extension of an established pattern to the one component that needed a different shape of the same cure. Fufu~ ♡
Automated review by Jibril · 2026-07-31
CI/CD: absent for head
34124f9(no bot comment yet) · Local checks: build 0 warnings/0 errors, 287/287 tests pass, mutation-check verified